Page MenuHomePhabricator

[cashtab] UI Overhaul
ClosedPublic

Authored by johnkuney on Fri, Nov 22, 16:29.

Details

Reviewers
bytesofman
emack
Group Reviewers
Restricted Project
Commits
rABC8eb013ff29be: [cashtab] UI Overhaul
Summary

As discussed here are some proposed UI changes to the app as a whole. It is maybe more changes than the
typical diff, but it kind of needs to be done in one go. It is all mostly CSS changes in any case.

A new desktop layout and color changes are the main updates here, with more granual design improvements to individual pages to come

Test Plan

preview the site and check it looks good on mobile and desktop

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cashtab-ui-overhaul
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 31351
Build 62200: Build Diffcashtab-tests
Build 62199: arc lint + arc unit

Event Timeline

Tail of the build log:

Run `npm audit` for details.

> ecash-lib@0.2.1 build
> tsc && tsc -p ./tsconfig.build.json && cp -r ./src/ffi ./dist

Installing ecash-agora dependencies...
/work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests

added 364 packages, and audited 367 packages in 1s

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

2 vulnerabilities (1 moderate, 1 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

> ecash-agora@0.1.1 build
> tsc && tsc -p ./tsconfig.build.json

/work/cashtab /work/modules/ecash-agora /work/modules/ecash-lib /work/modules/ecash-lib-wasm /work/modules/ecash-script /work/modules/chronik-client /work/modules/mock-chronik-client /work/modules/ecashaddrjs /work/abc-ci-builds/cashtab-tests
npm warn deprecated @humanwhocodes/config-array@0.11.14: Use @eslint/config-array instead
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated eslint@8.56.0: This version is no longer supported. Please see https://eslint.org/version-support for other options.

added 1483 packages, and audited 3329 packages in 24s

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

4 vulnerabilities (2 moderate, 2 high)

To address all issues, run:
  npm audit fix

Run `npm audit` for details.

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

Creating an optimized production build...

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

Failed to compile.

[eslint] 
src/components/Common/CustomIcons.js
  Line 139:7:  'Rotate' is assigned a value but never used  @typescript-eslint/no-unused-vars

src/components/Configure/Configure.js
  Line 24:10:  'AlertMsg' is defined but never used  @typescript-eslint/no-unused-vars


Build cashtab-tests failed with exit code 1

Failed tests logs:

====== CashTab Unit Tests: <App /> Navigation menu routes to expected components ======
Error: expect(element).toHaveStyle()

- Expected

- max-width: 0;
    at Object.toHaveStyle (/work/cashtab/src/components/App/__tests__/App.test.js:262:54)

Each failure log is accessible here:
CashTab Unit Tests: <App /> Navigation menu routes to expected components

another thing to check is npm run extension and extension still has pop-out link in its header

cashtab/src/components/Agora/OrderBook/styled.ts
119 ↗(On Diff #51053)

if it's worth specifying, should be in the theme

after this, we should also overhaul the theme so it's "easy" to create multiple themes.

141 ↗(On Diff #51053)

unrelated issue but any commented out css like this should be deleted

Failed tests logs:

====== CashTab Unit Tests: <App /> Navigation menu routes to expected components ======
Error: expect(element).toHaveStyle()

- Expected

- max-width: 0;
    at Object.toHaveStyle (/work/cashtab/src/components/App/__tests__/App.test.js:269:54)

Each failure log is accessible here:
CashTab Unit Tests: <App /> Navigation menu routes to expected components

bytesofman added inline comments.
cashtab/src/components/App/__tests__/App.test.js
219 ↗(On Diff #51055)

If you want to test that nav routing works for both mobile and desktop, you need two tests

1 with global.innerWidth for desktop and test checks for expected desktop css
1 with global.innerWidth for mobile and test checks for expected mobile css

264 ↗(On Diff #51055)

this is a constant because innerWidth is a constant in this test

use 2 tests and check against both consts.

This revision now requires changes to proceed.Fri, Nov 22, 17:58

nice!

much, much better UI

Some ticky-tack stuff. could be handled in separate diffs but imo might as well get it here

  1. Receive screen floating space on mobile ... mb ok to just keep it there, not sure how to best resolve (light grey space between qr code and menu)

image.png (744×335 px, 29 KB)

  1. On mobile, the menu start height seems to be higher than what it slides in as.
  1. good opportunity for black (purple? otherwise restyled to match?) favicon
  1. centering of Dragger on create token screen

image.png (693×1 px, 36 KB)

  1. NFTs and agora, can we modify the flex so we get two columns on large enough widths?

image.png (693×1 px, 101 KB)

  1. switch should be centered on wallet backup

image.png (693×1 px, 28 KB)

  1. Better use of space on Etokens

image.png (693×1 px, 150 KB)

mb search bar is left, tx list is right? or 2 columns for txs if enough width?

for the "better flex space" use points...mb better to handle in its own diff, up to you.

cashtab/src/components/App/App.js
110–138 ↗(On Diff #51058)

is any of this still used?

now that we aren't using the minified header -- this logic takes a lot of memory. can we get rid of this?

cashtab/src/components/App/__tests__/App.test.js
195 ↗(On Diff #51058)

back out unrelated change

cashtab/src/wallet/index.ts
85 ↗(On Diff #51058)

remove and update chronik-client as discussed

This revision now requires changes to proceed.Fri, Nov 22, 19:01
  1. Background and highlight colors of select menu dropdowns should be updated (see wallet select dropdown, currency select dropdowns)

left border missing? also highlight/bg colors of dropdown

image.png (209×363 px, 11 KB)

image.png (46×163 px, 3 KB)

the logo just seems kinda low res

could update it in this diff. Up to you on font, appearance...feels like it should change with this overhaul

nice lookin great

image.png (676×1 px, 284 KB)

  • add some horizontal spacing between the tiles

image.png (676×1 px, 143 KB)

  • mb not practical but would be nice if, on opening, the open tile gets width 100% ... or otherwise other stuff expands

agora looks sick on full screen desktop.

This revision now requires changes to proceed.Tue, Nov 26, 05:20
emack requested changes to this revision.Tue, Nov 26, 10:31
emack added a subscriber: emack.
emack added inline comments.
cashtab/src/components/Swap/Swap.js
24–26 ↗(On Diff #51102)

Not sure why but in this diff the sideshift widget is not loading. You can test in prod and it opens up all ok. Something about this <PageHeader> that's conflicting with the widget?

image.png (415×718 px, 13 KB)

cashtab/src/components/Swap/Swap.js
24–26 ↗(On Diff #51102)

I'm seeing the same thing at cashtab.com right now, seems like an unrelated issue

version patch and twitter card

nice changes look great. prob ready to land.

what about the logo above the menu? assets/cashtab_xec.png?

also looks kinda dated.

mb we should have an ecash "e" in there as well. say, to the left or right of Cashtab? whatever ... pops, as they say. Prob same for twitter.

This revision now requires changes to proceed.Tue, Nov 26, 20:35

For sure, what do you think about this one?
I think putting the eCash logo in there doesnt really work as they compete with eachother visually.
It kind of needs to be its own logo. But I did try making the cashtab logo reminiscent of the eCash logo mark to tie it in...

This revision is now accepted and ready to land.Wed, Nov 27, 10:13
This revision was landed with ongoing or failed builds.Wed, Nov 27, 19:27
This revision was automatically updated to reflect the committed changes.