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.

Sign up for GitHub

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

Jump to bottom

fix(per-dapp-selected-network): restore per-dapp network picker in permissions view #13417

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

fix(per-dapp-selected-network): restore per-dapp network picker in permissions view #13417

wants to merge 13 commits into from
+131 −22

Conversation

EtherWizard33
Copy link
Contributor

@EtherWizard33 EtherWizard33 commented Feb 11, 2025 •
edited
Loading

Description

This PR fixes the regression on PR #10788, and then restores the network picker functionality in the permissions view that was previously removed, while properly integrating with the per-dapp network selection feature. Changes include:

Add a touchable favicon to the permission summary header that enables network
switching for dapps. This combines the dapp's identity (favicon/token) with
network selection in a single interactive component.

  • Replace static WebsiteIcon with touchable BadgeWrapper pattern
  • Add network badge to indicate current network
  • Enable network switching via network selector bottom sheet
  • Add fallback to AvatarToken when favicon unavailable
  • Guard implementation behind per-dapp network selection feature flag
  • Preserve original WebsiteIcon when feature disabled
  • Add proper network info display in permissions view
  • Integrate with @metamask/selected-network-controller for per-dapp network state

Related issues

Contributes to: https://github.com/MetaMask/MetaMask-planning/issues/4104

Manual testing steps

Testing:

  1. Enable feature flags below if they are not enabled already
  2. Connect to a dapp and verify network picker appears in permissions view
  3. Switch networks and verify correct network displays for that specific dapp
  4. Verify network switching works properly and modal dismisses correctly
  5. Verify network info displays correctly in permissions view

Make sure to include all env vars:

export MM_MULTICHAIN_V1_ENABLED="true"
export MM_CHAIN_PERMISSIONS="true"
export MM_PER_DAPP_SELECTED_NETWORK="true"
export MULTICHAIN_V1="true"

Screenshots/Recordings

Before After
Screenshot 2024-04-18 at 3 56 43 PM Screenshot 2024-04-18 at 3 56 43 PM

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.

@EtherWizard33
feat: Add a temporary picker to test if per-dapp-selected-networks fe…
d84986d 
…ature can work when its feature flags are enabled

make sure to include all env vars:
export MM_MULTICHAIN_V1_ENABLED="true"
export MM_CHAIN_PERMISSIONS="true"
export MM_PER_DAPP_SELECTED_NETWORK="1"
export MULTICHAIN_V1="true"
@github-actions GitHub Actions
Copy link
Contributor

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.

@EtherWizard33 EtherWizard33 added the No QA Needed Apply this label when your PR does not need any QA effort. label Feb 11, 2025
salimtb and others added 7 commits February 11, 2025 14:38
@salimtb
fix: fix permissisons network switch
3849817
@EtherWizard33
style: remove console.logs
8617606
@EtherWizard33
Merge branch 'main' into feat-4144-per-dapp-selected-network-poc
be0f3cd
@EtherWizard33
style: remove console.log from permission summary
417aec7
@EtherWizard33
style: remove console.log from network selector
9283759
@EtherWizard33
feat: remove console.log from chain utils
cf54d10
@EtherWizard33
refactor(network): use isPerDappSelectedNetworkEnabled helper
ab50a98 
- Replace direct checks of process.env.MM_PER_DAPP_SELECTED_NETWORK in AccountPermissionsConnected.tsx with the isPerDappSelectedNetworkEnabled function.
- Update ethereum-chain-utils.js to use isPerDappSelectedNetworkEnabled when switching networks.
- Modify selectedNetworkController selectors to use isPerDappSelectedNetworkEnabled for feature flag checks.
- Add the isPerDappSelectedNetworkEnabled utility in util/networks/index.js.

This change centralizes the per-DApp network logic for better consistency and maintainability.
@EtherWizard33 EtherWizard33 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed No QA Needed Apply this label when your PR does not need any QA effort. labels Feb 12, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 12, 2025 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: ab50a98
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/3b82aa81-e1e9-4714-8125-02d2a555894b

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@EtherWizard33 EtherWizard33 changed the title feat: Add a temporary picker to test if per-dapp-selected-networks feature can work when its feature flags are enabled fix(per-dapp-selected-network): restore per-dapp network picker in permissions view Feb 12, 2025
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
61.6% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@EtherWizard33
Merge branch 'main' into feat-4144-per-dapp-selected-network-poc
30143fc
@EtherWizard33
feat(permissions): add interactive favicon with network switching
64ab7a5 
Add a touchable favicon to the permission summary header that enables network
switching for dapps. This combines the dapp's identity (favicon/token) with
network selection in a single interactive component.

- Replace static WebsiteIcon with touchable BadgeWrapper pattern
- Add network badge to indicate current network
- Enable network switching via network selector bottom sheet
- Add fallback to AvatarToken when favicon unavailable
- Guard implementation behind per-dapp network selection feature flag
- Preserve original WebsiteIcon when feature disabled

This matches the interaction pattern from AccountPermissionsConnected,
providing a consistent way to manage dapp-specific networks across the app.
@EtherWizard33 EtherWizard33 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Feb 13, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 13, 2025 •
edited by metamaskbot
Loading

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 64ab7a5
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d3858663-d695-4ec4-a54a-55fe198f4895

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Run Smoke E2E Triggers smoke e2e on Bitrise team-wallet-ux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@EtherWizard33 @metamaskbot @salimtb