Skip to content

UX: Updated Custom network displayed for selectable in network selection#22693

Merged
vthomas13 merged 5 commits intodevelopfrom
networks-same-id
Jan 30, 2024
Merged

UX: Updated Custom network displayed for selectable in network selection#22693
vthomas13 merged 5 commits intodevelopfrom
networks-same-id

Conversation

@NidhiKJha
Copy link
Copy Markdown
Member

@NidhiKJha NidhiKJha commented Jan 29, 2024

This PR is to make sure if two networks with same id but different rpcUrl are there, list should show both of them

Related issues

Fixes: #22559

Manual testing steps

  1. Have ganache running locally with npx ganache-cli --fork https://mainnet.infura.io/v3/[YOUROWNINFURAID] --chainId 1
  2. Add it as a custom network with chainId=1
  3. Select the newly added network
  4. Change to another network
  5. Note that you are able to change back to see the custom network in the list and able to switch back.

Screenshots/Recordings

Before

After

Screenshot 2024-01-29 at 6 07 35 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 29, 2024 12:37
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jan 29, 2024
DDDDDanica
DDDDDanica reviewed Jan 30, 2024
View reviewed changes
DDDDDanica
DDDDDanica reviewed Jan 30, 2024
View reviewed changes
@NidhiKJha NidhiKJha requested a review from DDDDDanica January 30, 2024 12:04
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 30, 2024

Codecov Report

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

Comparison is base (722d8fe) 68.13% compared to head (c580da4) 68.12%.
Report is 29 commits behind head on develop.

Files Patch % Lines
.../multichain/network-list-menu/network-list-menu.js 8.33% 11 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #22693      +/-   ##
===========================================
- Coverage    68.13%   68.12%   -0.00%     
===========================================
  Files         1087     1087              
  Lines        42665    42681      +16     
  Branches     11349    11354       +5     
===========================================
+ Hits         29066    29075       +9     
- Misses       13599    13606       +7

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

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jan 30, 2024

Builds ready [c580da4]
Page Load Metrics (869 ± 60 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint901921202814
domContentLoaded105621136
load737119286912660
domInteractive105621136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 176 Bytes (0.01%)
  • ui: 177 Bytes (0.00%)
  • common: 18 Bytes (0.00%)

darkwing
darkwing approved these changes Jan 30, 2024
View reviewed changes
Copy link
Copy Markdown
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.

Works wonderfully. We should move some code to selectors but can be done in a follow up!

@vthomas13 vthomas13 merged commit 3fe1f55 into develop Jan 30, 2024
@vthomas13 vthomas13 deleted the networks-same-id branch January 30, 2024 20:37
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2024
@metamaskbot metamaskbot added the release-11.11.0 Issue or pull request that will be included in release 11.11.0 label Jan 30, 2024
@chloeYue chloeYue added release-11.9.0 Issue or pull request that will be included in release 11.9.0 and removed release-11.11.0 Issue or pull request that will be included in release 11.11.0 labels Jan 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DDDDDanica DDDDDanica Awaiting requested review from DDDDDanica

2 more reviewers

@darkwing darkwing darkwing approved these changes

@vthomas13 vthomas13 vthomas13 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template release-11.9.0 Issue or pull request that will be included in release 11.9.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Custom network not displayed or selectable in network selection

6 participants

@NidhiKJha @metamaskbot @darkwing @vthomas13 @DDDDDanica @chloeYue