UX: Multichain: Add Account or Hardware Wallet button #21081

darkwing · 2023-09-27T17:36:46Z

Description

Replaces the "Add / Import / Hardware" links with a single button that takes the user to another quick modal to add, import, or connect a hardware wallet.

Manual testing steps

_1. Open the account menu
_2. Click the "Add account or hardware wallet" button
_3. Click each option, ensure the correct modal displays
_4. Click "back" on each screen, ensure it goes back logically in the nested cycle

Screenshots/Recordings

Screen.Recording.2023-09-27.at.12.24.02.PM.mov

Related issues

_Fixes https://github.com/MetaMask/MetaMask-planning/issues/1374

Pre-merge author checklist

Pre-merge reviewer checklist

I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

github-actions · 2023-09-27T17:36:59Z

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

NidhiKJha

Looking really good. Left some comments. Question: I think we should create a separate component for the New modal with add, import and hardware wallet buttons as we are gonna need this modal in connetions flow as well https://www.figma.com/file/LDlI7yjKBNIY60MtQv9QS0/Amon-Hen%2FV1?type=design&node-id=4878-849206&mode=dev. wdyt?

ui/components/multichain/account-list-menu/account-list-menu.js

…x tests

metamaskbot · 2023-09-28T20:50:45Z

Builds ready [89f31b0]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (950 ± 364 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	83	190	116	26	12
		domContentLoaded	71	228	109	32	16
		load	82	1845	950	758	364
		domInteractive	71	228	109	32	16

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 467 Bytes (0.01%)
common: 104 Bytes (0.00%)

codecov · 2023-09-28T20:51:08Z

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (05241ee) 68.49% compared to head (89f31b0) 68.50%.
Report is 9 commits behind head on develop.

❗ Current head 89f31b0 differs from pull request most recent head 9d3f911. Consider uploading reports for the commit 9d3f911 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #21081      +/-   ##
===========================================
+ Coverage    68.49%   68.50%   +0.01%     
===========================================
  Files         1010     1010              
  Lines        40398    40406       +8     
  Branches     10788    10792       +4     
===========================================
+ Hits         27669    27677       +8     
  Misses       12729    12729

Files	Coverage Δ
.../multichain/account-list-menu/account-list-menu.js	`80.22% <75.68%> (+1.91%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

NidhiKJha

LGTM!

ui/components/multichain/account-list-menu/account-list-menu.js

metamaskbot · 2023-09-29T20:31:26Z

Builds ready [16c317e]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5, 6, 7
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 12, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (755 ± 353 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	83	132	99	15	7
		domContentLoaded	69	113	88	14	7
		load	80	1768	755	734	353
		domInteractive	69	113	88	14	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 0 Bytes (0.00%)
ui: 437 Bytes (0.01%)
common: 104 Bytes (0.00%)

georgewrmarshall · 2023-10-02T08:35:55Z

app/_locales/en/messages.json

@@ -246,6 +246,9 @@
  "addIPFSGateway": {
    "message": "Add your preferred IPFS gateway"
  },
+  "addImportAccount": {
+    "message": "Add account or hardware wallet"


This seems like a very long string to have inside of a button just want to make sure that it's been signed off by content cc @coreyjanssen

I did sign off on the text but am now doubting it! It seemed like the best option to encompass what lies behind the CTA but perhaps Add account or wallet would suffice? Wdyt?

NidhiKJha

LGTM

coreyjanssen · 2023-10-03T00:19:58Z

What exactly is a custodial account? If this is something we're using in lieu of Account Snaps/Snap account, I don't know that it's the best option. Additionally, it doesn't follow our standards for sentence case and should probably have a relevant icon as with the rest of the menu.

darkwing requested a review from a team as a code owner September 27, 2023 17:36

darkwing added team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead needs-ux-ds-review labels Sep 27, 2023

NidhiKJha requested changes Sep 28, 2023

View reviewed changes

ui/components/multichain/account-list-menu/account-list-menu.js Outdated Show resolved Hide resolved

ui/components/multichain/account-list-menu/account-list-menu.js Outdated Show resolved Hide resolved

darkwing force-pushed the multichain-account-menu-button branch 3 times, most recently from 9e10574 to 38800ad Compare September 28, 2023 19:24

darkwing added 7 commits September 28, 2023 14:33

UX: Multichain: Add Account or Hardware Wallet button

3f72fe3

Implement separate screen action mode for add / import / hardware, fi…

88e4612

…x tests

Use SecondaryButton

97e61dc

lint

9ccdcc2

Use locale for button, fix formatting

f9b5b49

Simplify title code

1865d77

E2E Updates

89f31b0

darkwing force-pushed the multichain-account-menu-button branch from 38800ad to 89f31b0 Compare September 28, 2023 19:33

darkwing requested a review from NidhiKJha September 29, 2023 01:46

NidhiKJha previously approved these changes Sep 29, 2023

View reviewed changes

DDDDDanica reviewed Sep 29, 2023

View reviewed changes

ui/components/multichain/account-list-menu/account-list-menu.js Outdated Show resolved Hide resolved

Add action mode constants

9d3f911

darkwing dismissed NidhiKJha’s stale review via 9d3f911 September 29, 2023 16:45

Merge branch 'develop' into multichain-account-menu-button

16c317e

georgewrmarshall reviewed Oct 2, 2023

View reviewed changes

NidhiKJha approved these changes Oct 2, 2023

View reviewed changes

vthomas13 approved these changes Oct 2, 2023

View reviewed changes

darkwing merged commit 4468168 into develop Oct 2, 2023
9 checks passed

darkwing deleted the multichain-account-menu-button branch October 2, 2023 15:42

github-actions bot locked and limited conversation to collaborators Oct 2, 2023

metamaskbot added the release-11.3.0 Issue or pull request that will be included in release 11.3.0 label Oct 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UX: Multichain: Add Account or Hardware Wallet button #21081

UX: Multichain: Add Account or Hardware Wallet button #21081

darkwing commented Sep 27, 2023

github-actions bot commented Sep 27, 2023

NidhiKJha left a comment

metamaskbot commented Sep 28, 2023

codecov bot commented Sep 28, 2023 •

edited

Loading

NidhiKJha left a comment

metamaskbot commented Sep 29, 2023

georgewrmarshall Oct 2, 2023

coreyjanssen Oct 3, 2023

NidhiKJha left a comment

coreyjanssen commented Oct 3, 2023

UX: Multichain: Add Account or Hardware Wallet button #21081

UX: Multichain: Add Account or Hardware Wallet button #21081

Conversation

darkwing commented Sep 27, 2023

Description

Manual testing steps

Screenshots/Recordings

Related issues

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Sep 27, 2023

NidhiKJha left a comment

Choose a reason for hiding this comment

metamaskbot commented Sep 28, 2023

codecov bot commented Sep 28, 2023 • edited Loading

Codecov Report

NidhiKJha left a comment

Choose a reason for hiding this comment

metamaskbot commented Sep 29, 2023

georgewrmarshall Oct 2, 2023

Choose a reason for hiding this comment

coreyjanssen Oct 3, 2023

Choose a reason for hiding this comment

NidhiKJha left a comment

Choose a reason for hiding this comment

coreyjanssen commented Oct 3, 2023

codecov bot commented Sep 28, 2023 •

edited

Loading