SavedWallet balance display + enhancements
AbandonedPublic
Actions

Authored by emack on Oct 1 2021, 03:43.

Details

Reviewers

bytesofman
Fabien

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Summary

Created new function 'formatSavedBalance' in utils/validation.js to take a savedwallet balance as input and return a formated locale string so large XEC balances are legible
Imported the newly created locale string function into Configure/Configure.js and utilised as part of conditional rendering to mitigate edge cases that may prevent the app from rendering.
Added unit tests in validation.test.js to test edge case inputs for SWBalances, including undefined/null balances, zero and max supply trillion balances
Fixed overflow issue with long wallet names pushing buttons off the UI by adjusting overflow:hover css for both <SWName> and <SWBalance>
Added additional styling in Configuration.js's <SWBalance> component to enhance aesthetics and distinguish SWBalance values from SWName

Test Plan

npm test passed successfully, which includes unit tests for the newly created formatSavedBalance functions relating to edge cases
Correctly displaying a small XEC balance for saved wallets
Correctly displaying 0 XEC balance for saved wallets
Correctly displaying a large XEC balance for saved wallets
Correctly displaying N/A balance for a newly imported saved wallet (this will test undefined or null inputs from 'sw.state.balances' and 'sw.state.balances.totalBalance')
Correctly displaying 0 balance when viewing a recently imported saved wallet a second time in the Saved Wallets list
Correctly displaying a wallet with 24 characters where it's initially shortened ellipsis style and hovering over it will display the full value
Repeat steps 2-6 for browser extension mode
Test cross browser compatibility in Firefox in standard web mode
Cosmetic check by manually reducing browser width and ensure UI components are positioned accordingly

Diff Detail

Repository

rABC Bitcoin ABC

Branch

local-cashtab-branch

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 16866
Build 33571: Build Diff	cashtab-tests
Build 33570: arc lint + arc unit

Event Timeline

emack created this revision.Oct 1 2021, 03:43

Owners added a reviewer: Restricted Owners Package.Oct 1 2021, 03:43

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 1 2021, 03:43

emack requested review of this revision.Oct 1 2021, 03:43

