Skip to content

fix: snap confirmation queue #33040

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

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

Conversation

hmalik88
Copy link
Contributor

@hmalik88 hmalik88 commented May 17, 2025

Description

Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions:

  1. What is the reason for the change? The confirmation queue feature was never applied to snap install flow approval requests, so this creates for a weird UX when you have multiple confirmations with a snap install approval request in the queue.
  2. What is the improvement/solution? Add confirmation navigation to the snaps connect, install, update and install result screens via the Nav component. State was added to track the current snap approval that's being viewed. The last current snap in approval flow will be the snap who's next approval we will want to navigate to. State was also added to track a snap's connect times to reorder navigations in the confirmation queue (we want to maintain the position in the queue as we go from snap-connect -> snap-install -> snap-install-result or from snap-update -> snap-install-result).

NOTE: This requires an update in the approval controller & controller-utils side, so the new packages will have to be cut and integrated before this PR can be merged.

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

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.

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.

@metamaskbot metamaskbot added the team-snaps-platform Snaps Platform team label May 17, 2025
@metamaskbot
Copy link
Collaborator

✨ Files requiring CODEOWNER review ✨

✅ @MetaMask/confirmations

  • ui/pages/confirmations/hooks/useConfirmationNavigation.ts

🫰 @MetaMask/snaps-devs

  • ui/pages/permissions-connect/snaps/snap-install/snap-install.js
  • ui/pages/permissions-connect/snaps/snap-result/snap-result.js
  • ui/pages/permissions-connect/snaps/snap-update/snap-update.js
  • ui/pages/permissions-connect/snaps/snaps-connect/snaps-connect.js

🖥️ @MetaMask/wallet-ux

  • ui/pages/home/home.component.js
  • ui/pages/home/home.container.js

@@ -128,8 +121,30 @@ export function navigateToConfirmation(
return;
}

if (CONNECT_APPROVAL_TYPES.includes(type)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was an extra render for no reason, since we know the approval type, we can navigate directly to the appropriate snap page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants