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

Update Wallet Adapter #532

Merged

Conversation

jordaaash
Copy link
Contributor

@jordaaash jordaaash commented Oct 21, 2022

Description

This PR updates Wallet Adapter according to anza-xyz/wallet-adapter#604. This adds support for the Wallet Standard and updates Mobile Wallet Adapter.

The PR also adds yarn resolutions for @solana/web3.js and react to avoid multiple versions being resolved.

How has this been tested?

Building the project locally, opening the wallet modal and seeing Backpack and Glow detected as Standard Wallets, and connecting to Backpack without an adapter.

Types of changes

  • Technical Debt (non-breaking change which removes unused code/assets)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • The related ClickUp task has been linked to this PR
  • The person creating the pull request is listed in "Assignees"
  • Change requires updated documentation

Screenshots

Backpack and Glow detected as Standard Wallets, so they show up first.
Screen Shot 2022-10-21 at 2 56 49 PM

Connected to Backpack without an adapter.
Screen Shot 2022-10-21 at 2 59 47 PM

@walt-1
Copy link
Collaborator

walt-1 commented Oct 21, 2022

Hi @jordansexton,

I am playing around with your branch locally and am noticing some issues with connect/disconnect. If you are done with your implementation, I think it would be best to merge into a working branch rather than the dev branch so I can change the way we are handing that given the updates.

Let me know if this is still a work in progress or if it is ready to merge.

Thanks!

@walt-1 walt-1 changed the base branch from dev to mob-wallet-adapter-wip October 21, 2022 22:23
@jordaaash
Copy link
Contributor Author

It's ready to merge! What are the issues with connecting/disconnecting though? Anything we need to be concerned about in the library?

@jordaaash
Copy link
Contributor Author

I pushed some updates to this to include the latest Release Candidate of @solana/wallet-adapter-react and the latest version of @solana-mobile/wallet-adapter-mobile. Please let me know if your connect/disconnect issues persist!

@walt-1
Copy link
Collaborator

walt-1 commented Oct 24, 2022

Because we are not using the autoConnect param, we build custom logic around maintaining wallet connection. Some of this needs to be refined so its likely on our side, not on the library. But if I find any problems, I will let you know.

@jordaaash
Copy link
Contributor Author

Maybe don't merge yet, I'm reworking some things in the dependencies, will push a new version.

@jordaaash jordaaash changed the title Update Wallet Adapter [DRAFT] Update Wallet Adapter Oct 25, 2022
@jordaaash jordaaash changed the title [DRAFT] Update Wallet Adapter Update Wallet Adapter Oct 25, 2022
@jordaaash
Copy link
Contributor Author

We're at RC 9, which looks likely to be the last RC. This PR updates GooseFX 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.

@walt-1
Copy link
Collaborator

walt-1 commented Oct 26, 2022

@jordansexton - give a thumbs up if you are done and I will merge.

@jordaaash
Copy link
Contributor Author

Yep, I'm done!

@walt-1 walt-1 merged commit d58b9c3 into GooseFX1:mob-wallet-adapter-wip Oct 26, 2022
@jordaaash jordaaash deleted the update-wallet-adapter branch October 27, 2022 03:38
@steveluscher
Copy link

Hey! I think there's still a bug with mobile wallet adapter.

Screen.Recording.2022-10-27.at.11.20.22.AM.mov

Are you, perchance, recreating the SolanaMobileWalletAdapter after connecting (eg. because memoization is getting busted?).

@steveluscher
Copy link

I'll fix this.

@billythedummy
Copy link

billythedummy commented Oct 28, 2022

@steveluscher reporting another possible issue here: i don't get redirected to https://solanamobile.com/wallets when i click on the button with an android phone that has no wallet installed and seem to just get stuck on the loading state forever

Browser: firefox v99.2.0.

mobilebroken.mp4

On chrome v100.0.4896.127 the loading state disappears but nothing happens

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.

4 participants