-
Notifications
You must be signed in to change notification settings - Fork 27
Description
Describe the bug
Why do Modals (and Dropdowns) use keyDown while Overlays use keyUp? Is there a principle or is this just happenstance?
Motivation is that if you're building a UI element that responds to the escape key, you'll need to stopPropagation of both events in order to make it safe to use inside both modals and other types of overlays. Devs on my team have rediscovered this a couple of times now.
To Reproduce
Overlays
- Go here: https://react-restart.github.io/ui/useRootClose
- Press and hold the escape key
- Overlay is still there
- Release the escape key
- Overlay goes away
Modals
- Go here: https://react-restart.github.io/ui/Modal
- Press and hold the escape key
- Modal is gone!
Expected behavior
Presumably they should both trigger off the same event. Or maybe they shouldn't!
If so, probably keyDown since bootstrap unambiguously uses that for modals & dropdowns.
Even better would be to make it configurable, but implementation of that doesn't seem straightforward.
Environment (please complete the following information)
- Operating System: macOS
- Browser, Version: chrome, 118
- react-restart Version v1.6.3 (or whatever's deployed to github pages at the moment)
Additional context
Overlay:
Line 39 in 44dc346
| const handleKeyUp = useEventCallback((e: KeyboardEvent) => { |
Modal:
Lines 399 to 407 in 44dc346
| const handleDocumentKeyDown = useEventCallback((e: KeyboardEvent) => { | |
| if (keyboard && isEscKey(e) && modal.isTopModal()) { | |
| onEscapeKeyDown?.(e); | |
| if (!e.defaultPrevented) { | |
| onHide(); | |
| } | |
| } | |
| }); |
Bootstrap Modal:
https://github.com/react-bootstrap/react-bootstrap/blob/a58a0cd6e548c653cda23ed529f3cff69ec123cc/src/Modal.tsx#L382