[Cashtab] Make sure hide balance toggle also hides XECX balance
ClosedPublic
Actions

Authored by bytesofman on Sun, Feb 2, 19:21.

Details

Reviewers

emack

Group Reviewers

Restricted Project

Commits

rABC9b9a406c97e4: [Cashtab] Make sure hide balance toggle also hides XECX balance

Summary

Currently the "hide balance" switch does not hide the XECX balance bc the colors in the CSS are off.

Patch.

Discovered that the prop is also confusingly named (the implementation is different from the setting) -- so, update that so it all makes sense.

Test Plan

npm test, see screenshots

Before

After

Diff Detail

Repository

rABC Bitcoin ABC

Branch

xecx-privacy-fix

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 32291
Build 64072: Build Diff	cashtab-tests
Build 64071: arc lint + arc unit

Event Timeline

bytesofman created this revision.Sun, Feb 2, 19:21

Herald added a reviewer: Restricted Project. · View Herald TranscriptSun, Feb 2, 19:21

bytesofman edited the test plan for this revision. (Show Details)Sun, Feb 2, 19:22

patch css implementation

bytesofman edited the test plan for this revision. (Show Details)Sun, Feb 2, 19:25

Harbormaster completed remote builds in B32291: Diff 52495.Sun, Feb 2, 19:26

Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests: <BalanceHeader /> XEC and XECX and fiat balances are hidden if cashtabSettings.balanceVisible is false ======
Error: expect(element).toHaveStyle()

- Expected

- text-shadow: 0 0 15px #fff;
+ text-shadow: 0 0 15px undefined;
    at Object.toHaveStyle (/work/cashtab/src/components/Common/__tests__/BalanceHeader.test.tsx:150:29)
    at Promise.then.completed (/work/cashtab/node_modules/jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/work/cashtab/node_modules/jest-circus/build/utils.js:231:10)
    at _callCircusTest (/work/cashtab/node_modules/jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at _runTest (/work/cashtab/node_modules/jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (/work/cashtab/node_modules/jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (/work/cashtab/node_modules/jest-circus/build/run.js:121:9)
    at run (/work/cashtab/node_modules/jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (/work/cashtab/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (/work/cashtab/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (/work/cashtab/node_modules/jest-runner/build/runTest.js:367:16)
    at runTest (/work/cashtab/node_modules/jest-runner/build/runTest.js:444:34)
    at Object.worker (/work/cashtab/node_modules/jest-runner/build/testWorker.js:106:12)

Each failure log is accessible here:
CashTab Unit Tests: <BalanceHeader /> XEC and XECX and fiat balances are hidden if cashtabSettings.balanceVisible is false

Harbormaster completed remote builds in B32292: Diff 52496.Sun, Feb 2, 19:30

Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests: <BalanceHeader /> XEC and XECX and fiat balances are hidden if cashtabSettings.balanceVisible is false ======
Error: expect(element).toHaveStyle()

- Expected

- text-shadow: 0 0 15px #fff;
+ text-shadow: 0 0 15px undefined;
    at Object.toHaveStyle (/work/cashtab/src/components/Common/__tests__/BalanceHeader.test.tsx:150:29)
    at Promise.then.completed (/work/cashtab/node_modules/jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/work/cashtab/node_modules/jest-circus/build/utils.js:231:10)
    at _callCircusTest (/work/cashtab/node_modules/jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at _runTest (/work/cashtab/node_modules/jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (/work/cashtab/node_modules/jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (/work/cashtab/node_modules/jest-circus/build/run.js:121:9)
    at run (/work/cashtab/node_modules/jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (/work/cashtab/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (/work/cashtab/node_modules/jest-circus/build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (/work/cashtab/node_modules/jest-runner/build/runTest.js:367:16)
    at runTest (/work/cashtab/node_modules/jest-runner/build/runTest.js:444:34)
    at Object.worker (/work/cashtab/node_modules/jest-runner/build/testWorker.js:106:12)

Each failure log is accessible here:
CashTab Unit Tests: <BalanceHeader /> XEC and XECX and fiat balances are hidden if cashtabSettings.balanceVisible is false

update tests

Harbormaster completed remote builds in B32296: Diff 52500.Mon, Feb 3, 00:32

bytesofman published this revision for review.Mon, Feb 3, 00:54

emack accepted this revision.Mon, Feb 3, 01:50

emack added a subscriber: emack.

emack added inline comments.

cashtab/src/components/Common/BalanceHeader.tsx
128 ↗	(On Diff #52500)	I know it's existing code but why do you need to specify `=== false` here, isn't passing `settings.balanceVisible` directly enough? Is this another ts linting rule?

This revision is now accepted and ready to land.Mon, Feb 3, 01:50

bytesofman marked an inline comment as done.Mon, Feb 3, 03:34

bytesofman added inline comments.

cashtab/src/components/Common/BalanceHeader.tsx
128 ↗	(On Diff #52500)	I don't remember why it was implemented this way. Probably is a typescript thing, but before `settings` itself was typed -- so this was a way to make sure we didn't hide the balance on undefined? it could also just be some kind of bug. The css param was opposite-defined before this diff. Now that this is a `bool`, we could simplify here...but I think beyond the scope of the diff.