search

theKashey / react-focus-on

🎯 Solution for WAI ARIA compatible modal dialogs or full-screen tasks, you were looking for
MIT License
336 stars 14 forks source link

Escape key should be bound on keydown rather than keyup #15

Closed benoitgrelard closed 4 years ago

benoitgrelard commented 4 years ago

Hi @theKashey,

Looking at what the OS does for all types of modal layers (things like save dialog, confirmation dialogs, open menus, open selects, etc) close on escape always executes instantly as soon as touching the escape key, on keydown basically.

https://github.com/theKashey/react-focus-on/blob/master/src/Effect.tsx#L66

We are bound to keyup here which puts us a bit behind in terms of timing, and offers a different UX.

I believe we should also bind to keydown rather than keyup?

Small change, but I think offers much better consistency. Let me know your thoughts, I can do a PR if needed, although I doubt you need one considering the change.

Cheers 👍

theKashey commented 4 years ago

Double checked with a real modals - and, you are right, it's bound to keydown.And I would greatly appreciate if you draft a PR.

benoitgrelard commented 4 years ago

Cool, just sent a PR: https://github.com/theKashey/react-focus-on/pull/16

theKashey commented 4 years ago

Released as 3.0.7