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

GooseFX is now compatible with the Solana Mobile Wallet Adapter protocol #536

Merged
merged 3 commits into from
Oct 31, 2022
Merged

Conversation

steveluscher
Copy link

@steveluscher steveluscher commented Oct 27, 2022

Description

In this PR we filter out MWA if it's determined not to be supported, and make it so that, when the MWA is selected, clicking the connect button starts a connection instead of popping open the dialog.

Test plan

Mobile, with mobile wallet

480.mov

Desktop

Screen.Recording.2022-10-27.at.2.01.36.PM.mov

Comment on lines 21 to 31
new SolanaMobileWalletAdapter({
appIdentity: { name: 'Goosefx App' },
authorizationResultCache: createDefaultAuthorizationResultCache(),
addressSelector: createDefaultAddressSelector(),
cluster: network,
onWalletNotFound: createDefaultWalletNotFoundHandler()
}),
Copy link
Author

Choose a reason for hiding this comment

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

I moved this to the top of the list because it gets filtered out:

  • on desktop
  • when on a non-supported mobile device (eg. iOS)

You can see this in the desktop test plan video.

@walt-1
Copy link
Collaborator

walt-1 commented Oct 28, 2022

Hi @steveluscher - I believe these changes have already been pulled into a working branch via PR 532?

There are some changes I need to make regarding custom connection logic we have on our wallets. That is why I currently have those changes in a working branch, not in dev at moment.

If I need to disregard that PR and merge this, let me know. I will need to change the base branch to make those changes.

@steveluscher
Copy link
Author

I believe these changes have already been pulled into a working branch via #532?

Not quite! These are Mobile Wallet Adapter specific fixes that follow from that PR.

@walt-1 walt-1 changed the base branch from dev to mob-wallet-adapter-wip October 29, 2022 03:53
@walt-1
Copy link
Collaborator

walt-1 commented Oct 29, 2022

Since your changes are based on a branch that do not reflect Jordan's implementations, it is hard to decipher what changes are needed to promote. Specifically regarding the dependencies.

Jordan has changed react/types versions, solana package versions, and "@solana/wallet-adapter-base", "@solana/wallet-adapter-react", and "@solana/wallet-adapter-wallets" have more up to date versions.

Can you take a look at the package.json file with his branch being the base to sort out the dependencies/versions?

@steveluscher
Copy link
Author

Sure thing! I just mashed ‘fork’ presuming that Jordan’s changes were already merged. I’ll rebase a bit later!

@steveluscher
Copy link
Author

Done, @lukewalt! I updated this PR, with mob-wallet-adapter-wip as the base.

@steveluscher steveluscher marked this pull request as draft October 31, 2022 13:00
@walt-1
Copy link
Collaborator

walt-1 commented Oct 31, 2022

Thanks @steveluscher - I will keep an eye on the status of this PR and will merge when ready!

@steveluscher steveluscher marked this pull request as ready for review October 31, 2022 14:56
@steveluscher
Copy link
Author

Alright! I retested this and believe it's good to go. Review welcome!

@walt-1 walt-1 merged commit e83cd59 into GooseFX1:mob-wallet-adapter-wip Oct 31, 2022
@steveluscher steveluscher deleted the mwa-fixes branch November 1, 2022 00:08
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