Related to T2729. Added toggle switch in BalanceHeader component which controls toggleHideBalance setting in cashtabSettings object. BalanceHeaderFiat has also been changed to show or hide user balance in fiat using the same switch. Validation tests have been altered to account for the new setting. CashtabSettings is now passed down through the BalanceHeader component.
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABC3f04b8feaa69: [Cashtab] Added toggle hide balance switch
cd web/cashtab && npm start
from the home screen, toggle the switch
observe that xec and fiat balances are hidden
navigate to the send screen
observe that the balances are still hidden
from the send screen, toggle the switch
observe that the balances are now showing
check that setting persists across each component that contains the BalanceHeader
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Nice switch design and functionality is great.
- The switch should not change position when it changes value. Try to place it in the top right corner of the component.
- Add a balance header to the Receive page
web/cashtab/src/components/Common/BalanceHeader.js | ||
---|---|---|
18 ↗ | (On Diff #36013) | There's no need to have this be an option in cashtab settings. The toggle switch itself is the setting. |
web/cashtab/src/components/Common/BalanceHeader.js | ||
---|---|---|
18 ↗ | (On Diff #36013) | The initial task did request that is be a setting in cashtabSettings. From T2729 That said I can change it up if wanted. Making it a setting does seem like a clean way to implement it. |
web/cashtab/src/components/Common/BalanceHeader.js | ||
---|---|---|
18 ↗ | (On Diff #36013) | Sorry I thought I deleted this comment before I submitted the request for changes. Please ignore it. Your implementation is correct and the switch should change a setting. |
Responding to review feedback. I decided to make the switch a separate component. The switch in the top right of the banner looked off, so I am trying it on the same line as the wallet name and wallet rename button. The balance header has now been added to the receive.js page.
Switch positioning is much better.
The current implementation (rendering or not rendering the balance based on the switch) may be the best way to do it. However, I still really don't like content jumping anywhere in the app.
Ideally, hitting the switch will not cause any content jumping.
Best case scenario: a hidden balance has a neat effect, like spoilers in Telegram:
Next best: render dashes or some other same-sized component that illustrates the balance would be there but is hidden
Implement this and we'll compare if it's better than the current full-hide version with content jumping.
web/cashtab/src/components/Common/HideBalanceSwitch.js | ||
---|---|---|
9 ↗ | (On Diff #36068) | Because the parent components of this all already use WalletContext, changeCashtabSettings should be passed down as a prop and not initialized in this component |
Per review feedback: a blur effect is placed on the balance on toggle to avoid content jumping, changeCashtabSettings is passed to HideBalanceSwitch as a prop instead of initialized within the component.
Feature and styling look great. However, it is annoying that the UI locks when the switch is hit.
On reviewing the changeCashtabSettings function, this UI lock is implemented specifically to prevent incorrect balances being displayed when the fiat currency is updated. This makes sense. However, this was the first setting to be implemented. Other settings changes do not have this restriction.
Please modify the changeCashtabSettings function so that the UI is not locked when this setting is changed.
e.g. one way to do this is to only setLoading(true) if the requested settings change is not this one,
if (key !== 'toggleHideBalance') { setLoading(true); }