Skip to content
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

Escape Key Closes Modals onKeyDown and Overlays onKeyUp #96

Open
dleavitt opened this issue Nov 3, 2023 · 0 comments
Open

Escape Key Closes Modals onKeyDown and Overlays onKeyUp #96

dleavitt opened this issue Nov 3, 2023 · 0 comments

Comments

@dleavitt
Copy link

dleavitt commented Nov 3, 2023

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

  1. Go here: https://react-restart.github.io/ui/useRootClose
  2. Press and hold the escape key
  3. Overlay is still there
  4. Release the escape key
  5. Overlay goes away

Modals

  1. Go here: https://react-restart.github.io/ui/Modal
  2. Press and hold the escape key
  3. 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:

const handleKeyUp = useEventCallback((e: KeyboardEvent) => {

Modal:

ui/src/Modal.tsx

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant