-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
fix: display all available chains & check wallet VM when buying / minting nft #10720
Conversation
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@jarsen can you include #10796 in this pr ? likely after #10772.
that should fix
|
I've thought the same as you. We could show all available chains on the selector. Whenever the users do something on-chain operation(buy/mint), we will check if the current chain matches the wallet. If not, it will show the reconnect wallet popup modal. It would be the easiest way to handle multiple chains. cc @exezbcz |
@Jarsen136 yeah this way you are only delaying the reconnect modal to the last possible moment, i am for this. Right now there was the reconnect only when clicking the drop card we can change it later if we realize it is not optimal for now it will be quite aligned with the multi account thesis |
cc @hassnian |
@exezbcz s-works? anything that is missing? |
(params as ExecuteSubstrateTransactionParams).cb, | ||
arg, | ||
{ | ||
doAfterCheckCurrentChainVM(() => { |
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.
I get this approach but I'm not too sold on adding doAfterCheckCurrentChainVM
inside useTransaction
I'd rather show the modal before the action has started.
maybe because this is simplest solution atm?
@preschian wdyt
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.
I'd rather show the modal before the action has started.
maybe because this is simplest solution atm?
Yes, it's the simplest solution, then we don't need to add this check everywhere.
although I have added checks for most cases such as mint/drop/buy, I leave it here to make sure it is impossible to do on-chain operations with an incorrect wallet.
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.
spent some time doing qa
code has changed quite a bit, re-requesting @preschian review

@exezbcz need your input
- now that we have all chains, what should we do here, allow only the current vm wallet or both, same for the drop page
- we should remove that modal here and allow reconnect inside the drop (already working)
-
wallet connect modal selects the a defualt vm chain after the wallet is connected, pls remove that , otherwise you can do stuff like this
minting a collection on base
CleanShot.2024-08-15.at.10.44.51.mp4
- unrelated this pr (new issue pls) , drop page can't change prefix , it's kinda weird allowing to change the chain and changing it back
CleanShot.2024-08-15.at.10.49.31.mp4
Thanks for taking the time to test it!
-> 1, 2
-> 3, 4
will create another issue. |
thanks |
cc @exezbcz |
yeah this sounds like it will be the easiest for a second i though making it dynamic based on where the user currently is but that would bring even more complexity so to answer:
lets show both, with the "current VM tab" preselected
yeah lets delay the reconnect till the last possible moment thanks for testing @hassnian and thank you for the implementation @Jarsen136 ! |
✅ Updated |
what is left here? can you review please @preschian ? |
|
Thank you for your contribution to the Koda - Generative Art Marketplace.
👇 __ Let's make a quick check before the contribution.
PR Type
Needs QA check
Context
Display all available chains no matter what kind of wallet(SUB/EVM) you are connected to.
Did your issue had any of the "$" label on it?
Screenshot 📸