Page MenuHomePhabricator

[Cashtab] Move defaultSettings out of Ticker.js
ClosedPublic

Authored by emack on Jun 25 2023, 08:33.

Details

Reviewers
bytesofman
Fabien
Group Reviewers
Restricted Project
Commits
rABC00ecb4ef0c65: [Cashtab] Move defaultSettings out of Ticker.js
Summary

Similar to D14102, this diff moves the defaultSettings config out of Ticker.js and into it's own config.

Changed the name to cashtabSettings to reflect the fact that these settings can be updated within the Cashtab app, compared to all the other config files.

Test Plan
  • grep -r currency.defaultSettings src/ and ensure no outputs
  • npm test
  • npm start
  • access cashtab from a desktop browser with no existing cashtab cache and create a new wallet
  • navigate to Settings and verify that default values for:

Fiat Currency matches fiacCurrency in config
Send Confirmations matches sendModal in config
Hide messages from unknown sender matches hideMessagesFromUnknownSenders in config

  • navigate to Home screen and verify the balance eye toggle state (up top) matches balanceVisible in config
  • navigate to Send on a mobile (visit this diff on https://defaultsettings.netlify.app) and verify the camera automatically switches on to scan QR codes in line with autoCameraOn in config
  • change currency, send confirmation, balance visibility and hide messages from unknown senders settings and ensure they persist after closing and reopening app

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

emack requested review of this revision.Jun 25 2023, 08:33
Fabien requested changes to this revision.Jun 25 2023, 18:28
Fabien added inline comments.
cashtab/src/hooks/useWallet.js
46 ↗(On Diff #40975)

I don't think you need to alias cashtabSettings, this is unlikely to cause any name collision

This revision now requires changes to proceed.Jun 25 2023, 18:28
emack requested review of this revision.Jun 26 2023, 03:18
emack marked an inline comment as done.
emack added inline comments.
cashtab/src/hooks/useWallet.js
46 ↗(On Diff #40975)

there's a cashtabSettings state variable in useWallet.js (line 59) that makes these settings available to the rest of the app, so I think this aliasing is necessary in this instance

This revision is now accepted and ready to land.Jun 26 2023, 06:22
This revision was automatically updated to reflect the committed changes.
emack marked an inline comment as done.