fix: Issue 20485 #25518

ejwessel · 2024-06-26T00:18:15Z

Description

The networks indicator on the confirmations page was showing incorrectly for networks other than Ethereum and Sepolia. If the network was anything other than those two main networks then a gray question mark would show. Not only was this unsightly, but it's confusing for the user to see a known network in the network dropdown only for it to know be recognized in the confirmation screen.
The improvement is to repurpose the Network Display and reuse the the NetworkPicker so that there is consistency between the selected dropdown view and the confirmations network identifier.

Related issues

Fixes:

Manual testing steps

Go to 'Send'
Fill out 'Send' fields
Click 'Continue'
Validate that the network dropdown (of which is disabled) matches that of the network icon within the confirmation page; same icon and the same name.

Screenshots/Recordings

Before

After

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
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.

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.

Signed-off-by: Ethan Wessel <[email protected]>

github-actions · 2024-06-26T00:18:29Z

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.

Signed-off-by: Ethan Wessel <[email protected]>

georgewrmarshall

Looking great! We may want to add before/after screenshots to ensure no visual regressions. Made one suggestion that should fix the failing e2e tests

ui/components/app/network-display/network-display.js

Co-authored-by: George Marshall <[email protected]>

Signed-off-by: Ethan Wessel <[email protected]>

micaelae · 2024-06-26T19:12:59Z

ui/components/app/network-display/network-display.js

+      label={currentNetwork?.nickname}
+      labelProps={{ 'data-testid': 'network-display' }}
+      src={currentNetwork?.rpcPrefs?.imageUrl}
+      iconProps={{ display: 'none' }} // do not show the dropdown icon


Thoughts on making the icon smaller? It seems to be too close to the container's borders

Looks like this has been updated

codecov · 2024-06-26T20:47:33Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.67%. Comparing base (4e2596f) to head (0b1c367).
Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #25518      +/-   ##
===========================================
- Coverage    69.70%   69.67%   -0.03%     
===========================================
  Files         1349     1349              
  Lines        47864    47850      -14     
  Branches     13205    13195      -10     
===========================================
- Hits         33359    33337      -22     
- Misses       14505    14513       +8

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

metamaskbot · 2024-06-26T20:59:14Z

Builds ready [0b1c367]

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, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (128 ± 168 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	68	182	88	23	11
		domContentLoaded	9	15	11	1	1
		load	41	1652	128	350	168
		domInteractive	9	15	11	1	1

Bundle size diffs [🚀 Bundle size reduced!]

background: 0 Bytes (0.00%)
ui: -4.39 KiB (-0.06%)
common: -67 Bytes (-0.00%)

georgewrmarshall · 2024-06-26T21:23:11Z

ui/components/app/network-display/network-display.js

+      labelProps={{ 'data-testid': 'network-display' }}
+      src={currentNetwork?.rpcPrefs?.imageUrl}
+      iconProps={{ display: 'none' }} // do not show the dropdown icon
+      avatarNetworkProps={{ size: AvatarNetworkSize.Xs }}


non-blocking: not sure why this is required it's this size by default 🤔

georgewrmarshall

LGTM! 🚀

Checked NetworkDisplay in send flow ✅
Checked NetworkDisplay in storybook ✅

ejwessel added 2 commits June 25, 2024 17:06

fix: issue 20485

bc6f0d3

Signed-off-by: Ethan Wessel <[email protected]>

chore: update styling behavior

0b27fce

Signed-off-by: Ethan Wessel <[email protected]>

Merge branch 'develop' into issue-20485-fix

efcc6dc

ejwessel changed the title ~~Issue 20485 fix~~ fix: Issue 20485 Jun 26, 2024

ejwessel added the send-swap label Jun 26, 2024

This was referenced Jun 26, 2024

[Bug]: Chain icons are not displayed on confirmations #25447

Closed

[Bug]: Certain chains icons are not displayed in the Send flow #24450

Closed

ejwessel added the team-confirmations Push issues to confirmations team label Jun 26, 2024

ejwessel added 2 commits June 25, 2024 17:32

chore: lint fix

22e6b16

Signed-off-by: Ethan Wessel <[email protected]>

chore: update snapshot

c17e7a3

Signed-off-by: Ethan Wessel <[email protected]>

ejwessel marked this pull request as ready for review June 26, 2024 00:40

ejwessel requested review from a team as code owners June 26, 2024 00:40

ejwessel added 4 commits June 25, 2024 17:53

chore update snapshots

cd93695

chore: update locales

56d3e68

chore update snapshots

46398bf

chore: update snapshot

c634dd9

Signed-off-by: Ethan Wessel <[email protected]>

georgewrmarshall reviewed Jun 26, 2024

View reviewed changes

ui/components/app/network-display/network-display.js Show resolved Hide resolved

ejwessel and others added 5 commits June 26, 2024 10:42

Update ui/components/app/network-display/network-display.js

06ac174

Co-authored-by: George Marshall <[email protected]>

Merge branch 'develop' into issue-20485-fix

f594f30

chore: lint fix

aa3db1a

chore: update snapshot

568fd1f

Signed-off-by: Ethan Wessel <[email protected]>

chore: update snapshots

66afd12

micaelae reviewed Jun 26, 2024

View reviewed changes

chore: make icon smaller and update snapshot

0b1c367

micaelae approved these changes Jun 26, 2024

View reviewed changes

georgewrmarshall reviewed Jun 26, 2024

View reviewed changes

georgewrmarshall approved these changes Jun 26, 2024

View reviewed changes

OGPoyraz approved these changes Jun 26, 2024

View reviewed changes

ejwessel merged commit 69ec573 into develop Jun 26, 2024
91 checks passed

ejwessel deleted the issue-20485-fix branch June 26, 2024 23:31

github-actions bot locked and limited conversation to collaborators Jun 26, 2024

metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Issue 20485 #25518

fix: Issue 20485 #25518

ejwessel commented Jun 26, 2024 •

edited

Loading

github-actions bot commented Jun 26, 2024

georgewrmarshall left a comment

micaelae Jun 26, 2024

georgewrmarshall Jun 26, 2024

codecov bot commented Jun 26, 2024

metamaskbot commented Jun 26, 2024

georgewrmarshall Jun 26, 2024

georgewrmarshall left a comment

fix: Issue 20485 #25518

fix: Issue 20485 #25518

Conversation

ejwessel commented Jun 26, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jun 26, 2024

georgewrmarshall left a comment

Choose a reason for hiding this comment

micaelae Jun 26, 2024

Choose a reason for hiding this comment

georgewrmarshall Jun 26, 2024

Choose a reason for hiding this comment

codecov bot commented Jun 26, 2024

Codecov Report

metamaskbot commented Jun 26, 2024

georgewrmarshall Jun 26, 2024

Choose a reason for hiding this comment

georgewrmarshall left a comment

Choose a reason for hiding this comment

ejwessel commented Jun 26, 2024 •

edited

Loading