Page MenuHomePhabricator

[Cashtab] Make the header balance a localeString
Needs ReviewPublic

Authored by emack on Wed, Oct 13, 06:19.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary
  • Updated formatBalance in @utils/CashMethods.js which is used to format the header XEC balance. The new function takes in the balance and an optional locale parameter and returns a formatted value accordingly. This function previously hard coded spaces for large balances.
  • New unit tests created in @utils/__tests__cashMEthods.test.js for the redesigned formatBalance function.
  • New snapshots generated across Wallet, Send and Tokens UI.
Test Plan
  • npm start
  • navigate to Wallet, Tokens and Send UIs and ensure the header XEC balance is displayed correctly per locale.
  • npm run extension
  • Check header XEC balance is displayed per locale in browser extension mode

Diff Detail

Repository
rABC Bitcoin ABC
Branch
balanceHeaderLocaleString
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17101
Build 34034: Build Diffcashtab-tests
Build 34033: arc lint + arc unit

Event Timeline

emack requested review of this revision.Wed, Oct 13, 06:19
bytesofman added inline comments.
web/cashtab/src/utils/cashMethods.js
50 ↗(On Diff #30477)

let's call wBalance something more specific, e.g. unformattedBalance

87 ↗(On Diff #30477)

prettier version issue; need a diff upgrading prettier on whole repo before we can use the new version

This revision now requires changes to proceed.Thu, Oct 14, 22:57
  • wBalance variable changed to unformattedBalance
  • Whitespace removal reverted as it was a local IDE issue rather than Prettier (not upgraded).

Looks good, just some small changes to improve readability with nested conditions

web/cashtab/src/utils/cashMethods.js
54 ↗(On Diff #30565)

We don't need this else block; the return statement makes it obsolete. Please remove.

59 ↗(On Diff #30565)

Same for this else block, please remove

This revision now requires changes to proceed.Wed, Oct 20, 23:44

removed first else block per feedback, retained second else block as it is needed to check for an optional parameter rather than error checking.

bytesofman added inline comments.
web/cashtab/src/utils/cashMethods.js
56 ↗(On Diff #30618)

Remove this else { } (keep the code inside, but just put it after the if block).

because the if block already returns a value if (optionalLocale === undefined), the code behavior is identical whether or not the else is there.

This revision now requires changes to proceed.Thu, Oct 21, 20:36

Removed redundant else{} block