Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UX Multichain: Updated app header account picker and address button #22637

Merged
merged 11 commits into from Feb 8, 2024

Conversation

NidhiKJha
Copy link
Member

This PR is to update the following changes in App-Header:

  1. In the Account Picker,
  • Updated the AvatarSize
  • Removed address since address now has a copy button
  1. Updated Adress text and copy button
  2. I have also removed the copy button which was earlier introduced to remove connections icon for Multichain.

Related issues

Fixes: 1816

Manual testing steps

  1. Run extension with MULTICHAIN=1 yarn start
  2. Check for copy button under the account picker

Screenshots/Recordings

Before

Screenshot 2024-01-23 at 4 36 18 PM

After

Full Screen

Screenshot 2024-01-23 at 4 35 14 PM

Extension View

Screenshot 2024-01-23 at 4 35 29 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 NidhiKJha requested a review from a team as a code owner January 23, 2024 11:07
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jan 23, 2024
@NidhiKJha NidhiKJha added team-extension-client team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead labels Jan 23, 2024
Copy link
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few requests/suggestions:

  1. Can we make the entire address a button so that I can click the shorten address and get the copy functionality as well?
  2. I think I added a showAddress prop to the AccountPicker; is that still needed with these changes? (I guess maybe send flow?)
  3. Is there a way we can better center the address? The 0x pops out on the left, but the buttons line up on the right. Just a thought.
SCR-20240123-iqoe

@metamaskbot
Copy link
Collaborator

Builds ready [0c20608]
Page Load Metrics (757 ± 19 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint86148106147
domContentLoaded9251442
load6858487573919
domInteractive9251442
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -248 Bytes (-0.00%)
  • common: 0 Bytes (0.00%)

@NidhiKJha
Copy link
Member Author

  1. Can we make the entire address a button so that I can click the shorten address and get the copy functionality as well?

Updated here cbaef93

vthomas13
vthomas13 previously approved these changes Jan 31, 2024
Copy link
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As part of these changes, we need to remove the blue address button:

SCR-20240131-jkdp

Also, should we add a hover background to the address that looks like the Account Picker's?

@NidhiKJha
Copy link
Member Author

NidhiKJha commented Jan 31, 2024

As part of these changes, we need to remove the blue address button:

Updated here a7497f8

Also, should we add a hover background to the address that looks like the Account Picker's?

Updated here a8a53b1

@metamaskbot
Copy link
Collaborator

Builds ready [e0bce92]
Page Load Metrics (815 ± 25 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1022151232512
domContentLoaded10541994
load7469368155225
domInteractive10541994
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -108 Bytes (-0.00%)
  • common: 0 Bytes (0.00%)

darkwing
darkwing previously approved these changes Feb 2, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [f480ce1]
Page Load Metrics (825 ± 33 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint942101202412
domContentLoaded10461774
load71710348256933
domInteractive10461774
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -108 Bytes (-0.00%)
  • common: 0 Bytes (0.00%)

@darkwing darkwing added the needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. label Feb 7, 2024
@NidhiKJha NidhiKJha merged commit 0b52d3d into develop Feb 8, 2024
68 of 69 checks passed
@NidhiKJha NidhiKJha deleted the header-updates-v1 branch February 8, 2024 17:04
@github-actions github-actions bot locked and limited conversation to collaborators Feb 8, 2024
@metamaskbot metamaskbot added the release-11.12.0 Issue or pull request that will be included in release 11.12.0 label Feb 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
INVALID-PR-TEMPLATE PR's body doesn't match template needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-11.12.0 Issue or pull request that will be included in release 11.12.0 team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead team-wallet-ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants