fix: redirection issue after chain switch on sign request
Description
Issue Link: MetaMask/metamask-mobile#7878
Summary:
This PR addresses a redirection issue in MetaMask Mobile where users were prematurely redirected back to the dApp following a chain switch request, without waiting for the completion of a personal_sign request.
Key Points:
Problem: After initiating a chain switch as part of a personal_sign request, users were redirected back to the dApp immediately, rather than after the personal_sign request was completed. This led to a disjointed user experience.
Solution: We've introduced a mechanism to check for any pending personal_sign requests. The wallet will now redirect users back to the dApp only after these requests have been fully processed (either approved or denied).
Implementation:
- Check for Pending Sign Requests: Before redirecting, the wallet checks if there are any pending personal_sign requests in the session.
- Conditional Redirection: Users are redirected back to the dApp only if there are no pending personal_sign requests, ensuring the sign request process is not interrupted.
This update ensures a seamless integration between MetaMask Mobile and dApps, enhancing user experience by aligning redirection behavior with the expected transaction flow.
Related issues
Fixes:
Manual testing steps
- Go to this page...
Screenshots/Recordings
Before
After
https://github.com/MetaMask/metamask-mobile/assets/61094771/ba242bb7-870b-434f-882e-484f907bb14a
Pre-merge author checklist
- [x] I’ve followed MetaMask Coding Standards.
- [x] I've clearly explained what problem this PR is solving and how it is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using JSDoc format if applicable
- [x] I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
- [x] I’ve properly set the pull request status:
- [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to "non-draft".
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.
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.
Codecov Report
Attention: 4 lines in your changes are missing coverage. Please review.
Comparison is base (
4dd9073) 40.62% compared to head (6b74aa1) 40.66%. Report is 7 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| app/core/WalletConnect/WalletConnectV2.ts | 0.00% | 4 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #8496 +/- ##
==========================================
+ Coverage 40.62% 40.66% +0.04%
==========================================
Files 1239 1240 +1
Lines 29989 30015 +26
Branches 2870 2873 +3
==========================================
+ Hits 12182 12206 +24
- Misses 17109 17110 +1
- Partials 698 699 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Quality Gate passed
Kudos, no new issues were introduced!
0 New issues
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
When will this pull request be merged?
Looking good. Tested for both iOS and Android.