-
Notifications
You must be signed in to change notification settings - Fork 70
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
Open OnBoardingModal
for modals requiring a membership
#4859
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Hi @hlvhe. Thank you for the contribution. This approach works as expected when no wallet is connected but when a wallet is connected but no membership is selected the switch membership still should show. The way I see it the "OnBoardingModal" should not replace the "SwitchMemberModal", the 2 cases should somehow be discriminated.
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.
Here's the flow when you don't have a wallet connected now:
Screencast from 2024-07-29 09-31-44.webm
It's definitely an improvement compare to showing an empty membership switching modal. However the user still has to click the action once to pick a wallet then nothing happen until they click the action again.
Ideally they would:
- pick there wallet
- pick there membership
- start the action they clicked on in the first place (e.g announce a candidacy)
To do this the "original modal" would have to be passed to the onboarding modal too, and this "original modal" should be handled there. Which might make the onboarding logic even more complex than it already is.
@kdembler WDYT ? Is this flow good enough for now ?
@thesan I can't get into details on this right now but if the PR behaviour is not ideal but still improves what we have currently, then that's good enough for me! |
Closes #4648