Harbormaster completed remote builds in B16849: Diff 30261.Oct 1 2021, 03:45

Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests:  Configure without a wallet ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Configure without a wallet 1`

- Snapshot  - 4
+ Received  + 4

@@ -1,7 +1,7 @@
  <div
-   className="sc-kEYyzF jqzglm"
+   className="sc-kkGfuU liSflp"
  >
    <h2>
      <span
        aria-label="copy"
        className="anticon anticon-copy sc-bdVaJa loPqmo"
@@ -67,11 +67,11 @@
          Your seed phrase is the only way to restore your wallet. Write it down. Keep it safe.
        </div>
      </div>
    </div>
    <div
-     className="sc-kkGfuU lbBTMM"
+     className="sc-iAyFgw AbOBK"
    />
    <h2>
      <span
        aria-label="wallet"
        className="anticon anticon-wallet sc-htpNat fLtATP"
@@ -145,11 +145,11 @@
        </svg>
      </span>
       Import Wallet
    </button>
    <div
-     className="sc-kkGfuU lbBTMM"
+     className="sc-iAyFgw AbOBK"
    />
    <h2>
      <span
        aria-label="dollar"
        className="anticon anticon-dollar sc-bwzfXH iTQEyo"
@@ -262,11 +262,11 @@
          </span>
        </span>
      </div>
    </div>
    <div
-     className="sc-kkGfuU lbBTMM"
+     className="sc-iAyFgw AbOBK"
    />
    [
    <a
      className="sc-dxgOiQ jbuVAx"
      href="https://docs.cashtabapp.com/docs/"
    at Object.<anonymous> (/work/web/cashtab/src/components/Configure/__tests__/Configure.test.js:24: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:  Configure with a wallet ======
Error: expect(received).toMatchSnapshot()

Snapshot name: `Configure with a wallet 1`

- Snapshot  - 4
+ Received  + 4

@@ -1,7 +1,7 @@
  <div
-   className="sc-kEYyzF jqzglm"
+   className="sc-kkGfuU liSflp"
  >
    <h2>
      <span
        aria-label="copy"
        className="anticon anticon-copy sc-bdVaJa loPqmo"
@@ -105,11 +105,11 @@
          Click to reveal seed phrase
        </div>
      </div>
    </div>
    <div
-     className="sc-kkGfuU lbBTMM"
+     className="sc-iAyFgw AbOBK"
    />
    <h2>
      <span
        aria-label="wallet"
        className="anticon anticon-wallet sc-htpNat fLtATP"
@@ -183,11 +183,11 @@
        </svg>
      </span>
       Import Wallet
    </button>
    <div
-     className="sc-kkGfuU lbBTMM"
+     className="sc-iAyFgw AbOBK"
    />
    <h2>
      <span
        aria-label="dollar"
        className="anticon anticon-dollar sc-bwzfXH iTQEyo"
@@ -300,11 +300,11 @@
          </span>
        </span>
      </div>
    </div>
    <div
-     className="sc-kkGfuU lbBTMM"
+     className="sc-iAyFgw AbOBK"
    />
    [
    <a
      className="sc-dxgOiQ jbuVAx"
      href="https://docs.cashtabapp.com/docs/"
    at Object.<anonymous> (/work/web/cashtab/src/components/Configure/__tests__/Configure.test.js:35: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: Configure without a wallet
CashTab Unit Tests: Configure with a wallet

Thanks for the patch.

There is a broken test from your changes that needs to be fixed. Also is it possible to add a test for the new feature (balances printed as locale strings) ?

This revision now requires changes to proceed.Oct 1 2021, 08:24

Good progress on this. A few items required before this lands:

Overflow issue on how balances are displayed -- wallets with long names can push buttons and names off the interface. See below:

Please create a separate function to format the displayed balance as a locale string. This will help to handle edge cases that could prevent the app from rendering. Input of the function should be sw.state.balances.totalBalance and output should be the localeString of this value. Function should handle undefined or null values -- any error from functionInput.toLocaleString() and the function should return an appropriate marker denoting "balance not available." Balance N/A would be fine.

Please add unit tests for this function. Please confirm npm test is all clear before pushing up an amend to this diff.

General aesthetic improvement; could use some more styling in the new <SWBalance> component, mb a smaller font size or some other distinction from the wallet name column.

Let me know any questions, thanks again!

Summary:
Incorporates review feedback from Fabien and BytesOfMan from 2021-10-01

Created new function 'formatSavedBalance' in utils/validation.js to take a savedwallet balance as input and return a formated locale string so large XEC balances are legible
Imported the newly created locale string function into Configure/Configure.js and utilised as part of conditional rendering to mitigate edge cases that may prevent the app from rendering.
Added unit tests in validation.test.js to test edge case inputs for SWBalances, including undefined/null balances, zero and max supply trillion balances
Fixed overflow issue with long wallet names pushing buttons off the UI by adjusting overflow:hover css for both <SWName> and <SWBalance>
Added additional styling in Configuration.js's <SWBalance> component to enhance aesthetics and distinguish SWBalance values from SWName
Updated snapshot to fix earlier unit test errors

Test Plan:

npm test passed successfully, which includes unit tests for the newly created formatSavedBalance functions relating to edge cases
Correctly displaying a small XEC balance for saved wallets
Correctly displaying 0 XEC balance for saved wallets
Correctly displaying a large XEC balance for saved wallets
Correctly displaying N/A balance for a newly imported saved wallet (this will test undefined or null inputs from 'sw.state.balances' and 'sw.state.balances.totalBalance')
Correctly displaying 0 balance when viewing a recently imported saved wallet a second time in the Saved Wallets list
Correctly displaying a wallet name with 24 characters where it's initially shortened ellipsis style and hovering above it will display the full value
Repeat steps 2-6 for browser extension mode
Test cross browser compatibility in Firefox in standard web mode
Cosmetic check by manually reducing browser width and ensure UI components are positioned accordingly

Harbormaster completed remote builds in B16866: Diff 30283.Oct 2 2021, 05:56

emack edited the summary of this revision. (Show Details)Oct 2 2021, 06:01

emack edited the test plan for this revision. (Show Details)

emack edited the test plan for this revision. (Show Details)Oct 2 2021, 06:13

Hey looking good from the screenshots! Some suggested improvements in line by line comments.

web/cashtab/package.json
160	Why was jest downgraded? If this is not specifically necessary to get the diff to work, the dependency tree should remain unmodified.
web/cashtab/src/utils/validation.js
88	More robust to use a `try...catch` loop here. This way, if the function throws any kind of error from `Number(swBalance).toLocaleString()`, it will be caught and returned as `N/A`. (compare to approach here, where `N/A` is only returned in specific case of undefined or null, and any error from `.toLocaleString` is not handled.