metamask-extension icon indicating copy to clipboard operation
metamask-extension copied to clipboard
verified publisher icon MetaMask

UX MULTICHAIN: added connect account in account list menu item

Open NidhiKJha opened this issue 1 year ago • 8 comments

This PR is to add the connect account button in account list menu item

Related issues

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

Manual testing steps

  1. Go to account list menu
  2. Click on connect account, if account is not connected it should connect the account

Screenshots/Recordings

Before

Screenshot 2024-02-14 at 3 35 44 PM

After

Screenshot 2024-02-14 at 8 04 17 PM

Pre-merge author checklist

  • [ ] I’ve followed MetaMask Coding Standards.
  • [ ] I've clearly explained what problem this PR is solving and how it is solved.
  • [ ] I've linked related issues
  • [ ] I've included manual testing steps
  • [ ] I've included screenshots/recordings if applicable
  • [ ] I’ve included tests if applicable
  • [ ] I’ve documented my code using JSDoc format if applicable
  • [ ] I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • [ ] I’ve properly set the pull request status:
    • [ ] In case it's not yet "ready for review", I've set it to "draft".
    • [ ] In case it's "ready for review", I've changed it from "draft" to "non-draft".

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.

NidhiKJha avatar Feb 14 '24 10:02 NidhiKJha

Builds ready [b593602]
Page Load Metrics (1082 ± 61 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1322181812914
domContentLoaded1396432713
load9191506108212661
domInteractive1396432713
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 160 Bytes (0.00%)
  • common: 79 Bytes (0.00%)

metamaskbot avatar Feb 14 '24 10:02 metamaskbot

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (d3f0a32) 68.46% compared to head (653cfd2) 68.49%. Report is 22 commits behind head on develop.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #22941      +/-   ##
===========================================
+ Coverage    68.46%   68.49%   +0.03%     
===========================================
  Files         1089     1089              
  Lines        43045    43058      +13     
  Branches     11469    11474       +5     
===========================================
+ Hits         29469    29491      +22     
+ Misses       13576    13567       -9

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 14 '24 10:02 codecov[bot]

Builds ready [53a2f37]
Page Load Metrics (939 ± 23 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1302061752412
domContentLoaded972392512
load81810229394823
domInteractive972392512
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 160 Bytes (0.00%)
  • common: 79 Bytes (0.00%)

metamaskbot avatar Feb 14 '24 14:02 metamaskbot

Seeing this in ui/pages/connected-accounts/connected-accounts.container.js:

connectAccount: (address) =>
      dispatchProps.addPermittedAccount(activeTabOrigin, address),

I'm thinking addPermittedAccount could be the way to go!

darkwing avatar Feb 15 '24 01:02 darkwing

Also, adding a quick test in ui/components/multichain/account-list-item-menu/account-list-item-menu.test.js to ensure the correct action and account are used would be great!

darkwing avatar Feb 15 '24 01:02 darkwing

Also, adding a quick test in ui/components/multichain/account-list-item-menu/account-list-item-menu.test.js to ensure the correct action and account are used would be great!

@darkwing Updated the actions here, https://github.com/MetaMask/metamask-extension/pull/22941/commits/baaf4c1658954bca66379eb94bb6681f25dc2ee0

For the tests, since this component is accessible only in popup view, if I try to add the test. It's not accessible for jest and it starts failing here as the DOM doesn't render it in full screen view Screenshot 2024-02-16 at 4 24 06 PM

NidhiKJha avatar Feb 16 '24 10:02 NidhiKJha

Builds ready [7ea16ed]
Page Load Metrics (992 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1243641885527
domContentLoaded10101282512
load852134499210048
domInteractive10101282512
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 48 Bytes (0.00%)
  • common: 79 Bytes (0.00%)

metamaskbot avatar Feb 16 '24 11:02 metamaskbot

Would doing the following allow for testing?

{process.env.MULTICHAIN || process.env.IN_TEST ? (

darkwing avatar Feb 16 '24 22:02 darkwing

added tests

Updated here https://github.com/MetaMask/metamask-extension/pull/22941/commits/69e1e37f9f26c5ab3aeda98cfd45d3221ab719ee

NidhiKJha avatar Feb 19 '24 11:02 NidhiKJha

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.

github-actions[bot] avatar Feb 19 '24 14:02 github-actions[bot]

Builds ready [0bd64f6]
Page Load Metrics (952 ± 36 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1132521713517
domContentLoaded97222199
load83910829527536
domInteractive97222199
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 384 Bytes (0.01%)
  • common: 79 Bytes (0.00%)

metamaskbot avatar Feb 19 '24 14:02 metamaskbot

Builds ready [653cfd2]
Page Load Metrics (1981 ± 231 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint944521918440
domContentLoaded10148373416
load22826281981482231
domInteractive10148373416
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 265 Bytes (0.00%)
  • common: 79 Bytes (0.00%)

metamaskbot avatar Feb 21 '24 08:02 metamaskbot