Skip to content

fix: UX: NetworkList: Ensure setNetworkClientIdForDomain is called for test networks#25195

Merged
darkwing merged 3 commits intodevelopfrom
network-list-item-consolidation
Jun 11, 2024
Merged

fix: UX: NetworkList: Ensure setNetworkClientIdForDomain is called for test networks#25195
darkwing merged 3 commits intodevelopfrom
network-list-item-consolidation

Conversation

@darkwing
Copy link
Copy Markdown
Contributor

@darkwing darkwing commented Jun 11, 2024 •
edited by github-actions bot
Loading

Description

This PR removes duplicate code for NetworkListItem creation, instead using a helper function to create a single item.

Open in GitHub Codespaces

Related issues

Fixes: N/A

Manual testing steps

  1. Click "Polygon" and see chain switch to Polygon
  2. Click "Ethereum Mainnet" and see chain switch to Ethereum Mainnet
  3. Drag and drop networks around -- should work
  4. Click the toggle to view test networks
  5. Click a test network -- should change to correct network

Screenshots/Recordings

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.

@darkwing darkwing requested a review from a team as a code owner June 11, 2024 02:48
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 11, 2024

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.

@darkwing darkwing added team-core-extension-ux Core Extension UX team needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. labels Jun 11, 2024
@darkwing darkwing mentioned this pull request Jun 11, 2024
Closed
7 tasks
NidhiKJha
NidhiKJha approved these changes Jun 11, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@NidhiKJha NidhiKJha left a comment

Choose a reason for hiding this comment

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

LGTM

@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Jun 11, 2024

Builds ready [9420159]
Page Load Metrics (47 ± 3 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint66948084
domContentLoaded8121011
load39614773
domInteractive8121011
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -453 Bytes (-0.01%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link
Copy Markdown

codecov bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 70.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 65.62%. Comparing base (2b2ee43) to head (9420159).
Report is 3 commits behind head on develop.

Files Patch % Lines
.../multichain/network-list-menu/network-list-menu.js 70.00% 6 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #25195      +/-   ##
===========================================
+ Coverage    65.61%   65.62%   +0.01%     
===========================================
  Files         1365     1365              
  Lines        54224    54215       -9     
  Branches     14180    14175       -5     
===========================================
+ Hits         35576    35578       +2     
+ Misses       18648    18637      -11

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

@darkwing darkwing merged commit c67aa3f into develop Jun 11, 2024
@darkwing darkwing deleted the network-list-item-consolidation branch June 11, 2024 22:38
@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2024
@metamaskbot metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 11, 2024
@darkwing darkwing changed the title fix: UX: NetworkList: Consolidate NetworkListItem usages fix: UX: NetworkList: Ensure setNetworkClientIdForDomain is called for test networks Jun 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@NidhiKJha NidhiKJha NidhiKJha approved these changes

+1 more reviewer

@bergeron bergeron bergeron approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@NidhiKJha NidhiKJha

Labels

needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-12.1.0 Issue or pull request that will be included in release 12.1.0 team-core-extension-ux Core Extension UX team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@darkwing @metamaskbot @bergeron @NidhiKJha