-
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
feat: revamp particleAdapter
#854
Conversation
No top level dependency changes detected. Learn more about Socket for GitHub ↗︎ |
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.
Thanks, much easier to review. Still have some questions, primary about the configuration interface.
Co-authored-by: Jordan <jordan@jordansexton.com>
GM @jordaaash! Anything else you'd like to be done here? Happy to make any other changes; let me know 👍 |
Just checking in here again @jordaaash |
Hey @jordaaash! Happy New Year! Just wanted to bump this and see if there was anything else that I could do here to push it through? |
Catching up on this now. Sorry for the delay! |
No problem at all, thank you! |
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.
Okay, a few small changes left. I enabled the build to run, but it looks broken. Can you commit the pnpm lockfile changes? I also ran the build locally, and it fails with this error:
@solana/wallet-adapter-particle:build: ../../../node_modules/.pnpm/@particle-network+solana-wallet@1.2.1_@particle-network+auth@0.5.6_@solana+web3.js@1.78.0_bs58@4.0.1/node_modules/@particle-network/solana-wallet/lib/types/solana-wallet.d.ts(1,16): error TS2305: Module '"@particle-network/auth"' has no exported member 'LoginOptions'.
Just pushed changes addressing those additional comments and fixed the build issue - I went ahead and added my pnpm lockfile to the commit to. The build is working locally for me, could you check again on your side? Additionally, important to note that this change adds an additional dev dependency to the adapter package, |
Followup PR to fix and release this @ #877 |
Released via #878 |
As an alternative to the previous PR, this purely updates the adapter to function with the new version of Particle's
solana-wallet
SDK, restoring standard functionality to the adapter (which it currently lacks due to the outdated structure andsolana-wallet
version).This removes the previous social login display customization changes & abides by the 1 SDK rule.