Page MenuHomePhabricator

[Cashtab] Convert calculated fiat amount string to locale string
Needs ReviewPublic

Authored by emack on Wed, Oct 13, 23:42.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary

Linked to T1710:

  • Created new formatFiatBalance function in validation.js which takes in the fiatBalance and an optional locale parameter. A locale formatted string is then returned accordingly.
  • Created unit tests for formatFiatBalance in validation.test.js
  • Updated Send.js to pass in fiatPriceString to the formatFiatBalance function
  • Updated UI snapshots for Send
Test Plan
  • npm start
  • navigate to the Send UI
  • Ensure it is displaying zero fiat currency by default
  • Input 500000000 as the XEC amount and ensure the calculated fiat figure is displayed in locale format
  • npm run extension
  • Repeat above in browser extension mode

Diff Detail

Repository
rABC Bitcoin ABC
Branch
calcFiatToLocaleString
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17100
Build 34032: Build Diffcashtab-tests
Build 34031: arc lint + arc unit

Event Timeline

emack requested review of this revision.Wed, Oct 13, 23:43
bytesofman added inline comments.
web/cashtab/src/components/Send/Send.js
254 ↗(On Diff #30505)

need to do a diff with new version of prettier run on whole web/ folder before we use that version

web/cashtab/src/utils/validation.js
143 ↗(On Diff #30505)

let's call fBalance fiatBalance to fit better with the name of the function

This revision now requires changes to proceed.Thu, Oct 14, 22:49
emack marked 2 inline comments as done.
  • Removal of whitespaces was due to local IDE rather than Prettier. This has since been disabled in my IDE and whitespace removal reverted.
  • fBalance variable changed to fiatBalance.
bytesofman added inline comments.
web/cashtab/src/components/Send/Send.js
363 ↗(On Diff #30563)

This code block (inside this if() {} statement) needs to handle the case where cashtabSettings is false, similar to the code block it is replacing.

If cashtabSettings is false, assume currency is USD.

It's an edge case for wallets that haven't been loaded since this feature was pushed, but no reason not to preserve it.

web/cashtab/src/utils/validation.js
147 ↗(On Diff #30563)

Remove else {} ; redundant since using return

152 ↗(On Diff #30563)

remove else {}; redundant since using return

This revision now requires changes to proceed.Thu, Oct 21, 20:43
  • Added check for cashtabSettings edge case in Send.js
  • Removed the two redundant else{} blocks in Validation.js
  • Updated Send UI snapshots