Page MenuHomePhabricator
Differential D10351

[Cashtab] updated 5 styled components
AbandonedPublic
Actions

Authored by kieran709 on Oct 14 2021, 16:57.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary

updated several styled components based on input from community member. Related to task: T1902

Test Plan

navigate to cash tab app
npm start
review style changes

Diff Detail

Repository
rABC Bitcoin ABC
Branch
styling-updates
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17097
Build 34026: Build Diffcashtab-tests
Build 34025: arc lint + arc unit

Event Timeline

kieran709 created this revision.Oct 14 2021, 16:57
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 14 2021, 16:57
kieran709 requested review of this revision.Oct 14 2021, 16:57
teamcity added a comment.Oct 14 2021, 17:00
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests:  Wallet without BCH balance ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet without BCH balance 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
        >
          ecash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7ccxkrr6zd
        </span>
      </div>
      <svg
-       className="sc-jzJRlG esZxys"
+       className="sc-jzJRlG eNqaIQ"
        height={210}
        id="borderedQRCode"
        shapeRendering="crispEdges"
        viewBox="0 0 37 37"
        width={210}
    at Object.<anonymous> (/work/web/cashtab/src/components/Wallet/__tests__/Wallet.test.js:36:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
====== CashTab Unit Tests:  Wallet with BCH balances ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
        >
          ecash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7ccxkrr6zd
        </span>
      </div>
      <svg
-       className="sc-jzJRlG esZxys"
+       className="sc-jzJRlG eNqaIQ"
        height={210}
        id="borderedQRCode"
        shapeRendering="crispEdges"
        viewBox="0 0 37 37"
        width={210}
    at Object.<anonymous> (/work/web/cashtab/src/components/Wallet/__tests__/Wallet.test.js:49:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
====== CashTab Unit Tests:  Wallet with BCH balances and tokens ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances and tokens 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
        >
          ecash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7ccxkrr6zd
        </span>
      </div>
      <svg
-       className="sc-jzJRlG esZxys"
+       className="sc-jzJRlG eNqaIQ"
        height={210}
        id="borderedQRCode"
        shapeRendering="crispEdges"
        viewBox="0 0 37 37"
        width={210}
    at Object.<anonymous> (/work/web/cashtab/src/components/Wallet/__tests__/Wallet.test.js:62:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
====== CashTab Unit Tests:  Wallet with BCH balances and tokens and state field ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances and tokens and state field 1`

- Snapshot  - 1
+ Received  + 1

@@ -43,11 +43,11 @@
        >
          ecash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7ccxkrr6zd
        </span>
      </div>
      <svg
-       className="sc-jzJRlG esZxys"
+       className="sc-jzJRlG eNqaIQ"
        height={210}
        id="borderedQRCode"
        shapeRendering="crispEdges"
        viewBox="0 0 37 37"
        width={210}
    at Object.<anonymous> (/work/web/cashtab/src/components/Wallet/__tests__/Wallet.test.js:75:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances and tokens and state field 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
      <a
        href="/send-token/bd1acc4c986de57af8d6d2a64aecad8c30ee80f37ae9d066d758923732ddc9ba"
        onClick={[Function]}
      >
        <div
-         className="sc-jKJlTe gnEvYb"
+         className="sc-jKJlTe fbOuLH"
        >
          <div
            className="sc-dxgOiQ ehOwnn"
          >
            <img
    at Object.<anonymous> (/work/web/cashtab/src/components/Tokens/__tests__/Tokens.test.js:96:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)

Each failure log is accessible here:
CashTab Unit Tests: Wallet without BCH balance
CashTab Unit Tests: Wallet with BCH balances
CashTab Unit Tests: Wallet with BCH balances and tokens
CashTab Unit Tests: Wallet with BCH balances and tokens and state field

Harbormaster completed remote builds in B17035: Diff 30557.Oct 14 2021, 17:00
bytesofman requested changes to this revision.Oct 14 2021, 17:45

Need to update snapshots if they changed. Here's the procedure:

npm test

[ tests will run and fail]
[ Menu will come up]

Press "u" to update snapshots

Run the tests again and confirm they all pass

Push up the amended diff.

This revision now requires changes to proceed.Oct 14 2021, 17:45
kieran709 updated this revision to Diff 30560.Oct 14 2021, 19:27

responding to review feedback

Harbormaster completed remote builds in B17037: Diff 30560.Oct 14 2021, 19:30
bytesofman requested changes to this revision.Oct 14 2021, 22:35

Looks good, some nits. In general I think the shadow effect is an improvement.

Please test the QR codes -- make sure you can still scan them with the Cashtab app on your phone. Sometimes small changes to the CSS stops them from rendering correctly...so even though everything "looks" right, the app would actually be non-functional.

web/cashtab/src/components/App.js
93 ↗(On Diff #30560)

Delete instead of commenting them out. I'll be able to see what was removed, and the previous code is still available in the history.

169 ↗(On Diff #30560)

Delete instead of commenting them out. I'll be able to see what was removed, and the previous code is still available in the history.

web/cashtab/src/components/Common/QRCode.js
17 ↗(On Diff #30560)

This value should be added to the props.theme.qr array; replacing what is already there under props.theme.qr.shadow if that is not used distinctly anywhere else.

20 ↗(On Diff #30560)

Delete instead of commenting them out. I'll be able to see what was removed, and the previous code is still available in the history.

web/cashtab/src/components/Tokens/__tests__/__snapshots__/Tokens.test.js.snap
1 ↗(On Diff #30560)

This is a weird one.

When you update snapshots, we don't want to see the files in diff review. You can prevent the file from being displayed in diff review by adding @generated to the comment at the top of the file.

Would be nice to automate this in the repo, but I keep kicking the can on that...just created a task for it.

web/cashtab/src/components/Wallet/TokenListItem.js
25 ↗(On Diff #30560)

This change should be in props.theme.tokenListItem.boxShadow , not hardcoded here

27 ↗(On Diff #30560)

Delete instead of commenting them out. I'll be able to see what was removed, and the previous code is still available in the history.

web/cashtab/src/components/Wallet/Tx.js
137 ↗(On Diff #30560)

As above -- change the object being referenced, not the CSS

web/cashtab/src/components/Wallet/__tests__/__snapshots__/Wallet.test.js.snap
1 ↗(On Diff #30560)

Add @generated like above

This revision now requires changes to proceed.Oct 14 2021, 22:35
kieran709 updated this revision to Diff 30583.Oct 15 2021, 17:59

responding to review feedback

kieran709 retitled this revision from updated 5 styled components to [Cashtab] updated 5 styled components.Oct 15 2021, 18:01
Harbormaster completed remote builds in B17052: Diff 30583.Oct 15 2021, 18:02
teamcity added a comment.Oct 15 2021, 18:02
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests:  Wallet with BCH balances and tokens and state field ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances and tokens and state field 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
      <a
        href="/send-token/bd1acc4c986de57af8d6d2a64aecad8c30ee80f37ae9d066d758923732ddc9ba"
        onClick={[Function]}
      >
        <div
-         className="sc-jKJlTe fbOuLH"
+         className="sc-jKJlTe XFMqt"
        >
          <div
            className="sc-dxgOiQ ehOwnn"
          >
            <img
    at Object.<anonymous> (/work/web/cashtab/src/components/Tokens/__tests__/Tokens.test.js:96:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances and tokens and state field 1`

- Snapshot  - 1
+ Received  + 1

@@ -43,11 +43,11 @@
        >
          ecash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7ccxkrr6zd
        </span>
      </div>
      <svg
-       className="sc-jzJRlG eNqaIQ"
+       className="sc-jzJRlG dYsuyj"
        height={210}
        id="borderedQRCode"
        shapeRendering="crispEdges"
        viewBox="0 0 37 37"
        width={210}
    at Object.<anonymous> (/work/web/cashtab/src/components/Wallet/__tests__/Wallet.test.js:75:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
====== CashTab Unit Tests:  Wallet without BCH balance ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet without BCH balance 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
        >
          ecash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7ccxkrr6zd
        </span>
      </div>
      <svg
-       className="sc-jzJRlG eNqaIQ"
+       className="sc-jzJRlG dYsuyj"
        height={210}
        id="borderedQRCode"
        shapeRendering="crispEdges"
        viewBox="0 0 37 37"
        width={210}
    at Object.<anonymous> (/work/web/cashtab/src/components/Wallet/__tests__/Wallet.test.js:36:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
====== CashTab Unit Tests:  Wallet with BCH balances ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
        >
          ecash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7ccxkrr6zd
        </span>
      </div>
      <svg
-       className="sc-jzJRlG eNqaIQ"
+       className="sc-jzJRlG dYsuyj"
        height={210}
        id="borderedQRCode"
        shapeRendering="crispEdges"
        viewBox="0 0 37 37"
        width={210}
    at Object.<anonymous> (/work/web/cashtab/src/components/Wallet/__tests__/Wallet.test.js:49:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
====== CashTab Unit Tests:  Wallet with BCH balances and tokens ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances and tokens 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
        >
          ecash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7ccxkrr6zd
        </span>
      </div>
      <svg
-       className="sc-jzJRlG eNqaIQ"
+       className="sc-jzJRlG dYsuyj"
        height={210}
        id="borderedQRCode"
        shapeRendering="crispEdges"
        viewBox="0 0 37 37"
        width={210}
    at Object.<anonymous> (/work/web/cashtab/src/components/Wallet/__tests__/Wallet.test.js:62:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)

Each failure log is accessible here:
CashTab Unit Tests: Wallet with BCH balances and tokens and state field
CashTab Unit Tests: Wallet without BCH balance
CashTab Unit Tests: Wallet with BCH balances
CashTab Unit Tests: Wallet with BCH balances and tokens

kieran709 updated this revision to Diff 30585.Oct 15 2021, 18:14

Update snapshots

Harbormaster completed remote builds in B17054: Diff 30585.Oct 15 2021, 18:17
bytesofman added a comment.Oct 15 2021, 21:10

Looking good. I fwded a few questions to the original designer on these. Personal quibbles:

  1. I don't really like how the footer is rounded at the bottom now, I think the old version is better
  2. I think some kind of hover effect should be preserved on the tx history and token tiles

I've fwded these concerns and will see what the designer says. For now, table this. I think we'll land it eventually but want to get those addressed first.

bytesofman requested changes to this revision.Oct 15 2021, 23:00

OK we got some more updates and hover effects, see telegram msg for animations and this link for the code.

https://wheat-letter-b76.notion.site/cashtab-css-161e38a8bddb43e5b28ec498f0d47b23

Please:

  1. Add the hover effects
  2. Revert the CSS changes to the footer that rounded the bottom left and right corners; keep its original shape

thanks

This revision now requires changes to proceed.Oct 15 2021, 23:00
kieran709 updated this revision to Diff 30598.Oct 18 2021, 17:50

responding to review feedback

Harbormaster completed remote builds in B17064: Diff 30598.Oct 18 2021, 17:53
teamcity added a comment.Oct 18 2021, 17:53
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests:  Wallet with BCH balances and tokens and state field ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Wallet with BCH balances and tokens and state field 1`

- Snapshot  - 1
+ Received  + 1

@@ -61,11 +61,11 @@
      <a
        href="/send-token/bd1acc4c986de57af8d6d2a64aecad8c30ee80f37ae9d066d758923732ddc9ba"
        onClick={[Function]}
      >
        <div
-         className="sc-jKJlTe XFMqt"
+         className="sc-jKJlTe beEKJc"
        >
          <div
            className="sc-dxgOiQ ehOwnn"
          >
            <img
    at Object.<anonymous> (/work/web/cashtab/src/components/Tokens/__tests__/Tokens.test.js:96:18)
    at Object.asyncJestTest (/work/web/cashtab/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:106:37)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:45:12
    at new Promise (<anonymous>)
    at mapper (/work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:28:19)
    at /work/web/cashtab/node_modules/jest-jasmine2/build/queueRunner.js:75:41
    at processTicksAndRejections (node:internal/process/task_queues:94:5)

Each failure log is accessible here:
CashTab Unit Tests: Wallet with BCH balances and tokens and state field

kieran709 updated this revision to Diff 30599.Oct 18 2021, 18:00

update snapshots

Harbormaster completed remote builds in B17065: Diff 30599.Oct 18 2021, 18:03
bytesofman requested changes to this revision.Oct 18 2021, 20:24

image.png (102×525 px, 11 KB)

I think this is good to go except I don't like the bottom left and bottom right corners of the menu -- I think rounding should be preserved on top left and top right corners but not bottom left and bottom right.

Please adjust, thanks!

This revision now requires changes to proceed.Oct 18 2021, 20:24
kieran709 updated this revision to Diff 30614.Oct 20 2021, 18:50

responding to review feedback

Harbormaster completed remote builds in B17076: Diff 30614.Oct 20 2021, 18:53
kieran709 updated this revision to Diff 30637.Oct 21 2021, 22:00

responding to review feedback

Harbormaster completed remote builds in B17097: Diff 30637.Oct 21 2021, 22:04
kieran709 abandoned this revision.Oct 22 2021, 14:47

Abandonning due to error in rebasing.

Revision Contents
Changeset List

PathSize
web/
cashtab/
src/
assets/
styles/
11 lines
components/
6 lines
Common/
3 lines
Tokens/
__tests__/
__snapshots__/
Wallet/
10 lines
10 lines
__tests__/
__snapshots__/

Diff 30637

View Options

web/cashtab/src/assets/styles/theme.js

Loading...
View Options

web/cashtab/src/components/App.js

Loading...
View Options

web/cashtab/src/components/Common/QRCode.js

Loading...
View Options

web/cashtab/src/components/Tokens/__tests__/__snapshots__/Tokens.test.js.snap

Loading...
View Options

web/cashtab/src/components/Wallet/TokenListItem.js

Loading...
View Options

web/cashtab/src/components/Wallet/Tx.js

Loading...
View Options

web/cashtab/src/components/Wallet/__tests__/__snapshots__/Wallet.test.js.snap

Loading...