-
Notifications
You must be signed in to change notification settings - Fork 408
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: allow using the injected wallet on mobile #1880
Conversation
Branch preview✅ Deploy successful! |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
Tested it on iOS with the MM app and it gives me options to choose from 👍 Tested it on iOS with the Coinbase Wallet app and it also lets me choose and connects to the Coinbase Wallet. One small issue I noticed is that there seem to be 2 scrollbars present and sometimes it doesn't let me scroll to the bottom of the list. It might be something that needs to be fixed in Onboard though. |
src/hooks/wallets/useOnboard.ts
Outdated
if (typeof window === 'undefined') { | ||
return false | ||
} | ||
return !!window?.ethereum |
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.
Can you please make this one line?
// On mobile, automatically choose WalletConnect | ||
if (!options && isMobile()) { | ||
// On mobile, automatically choose WalletConnect if there is no injected wallet | ||
if (!options && isMobile() && !hasInjectedWallet()) { |
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! Did you manage to test it with the coinbase wallet?
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, it works nicely.
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.
🚀
Note: I do not have access to an iOS device and have only tested this on Android. Faking the UA on desktop shows it working on iOS, but it should still be tested on a physical device.
What it solves
Resolves #1878
How this PR fixes it
The presence of
window.ethereum
is also checked before automatically connecting to WalletConnect on mobile. If it is present (an inject wallet exists), no autoconnection occurs.How to test it
Without injected wallet
With injected wallet
Screenshots
Without injected wallet
With injected wallet
Checklist