-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add Modal #321
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/oacore/design/jx1xp4bno |
@viktor-yakubiv what do you think? Do we need anything else? (closable icon in the right top corner, animations...) |
It's too wide to my eye but works nice |
I agree especially if the terms and conditions have 80characters per line. Let me change that. |
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.
It's done very well. It makes me think about a few things related to the usage of the popup.
Perhaps modals must be auto-closable with 'Escape' always with automatic choosing less dangerous action, e.g. 'cancel' or 'no', and should not be used where such an action cannot be chosen.
Also, the mobile view and behaviour are not obvious to me.
However, I don't mind shipping this as the first iteration.
No description provided.