Page MenuHomePhabricator

[Cashtab] UI Updates
ClosedPublic

Authored by johnkuney on Feb 3 2022, 01:35.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABCfaffd4737049: [Cashtab] UI Updates
Summary

Update to the overall design of Cashtab. Unfortunately it is a rather large diff, but necessary to have a cohesive change.
Majority of edits are just CSS rules with the exception of the added receive screen and layout edits.

Test Plan

npm start

  • make a new wallet and test sending and receiving
  • create a token and send it to another address
  • test all configuration settings

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cashtab-ui-update
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 18225
Build 36258: Build Diffcashtab-tests
Build 36257: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
web/cashtab/src/components/Tokens/CreateTokenForm.js
571 ↗(On Diff #32248)

Remove, don't just comment out

597 ↗(On Diff #32248)

Remove, don't just comment out

623 ↗(On Diff #32248)

Remove

800 ↗(On Diff #32248)

Remove commented out code like this

web/cashtab/src/components/Wallet/Tx.js
5 ↗(On Diff #32248)

These icons should be collected in their own file. Add them to already existing CustomIcons.js

215 ↗(On Diff #32248)

Remove commented out code

Edit: add camel cases, move icons, delete commented out code, rename theme file

Edit: Update extension app.js, move "create token" button so it always shows

bytesofman requested changes to this revision.Feb 8 2022, 17:15
  1. See naming adjustments in inline comments above
  2. Add unit tests for new Receive.js
  3. Rebase to latest master
web/cashtab/src/components/Receive/Receive.js
115 ↗(On Diff #32256)

This should be renamed Receive in line with the screen's name change

137 ↗(On Diff #32256)

Same here

web/cashtab/src/components/Wallet/Wallet.js
235 ↗(On Diff #32256)

Seems like this should be Home now -- also rename the file to Home.js instead of Wallet.js to avoid ambiguity with Receive.js

257 ↗(On Diff #32256)

Home

This revision now requires changes to proceed.Feb 8 2022, 17:15

Edit: add test for receive, rename wallet directory to Home

Edit update service worker

Make revovery phrase selectable

  1. Modal bg color looks a little weird with new input background -- can you adjust it? e.g. when deleting a wallet:

image.png (258×546 px, 18 KB)

  1. Same same for the scan qr modal

image.png (507×515 px, 20 KB)

web/cashtab/src/components/Home/Home.js
221 ↗(On Diff #32310)

Move this button so it displays above the list -- for wallets with a lot of tokens, it's not intuitive that this will be at the bottom.

Change text from "Create a Token" to "Create eToken"

This revision now requires changes to proceed.Feb 10 2022, 01:09

edit: style tweaks to modals, move create token button up

Moving the "Multiple recipients" switch to Advanced is good UX. However I think we need one more tweak. Please disable the "private message" switch when multi-send is enabled -- e.g. grey it out, make it impossible to switch it away from "Public" if multi-send is enabled.

Currently, the app will let you turn on "private", but it reverts back to single send mode --- while keeping the multi-send text in the address bar (though it's impossible for the user to tell). The validation doesn't catch this, so if you try to send you get a weird msg like this:

image.png (99×1 px, 17 KB)

This revision now requires changes to proceed.Feb 10 2022, 18:31

edit: disable private message switch when multiple send is enable so user cannot change it after they've filled out multiple addresses

edit: use antd disable for private message toggle

edit: prevent screen jump on multi address select

  1. cashtab_bg.png should be deleted if it is not used anywhere. Loading up the image assets can take a couple of seconds
  2. Update icon imports for App.js for web app and extension to pull from CustomIcons and not @assets
  3. web/cashtab/src/components/Home/__tests__/Wallet.test.js should be renamed web/cashtab/src/components/Home/__tests__/Home.test.js
  4. Delete the TokenIconAlert component and its imports as it is no longer used. Some other commented out things that should be deleted, see inline comments.
  5. Weird that we are seeing changes to useBCH.test.js -- do another rebase to master.
web/cashtab/extension/src/components/App.js
8 ↗(On Diff #32343)

Add these icons to the existing CustomIcons.js file, export them from there, then you can import them directly here, import {HomeIcon, SendIcon, ReceiveIcon, SettingsIcon} from @components/customicons -- better code readability and keeps everything in one place.

web/cashtab/src/components/App.js
8 ↗(On Diff #32343)

Import all ReactComponent icon mods from CustomIcons

web/cashtab/src/components/Common/CustomIcons.js
80 ↗(On Diff #32343)

oh nice -- already here -- just update the App.js files to import from here

web/cashtab/src/components/Send/SendToken.js
5 ↗(On Diff #32343)

Delete this import and also delete the component from Alerts.js

256 ↗(On Diff #32343)

Delete

web/cashtab/src/components/Tokens/CreateTokenForm.js
513 ↗(On Diff #32343)

Delete instead of commenting out

538 ↗(On Diff #32343)

Delete instead of commenting out

web/cashtab/src/components/Tokens/Tokens.js
5 ↗(On Diff #32343)

Delete

71 ↗(On Diff #32343)

Delete instead of commenting out

This revision now requires changes to proceed.Feb 11 2022, 19:10

edit: fix conflict in tokens.js, remove commented code, import icons properly

Looks good. Couple of clean ups.

web/cashtab/src/components/Home/Home.js
38 ↗(On Diff #32376)

Delete instead of commenting out

web/cashtab/src/components/Tokens/Tokens.js
36 ↗(On Diff #32376)

Looks like tokens is no longer used on this page, please delete

This revision now requires changes to proceed.Feb 15 2022, 17:11

edit: remove commented code and rebase

This revision is now accepted and ready to land.Feb 16 2022, 03:46
This revision was automatically updated to reflect the committed changes.