Page MenuHomePhabricator

[Cashtab] Replace antd notifications and pop-up modals with react-toastify
ClosedPublic

Authored by bytesofman on Mar 23 2024, 13:22.

Details

Summary

T2207

Implement a non-antd notification component for Cashtab

Went with react-toastify as

  • it looks great
  • it is well supported
  • it "just works" out of the box, with the same API as antd
  • A custom notification component actually is a pretty big job if you want it to look good. No need to reinvent the wheel.

I'm sure toastify will have things I dislike but it won't consume the entire app like antd has.

Test Plan

npm test

This diff is deployed at https://cashtab-local-dev.netlify.app/

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cleanup-formats
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 28021
Build 55587: Build Diffcashtab-tests
Build 55586: arc lint + arc unit

Event Timeline

Tail of the build log:

Installing mock-chronik-client dependencies...
/work/modules/mock-chronik-client /work/abc-ci-builds/cashtab-tests

added 236 packages, and audited 237 packages in 1s

35 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
/work/cashtab /work/modules/mock-chronik-client /work/abc-ci-builds/cashtab-tests
npm WARN deprecated @babel/plugin-proposal-private-methods@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-methods instead.
npm WARN deprecated @babel/plugin-proposal-numeric-separator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-numeric-separator instead.
npm WARN deprecated @babel/plugin-proposal-nullish-coalescing-operator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm WARN deprecated rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
npm WARN deprecated @babel/plugin-proposal-class-properties@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm WARN deprecated @babel/plugin-proposal-private-property-in-object@7.21.11: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-property-in-object instead.
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
npm WARN deprecated abab@2.0.6: Use your platform's native atob() and btoa() methods instead
npm WARN deprecated domexception@4.0.0: Use your platform's native DOMException instead
npm WARN deprecated stable@0.1.8: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm WARN deprecated @babel/plugin-proposal-optional-chaining@7.21.0: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm WARN deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0

added 1702 packages, and audited 1704 packages in 21s

262 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

> cashtab@2.7.0 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

Module not found: Error: Can't resolve 'chronik-client' in '/work/cashtab/src'


Build cashtab-tests failed with exit code 1

Tail of the build log:

Installing mock-chronik-client dependencies...
/work/modules/mock-chronik-client /work/abc-ci-builds/cashtab-tests

added 236 packages, and audited 237 packages in 1s

35 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
/work/cashtab /work/modules/mock-chronik-client /work/abc-ci-builds/cashtab-tests
npm WARN deprecated @babel/plugin-proposal-private-methods@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-methods instead.
npm WARN deprecated @babel/plugin-proposal-numeric-separator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-numeric-separator instead.
npm WARN deprecated @babel/plugin-proposal-nullish-coalescing-operator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm WARN deprecated rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
npm WARN deprecated @babel/plugin-proposal-class-properties@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm WARN deprecated @babel/plugin-proposal-private-property-in-object@7.21.11: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-private-property-in-object instead.
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
npm WARN deprecated abab@2.0.6: Use your platform's native atob() and btoa() methods instead
npm WARN deprecated domexception@4.0.0: Use your platform's native DOMException instead
npm WARN deprecated stable@0.1.8: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm WARN deprecated @babel/plugin-proposal-optional-chaining@7.21.0: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm WARN deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0

added 1702 packages, and audited 1704 packages in 22s

262 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

> cashtab@2.7.0 build
> node scripts/build.js

Creating an optimized production build...
Failed to compile.

Module not found: Error: Can't resolve 'chronik-client' in '/work/cashtab/src'


Build cashtab-tests failed with exit code 1
bytesofman retitled this revision from [Cashtab] Custom non-antd notification to [Cashtab] Replace antd notifications and pop-up modals with react-toastify.

use pkg chronik-client, fix extension modal

bytesofman added inline comments.
cashtab/src/chronik/fixtures/mocks.js
7891 ↗(On Diff #46408)

we return a number instead of a string here because it allows us to improve formatting of notifications.

cashtab/src/components/AppModes/Extension.js
137 ↗(On Diff #46408)

big improvement aesthetically, also works

image.png (684×439 px, 43 KB)

cashtab/src/components/SignVerifyMsg/SignVerifyMsg.js
336 ↗(On Diff #46408)

we really never needed confirmation modals for sign / verify. just sign and verify when the user clicks sign or verify.

emack requested changes to this revision.Mar 24 2024, 02:05
emack added a subscriber: emack.

It's not immediately clear whether each notification is an error, success or warning message at a quick glance. I would suggest using either a green tick/red cross/yellow exclamation mark to accompany each notification (via the icon prop) or customize the background color depending on the nature of the message. White text on black bg can be interpreted in different ways.

image.png (112×356 px, 13 KB)

Edit: ok it looks like some (Adding to contacts) have the green tick, whereas others like sending token are using the eCash logo.

image.png (111×349 px, 10 KB)

image.png (85×330 px, 5 KB)

I know these are legacy choices prior to this diff but they should be made consistent as part of this diff.

Also black isn't part of the Cashtab color scheme so perhaps something that's already on the app?

image.png (95×244 px, 7 KB)

image.png (70×125 px, 360 B)

image.png (101×146 px, 2 KB)

image.png (87×119 px, 1 KB)

This revision now requires changes to proceed.Mar 24 2024, 02:05

use light mode on notifications for now, show success check on copy success, improve copytoclipboard toast handling

It's not immediately clear whether each notification is an error, success or warning message at a quick glance. I would suggest using either a green tick/red cross/yellow exclamation mark to accompany each notification (via the icon prop) or customize the background color depending on the nature of the message. White text on black bg can be interpreted in different ways.

Added the success check to the copy success ones.

Also black isn't part of the Cashtab color scheme so perhaps something that's already on the app?

Went with the light mode. imo the black looks a little better and it's ok for notifications to be "different" .... to a degree. But the dark mode notifications should wait until we have a more standardized dark mode. In general, I think Cashtab styles and colors could use some focused attention outside this diff. Light mode is ok with the theme and a big improvement on the antd notifications.

I'm okay keeping the not-standard-to-cashtab green for success and red for errors, as I think these have a more universal recognition. Might come back and swap the green success check mark later, but we already have "cashtab uses green checkmarks" right now with antd notifications.

better styling for incoming and outgoing tx notifications, use token icon for token txs (not genesis as it may not be available yet)

image.png (142×343 px, 16 KB)

image.png (142×343 px, 12 KB)

also updated the token icon to be the token icon

image.png (142×343 px, 14 KB)

image.png (142×343 px, 10 KB)

This revision is now accepted and ready to land.Mar 24 2024, 13:23