-
Notifications
You must be signed in to change notification settings - Fork 972
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 docs for wallet standard #761
base: master
Are you sure you want to change the base?
Conversation
|
No dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No dependency changes detected in pull request Pull request alert summary
|
6016141
to
9b7ceb0
Compare
Thanks for the initiative. I did a quick glance over this, but will need to do a more complete review. The adapters have not been deprecated, so I think this may be jumping the gun a bit. Some adapters are for wallets that need to load a third party library. There are also commonly-used wallets like Solflare which have not implemented the Wallet Standard, and others like Phantom which have, but the adapter provides functionality on iOS that isn't handled by MWA. I imagine most of these adapters could be refactored so that the app instead loads a library which registers a Standard Wallet with the app, and there's no need for the |
Got it - that sounds completely reasonable re: Solflare not being ready and Phantom providing extra hardware support on iOS. It's a more nuanced situation than what I originally thought. Will the instructions in My concern after using https://github.com/solana-labs/dapp-scaffold (which I know is a separate Solana Labs repo, but linked to from here) is that new dApps won't support wallet-standard, which makes it harder for new wallets to be created on Solana and also wallets from other chains to add Solana support. |
No, all Standard Wallets and Mobile Wallet Adapter are supported in the React It would be cool if you didn't have to pass any adapters at all though, and instead you just called something like e.g. It should be pretty straightforward to refactor most of the adapters to this. I'm all for (eventually) deprecating/EOL-ing all adapters that could use the Wallet Standard directly though, since they really should do that instead of making every app bundle their dependencies. |
1ab8740
to
b93af5e
Compare
This is a smaller PR that makes some aspects of the docs more apparent but, per discussion, doesn't mark individual wallet adapters as deprecated. - State what wallet adapter and wallet standard are in the README - Mention pnpm 8 requirement explicitly (I'm relatively sure pnpm 7 won't work) - Make Starter Projects more prominent by moving it before the long list of individual wallet adapters - Make demo and demo code more prominent in README - Use relative links wherever possible (which keeps links working across forks and looks neater) - Use a neater syntax so the markdown is viewable in terminal, VScode, and other apps that do not render markdown.
Ah it does say that in a comment in the code, I just missed it. Thanks! I've replaced the changes here with a much smaller version that just adds some additional information about wallet adapter per our discussion, and doesn't mark individual adapters ads deprecated. It should hopefully be useful to anyone else implementing wallet adapter in future, see the updated PR description. |
Update
This is a smaller PR that makes some aspects of the docs more apparent but, per discussion, doesn't mark individual wallet adapters as deprecated.
See what the docs look like after this PR is merged
Original comment
Rewrite docs to focus on wallet-standard, and note that wallet adapters for individual wallet apps are deprecatedMake demo and demo code more prominent in READMEMove deprecated individual wallet adapter links into DEPRECATED.mdMove current instructions from PACKAGES.md and WALLET.md (now that PACKAGES is much smaller) to main READMEAdd notes that some community packages need updated for wallet-standardUse relative links wherever possible (which keeps links working across forks)Use a neater syntax (lists rather than markdown tables) so the markdown is viewable in terminal, VScode, and other apps that do not render markdown.Remove APP.md as it doesn't use wallet-standard (we have link to demo code for something that does use wallet-standard)