Skip to content
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

Use @solana/wallet-adapter-react for compatibility with Mobile Wallet Adapter and the Wallet Standard #423

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jordaaash
Copy link

In this PR we upgrade @solana/wallet-adapter-react and replace a modified WalletProvider and various hooks. The main benefit of doing so is that Mango gets the following for free:

  1. The ability to connect to any mobile wallet app that supports the Solana Mobile Stack Mobile Wallet Adapter Protocol.
  2. The ability to connect to any desktop wallet that supports the Solana Wallet Standard.

You can read much more about the wider effort to upgrade the entire ecosystem of Solana web apps at: anza-xyz/wallet-adapter#604

Approach

Mango's UI makes use of a custom WalletProvider context provider implementation. This copies many behaviors from the one in @solana/wallet-adapter-react, but removes the clearing of wallets on disconnection and errors. Because the stock @solana/wallet-adapter-react WalletProvider is needed to get the above benefits, we restore it in this PR.

However, we extend this with a custom child provider that implements the behavior that Mango's UI currently has, of preselecting the first displayed wallet, remembering it, and displaying it in the connect button. In this way, we're able to keep what I think is the desired behavior, while restoring auto-connect functionality that improves the UX on desktop and especially on mobile using Mobile Wallet Adapter.

I've (manually, but heavily) tested connect, disconnect, wallet switching, error handling, autoconnect, and account viewing on desktop with multiple wallets. I didn't test transactions in Mango because the UI is currently warned to be non-functional.

Screenshots

Wallet Menu

Backpack and Glow, both Standard Wallets, are detected without using adapters. Glow's adapter is included in the code, but the Standard Wallet is used instead, without duplication.

Screen Shot 2022-10-23 at 12 34 26 AM

Connected Wallet

Backpack, a Standard Wallet, is shown connected to Mango without using an adapter.

Screen Shot 2022-10-23 at 12 36 10 AM

@vercel
Copy link

vercel bot commented Oct 23, 2022

@jordansexton is attempting to deploy a commit to the blockworks-foundation Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 23, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
mango-ui-v3-devnet ✅ Ready (Inspect) Visit Preview Oct 23, 2022 at 0:58AM (UTC)
mango-ui-v3-testnet ✅ Ready (Inspect) Visit Preview Oct 23, 2022 at 0:58AM (UTC)

const wallets = useMemo(
() => [
new BackpackWalletAdapter(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like backpack disappeared

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i see this works now without adapter, awesome

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly! We should be able to remove the Glow adapter soon also. The desktop browser extension supports the standard, but the mobile WebView version doesn't yet.

@jordaaash
Copy link
Author

Updated to the latest RC of wallet-adapter-react. No functional changes for dapps, just makes how wallets are detected on the window more robust.

@jordaaash
Copy link
Author

Maybe don't review just yet, still making some internal changes. This won't affect you, but want to avoid spamming you with new RCs.

@jordaaash jordaaash changed the title Use @solana/wallet-adapter-react for compatibility with Mobile Wallet Adapter and the Wallet Standard [DRAFT] Use @solana/wallet-adapter-react for compatibility with Mobile Wallet Adapter and the Wallet Standard Oct 25, 2022
@jordaaash
Copy link
Author

We're at RC 9, which looks likely to be the last RC. This now updates Mango to use the latest RC. I'll follow up once we hit full release. There are no functional changes for apps with this update. It makes how wallets attach to the window more robust.

@jordaaash jordaaash changed the title [DRAFT] Use @solana/wallet-adapter-react for compatibility with Mobile Wallet Adapter and the Wallet Standard Use @solana/wallet-adapter-react for compatibility with Mobile Wallet Adapter and the Wallet Standard Oct 26, 2022
@mschneider
Copy link
Contributor

mschneider commented Oct 28, 2022

i just tried your changes locally on devnet yarn devnet

  1. an error appears after connecting sollet and navigating the page a bit:

image

  1. after reconnect the wallet adapter tries to auto-reconnect sollet, which causes chrome to "prevent a popup", after allowing the popup, sollet does not load the wallet connect dialogue, but show the balances, as if sollet.io was opened in a new window:

image

  1. after closing the popup i try to connect sollet manually again, wallet never really connects. then tried to open the list of accounts (click on the wallet connect button that now displays profile name and wallet pubkey) and got another error:

image

@jordaaash
Copy link
Author

@mschneider thanks for the repro steps, will try this. What version of node are you on so I can run the same?

@mschneider
Copy link
Contributor

17.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants