-
Notifications
You must be signed in to change notification settings - Fork 461
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: disconnect previous wallet #2712
Conversation
Branch preview✅ Deploy successful! https://fix_disconnect_previous_wallet--walletweb.review-wallet-web.5afe.dev |
That was actually a feature. If we're throwing it away, you can just remove the Switch wallet button altogether. Alternatively, disconnect only if it's a social login. |
I would also be in favour of limiting this to social login only and adding tracking for the Switch Wallet button to decide later if we should remove it or not. |
} | ||
|
||
if (newWalletLabel !== oldWalletLabel) { | ||
await onboard.disconnectWallet({ label: oldWalletLabel }) |
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.
So we disconnect if the old wallet is social login? What if the new wallet is social login and the old one is MM? Will it work fine?
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.
Yes exactly.
Only the Social signer will be disconnected if switched to another wallet.
We could also remove this behavior once we support every chain. But this is currently causing issues QA found.
What it solves
When switching the wallet the previously connected wallet stays connected.
This can lead to many edge cases for the social signer.
How this PR fixes it
If the user successfully switches the wallet, we disconnect the old wallet label.
How to test it
Checklist