fix: mm request permissions on request accounts pending #656
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
gomesalexandre
changed the title
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
3 tasks
gomesalexandre
changed the title
gomesalexandre
force-pushed
the
feat_mm_locked_request_ccounts
branch
2 times, most recently
from
ef411a8 to
32b94e8
Compare
gomesalexandre
force-pushed
the
feat_mm_locked_request_ccounts
branch
from
2ea5ed9 to
08db6cb
Compare
0xApotheosis
approved these changes
Dec 12, 2023
There was a problem hiding this comment.
This makes a really annoying UX moderately annoying 🙏
Played with wallet_requestPermissions and confirmed it does what we want it to do by forcing the Metamask window to open (albeit forcing the user to reselect accounts, but it's a definite improvement nonetheless!).
woodenfurniture
approved these changes
Dec 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See MetaMask/metamask-extension#10085 for reference
tl;dr is MM API doesn't make any sense, and doesn't allow for
eth_requestAccountsrequests if there's already a pending one. This will throw errors and make users unable to unlock their wallet, effectively being in an infinite loop.Having request/s pending could happen if users dismissed previous connection requests and/or wallet unlock, or worse, if MM popup never properly popped up and discretely went with a notification instead (which can sometimes happen and is out of our control).
This PR uses
wallet_requestPermissionsinstead when the former failed. That will ensure MM properly pops up when it should, the downside of this being users will have to "reconnect" accounts even if they're already connected.Screenshots
(Don't mind the slowness / snap UI missing, this was with localhost debugging)
Untitled.mov
Issue
Relates to web's shapeshift/web#5453