feat: Implement ConnectedAccountsMenu buttons by jonybur · Pull Request #23258 · MetaMask/metamask-extension

jonybur · 2024-03-01T04:00:06Z

Description

Implements the 'Switch to this account' and the 'Disconnect' buttons on the ConnectedAccountsMenu panel.

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2172

Manual testing steps

Connect two accounts to a dApp
Open the Connected Accounts menu panel
Switch between the accounts
Disconnect an account

Screenshots/Recordings

vid1.mp4

switch2.mp4

Before

After

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.

… fix-2172

github-actions · 2024-03-01T04:00:19Z

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.

… fix-2172

…o fix-2172

NidhiKJha

Great work @jonybur Left some suggestions but overall looks great!
Can we add a condition to not show switch to this account option when there is only one account connected?

NidhiKJha · 2024-03-04T13:45:36Z

ui/components/multichain/connected-accounts-menu/index.scss

@@ -0,0 +1,8 @@
+@use "design-system";


Do we need this file?

My understanding is that it is needed for this line:
z-index: design-system.$popover-in-modal-z-index;

ui/components/multichain/connected-accounts-menu/connected-accounts-menu.tsx

…ounts-menu.tsx Co-authored-by: Nidhi Kumari <nidhi.kumari@consensys.net>

…o fix-2172

NidhiKJha

Almost there! Just some small nits

ui/components/multichain/connected-accounts-menu/connected-accounts-menu.test.tsx

NidhiKJha · 2024-03-05T12:24:27Z

ui/components/multichain/connected-accounts-menu/connected-accounts-menu.tsx

  isOpen,
+  identity,
  anchorElement,
+  disableAccountSwitcher,


By default this should be false?

NidhiKJha · 2024-03-05T12:25:00Z

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

+          identity={identity}
+          onClose={() => setAccountOptionsMenuOpen(false)}
+          closeMenu={closeMenu}
+          disableAccountSwitcher={accountsCount === 1}


Hmmm...Do you think we should create a separate variable for this?

…o fix-2172

metamaskbot · 2024-03-05T13:19:19Z

Builds ready [d5e445c]

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, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1245 ± 364 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	85	239	133	38	18
		domContentLoaded	10	83	39	24	11
		load	71	1891	1245	759	364
		domInteractive	10	83	39	24	11

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 944 Bytes (0.01%)
common: 0 Bytes (0.00%)

codecov · 2024-03-05T13:20:00Z

Codecov Report

Attention: Patch coverage is 44.44444% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 68.68%. Comparing base (c249bd8) to head (903f702).
Report is 1 commits behind head on develop.

Files	Patch %	Lines
...onnected-accounts-menu/connected-accounts-menu.tsx	42.86%	12 Missing ⚠️
.../multichain/account-list-item/account-list-item.js	50.00%	2 Missing ⚠️
...nents/multichain/pages/connections/connections.tsx	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #23258      +/-   ##
===========================================
- Coverage    68.70%   68.68%   -0.01%     
===========================================
  Files         1102     1102              
  Lines        43221    43246      +25     
  Branches     11558    11566       +8     
===========================================
+ Hits         29691    29702      +11     
- Misses       13530    13544      +14

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

NidhiKJha

LGTM

metamaskbot · 2024-03-06T12:22:35Z

Builds ready [903f702]

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, 5
- common: 0, 1, 2, 3, 4, 5, 6
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1069 ± 474 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	99	198	136	26	12
		domContentLoaded	8	74	33	17	8
		load	89	2485	1069	987	474
		domInteractive	8	74	33	17	8

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 1.13 KiB (0.02%)
common: 0 Bytes (0.00%)

NidhiKJha and others added 9 commits February 28, 2024 23:38

added menu types to render in account list item

313024d

Merge branch 'develop' into fix-account-menu

afafe79

Merge branch 'develop' into fix-account-menu

355e5fa

Merge branch 'develop' into fix-account-menu

ab3ca1b

move account menu types to types file

b1201f5

Merge branch 'develop' into fix-account-menu

0b9b7d5

updated types to use none

50e9cfc

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

33e736a

… fix-2172

Implement account switcher

0334521

jonybur requested review from a team, brad-decker and kumavis as code owners March 1, 2024 04:00

jonybur added the team-core-extension-ux Core Extension UX team label Mar 1, 2024

jonybur marked this pull request as draft March 1, 2024 04:00

Implement disconnect

6ec721f

Base automatically changed from fix-account-menu to develop March 4, 2024 08:10

Jonathan Bursztyn and others added 4 commits March 4, 2024 18:12

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

d275fef

… fix-2172

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

ceed8d2

… fix-2172

Add Connection menu to AccountListItem

9c308dc

Merge branch 'develop' into fix-2172

b01d9a4

jonybur marked this pull request as ready for review March 4, 2024 12:51

Jonathan Bursztyn added 3 commits March 4, 2024 22:02

Add Identity type

cbf600e

Merge branch 'fix-2172' of github.com:MetaMask/metamask-extension int…

53c7fe0

…o fix-2172

Readd AccountListMenu export

80a59bf

jonybur added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Mar 4, 2024

NidhiKJha requested changes Mar 4, 2024

View reviewed changes

NidhiKJha removed the request for review from brad-decker March 4, 2024 13:52

Update ui/components/multichain/connected-accounts-menu/connected-acc…

0fdfad6

…ounts-menu.tsx Co-authored-by: Nidhi Kumari <nidhi.kumari@consensys.net>

jonybur requested a review from NidhiKJha March 4, 2024 14:08

Jonathan Bursztyn and others added 7 commits March 4, 2024 23:49

Separate types

4cfb27f

Merge branch 'fix-2172' of github.com:MetaMask/metamask-extension int…

29cd459

…o fix-2172

Add Identity type

b9e7da6

Remove MetaMetrics

d6d4e31

Dont show account switcher when user has one account

b4b318c

Add test

a56647c

Merge branch 'develop' into fix-2172

db6bdd3

NidhiKJha requested changes Mar 5, 2024

View reviewed changes

Jonathan Bursztyn added 4 commits March 5, 2024 21:39

Default disableAccountSwitcher to false

ce10113

Merge branch 'fix-2172' of github.com:MetaMask/metamask-extension int…

a548685

…o fix-2172

Rename variable

b42d556

Remove unused attributes from Identity

d5e445c

NidhiKJha previously approved these changes Mar 5, 2024

View reviewed changes

darkwing previously approved these changes Mar 5, 2024

View reviewed changes

Fix multiple menus being open

fa44bb3

jonybur dismissed stale reviews from darkwing and NidhiKJha via fa44bb3 March 5, 2024 16:07

Merge branch 'develop' into fix-2172

903f702

darkwing approved these changes Mar 7, 2024

View reviewed changes

DDDDDanica approved these changes Mar 7, 2024

View reviewed changes

jonybur merged commit b110819 into develop Mar 7, 2024

jonybur deleted the fix-2172 branch March 7, 2024 13:10

github-actions bot locked and limited conversation to collaborators Mar 7, 2024

metamaskbot added the release-11.14.0 Issue or pull request that will be included in release 11.14.0 label Mar 7, 2024

Conversation

jonybur commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Mar 1, 2024

Uh oh!

NidhiKJha left a comment

Choose a reason for hiding this comment

Uh oh!

NidhiKJha Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

jonybur Mar 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NidhiKJha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

NidhiKJha Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

NidhiKJha Mar 5, 2024

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Mar 5, 2024

Uh oh!

codecov bot commented Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NidhiKJha left a comment

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented Mar 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jonybur commented Mar 1, 2024 •

edited

Loading

codecov bot commented Mar 5, 2024 •

edited

Loading