-
Notifications
You must be signed in to change notification settings - Fork 12
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(modal): listen on mousedown instead of click event #146
Conversation
✅ Deploy Preview for sefirot-story ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for sefirot-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Codecov ReportBase: 35.12% // Head: 35.12% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
=======================================
Coverage 35.12% 35.12%
=======================================
Files 95 95
Lines 7880 7880
Branches 126 126
=======================================
Hits 2768 2768
Misses 5112 5112
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Thanks a lot!
Yeah we could, though I think we can do it in other PR. Maybe we need to be careful a bit. If we have for example dropdown menu input on modal, and maybe user might hit Not sure how other modals out there handle such cases 🤔 |
Yeah, I was just searching the same thing 😅. Directly adding event listener to document likely won't work. |
Maybe each input must catch |
@kiaking all element events bubble. Though this is a user land decision, not something Sefirot will know about 👍 |
fixes #145
For accessibility we can also add an event listener on keyup for escape.