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

fix: Updated selector for the test networks #23986

Merged
merged 5 commits into from
Apr 16, 2024
Merged

fix: Updated selector for the test networks #23986

merged 5 commits into from
Apr 16, 2024

Conversation

NidhiKJha
Copy link
Member

This PR is to ensure that the permissions page renders list of all the permissions with the current networks. For a few networks, we don't have the rpcPrefs that was causing the siteKey nd the rpcPrefs to be undefined.
This PR also makes sure that the defined background color is there for the test networks that doesn't have a logo

Related issues

Fixes: #23812 #23793

Manual testing steps

  1. Connect an account on Uniswap
  2. Open the popup
  3. Open the three-dot menu and choose "All Permissions"
  4. It should render the permission with the accounts

Screenshots/Recordings

Before

Screen.Recording.2024-04-12.at.12.00.01.AM.mov

After

Screen.Recording.2024-04-11.at.11.58.43.PM.mov

Pre-merge author checklist

  • I’ve followed MetaMask 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.

@NidhiKJha NidhiKJha requested a review from a team as a code owner April 11, 2024 18:30
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Apr 11, 2024
@NidhiKJha NidhiKJha changed the title fix:updated selector for the test networks fix: Updated selector for the test networks Apr 11, 2024
@NidhiKJha NidhiKJha added team-wallet-ux needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. labels Apr 11, 2024
@@ -1436,6 +1436,7 @@ export const getConnectedSitesList = createDeepEqualSelector(
getAllConnectedAccounts,
(connectedSubjectsForAllAddresses, identities, connectedAddresses) => {
const sitesList = {};
console.log(sitesList)
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs removal

@@ -1467,7 +1468,8 @@ export const getConnectedSitesListWithNetworkInfo = createDeepEqualSelector(
const connectedNetwork = networks.find(
(network) => network.id === domains[siteKey],
);
sitesList[siteKey].networkIconUrl = connectedNetwork.rpcPrefs.imageUrl;
// For the testnets, if we do not have an image, we will have a fallback string
sitesList[siteKey].networkIconUrl = connectedNetwork.rpcPrefs ? connectedNetwork.rpcPrefs.imageUrl : '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be:

connectedNetwork.rpcPrefs??.imageUrl || ''

@NidhiKJha NidhiKJha requested a review from darkwing April 15, 2024 07:11
Copy link

codecov bot commented Apr 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.53%. Comparing base (b056a8b) to head (f050198).
Report is 60 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #23986      +/-   ##
===========================================
- Coverage    67.53%   67.53%   -0.00%     
===========================================
  Files         1245     1245              
  Lines        48848    48848              
  Branches     12743    12746       +3     
===========================================
- Hits         32989    32987       -2     
- Misses       15859    15861       +2     

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

@metamaskbot
Copy link
Collaborator

Builds ready [f050198]
Page Load Metrics (1981 ± 736 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint683452057034
domContentLoaded11203634522
load51390419811533736
domInteractive11203634522
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 192 Bytes (0.00%)
  • common: 44 Bytes (0.00%)

@NidhiKJha NidhiKJha merged commit 01a4d60 into develop Apr 16, 2024
67 of 68 checks passed
@NidhiKJha NidhiKJha deleted the selector-fix branch April 16, 2024 08:43
@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2024
@metamaskbot metamaskbot added the release-11.16.0 Issue or pull request that will be included in release 11.16.0 label Apr 16, 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.16.0 Issue or pull request that will be included in release 11.16.0 team-wallet-ux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Multichain: All Permissions Page Breaks in Popup
4 participants