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

Capture escape key in combobox and listbox so it doesn't close form #2557

Closed
david-crespo opened this issue Nov 14, 2024 · 2 comments · Fixed by #2610
Closed

Capture escape key in combobox and listbox so it doesn't close form #2557

david-crespo opened this issue Nov 14, 2024 · 2 comments · Fixed by #2610
Assignees
Milestone

Comments

@david-crespo
Copy link
Collaborator

david-crespo commented Nov 14, 2024

Pressing escape in a combobox to close the popover currently gets you out of the entire form. #2328 or similar would block the form from closing, which is an improvement, but really we shouldn't even see that popup if we're just pressing escape to close a popover. So let's consider this a separate issue from #2186.

@david-crespo david-crespo added this to the 12 milestone Nov 14, 2024
@david-crespo
Copy link
Collaborator Author

david-crespo commented Dec 10, 2024

This is not easy! It seems like the combobox should be preventing the Esc from bubbling up (headless source), but as far as I can tell, the combobox is not even seeing the event, which suggests the side modal form dialog is getting it first and eating it.

@david-crespo
Copy link
Collaborator Author

Ok, I was able to figure this out thanks to Chrome's UI for looking at all event listeners relevant to an element:

Screenshot of Chrome dev tools

Image

The reason the modal dialog is getting the event first is that Radix's useKeyDownEvent uses capture: true (MDN, TIL), which means the handler runs during the capture phase (moving down the tree to the target), which happens before the other handlers.

Claude 3.5 Sonnet explaining propagation phases
  1. Propagation Phases:

    • Capture Phase: top → down (parent to target)
    • Target Phase: at the element
    • Bubbling Phase: bottom → up (target to parent)
  2. Order Determinants:

    • Event listeners with capture: true run first
    • Non-captured listeners run during bubbling
    • Within the same element, listeners run in registration order
    • React's event system generally attaches listeners at the root, but maintains synthetic propagation matching these rules
    • stopPropagation() prevents further propagation
    • stopImmediatePropagation() prevents other handlers on same element

React's event delegation can sometimes make the actual order less obvious because all events are attached to the root, but the synthetic event system simulates natural DOM propagation.

This was added in radix-ui/primitives#2761 in response to radix-ui/primitives#2653.

This is hooked up as follows: Radix Dialog.Content uses DismissableLayer, which calls useEscapeKeyDown. This means we can try to preempt the event handler in certain cases by passing onEscapeKeyDown to dialog content in the implementation of SideModal.

onKeyDown={(e) => {
  console.log('modal content keydown', e)
}}
onEscapeKeyDown={(e) => {
  console.log('modal content esc keydown', e)
  if (e.target instanceof HTMLInputElement) {
    console.log({ open: e.target.dataset.open !== undefined })
    // e.preventDefault()
  }
}}

We can detect that the escape keypress comes from a relevant input, but it all feels rather backwards. More importantly, calling preventDefault also prevents the combobox close options event listener from running, so it's a no go. There may be a way to make this work, but I wanted to switch to Headless dialog anyway, so maybe this is a good excuse to do that.

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

Successfully merging a pull request may close this issue.

1 participant