-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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: Fix snaps permission connection for CHAIN_PERMISSIONS
feature flag
#27459
Conversation
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. |
render={() => | ||
process.env.CHAIN_PERMISSIONS && !permissionsRequest?.diff ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you briefly explain why !permissionsRequest?.diff
isn't necessary anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
metamask-extension/ui/pages/permissions-connect/permissions-connect.component.js
Lines 152 to 158 in ed3311a
if ( | |
permissionsRequest?.diff?.permissionDiffMap?.[ | |
PermissionNames.permittedChains | |
] | |
) { | |
history.replace(confirmPermissionPath); | |
} |
This guard was ensuring that the incremental request for permittedChains goes to the confirmPermissionPath
route in the first place. Now the new ConnectPage
component has been moved out of the confirmPermissionPath
and into the connectPath
route.
I recommend viewing the file as a whole as the github diff makes it a little bit difficult to notice this change
CHAIN_PERMISSIONS
feature flag
|
Builds ready [ed3311a]
Page Load Metrics (1792 ± 83 ms)
Bundle size diffs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Recent permission flow changes introduced behind the
CHAIN_PERMISSIONS
feature flag have broken permission connection for Snaps. This PR fixes it by removing an incorrect forced route direct in the permission connection component.Related issues
Fixes: #26635
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist