compose/useDialog: add stopPropagation() to Escape handler#76861
Merged
compose/useDialog: add stopPropagation() to Escape handler#76861
stopPropagation() to Escape handler#76861Conversation
When a legacy Popover (using useDialog) is open inside a Base UI Dialog, pressing Escape should only close the Popover, not bubble up and also dismiss the parent Dialog. Adding event.stopPropagation() to the keydown handler prevents this double-close behavior. Made-with: Cursor
|
Size Change: +5 B (0%) Total Size: 7.73 MB
ℹ️ View Unchanged
|
5 tasks
ciampo
changed the title
stopPropagation() to Escape handler
|
Flaky tests detected in 07aa69c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23645649384
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
adamsilverstein
pushed a commit
that referenced
this pull request
Mar 31, 2026
* Compose: Add stopPropagation to useDialog Escape handler When a legacy Popover (using useDialog) is open inside a Base UI Dialog, pressing Escape should only close the Popover, not bubble up and also dismiss the parent Dialog. Adding event.stopPropagation() to the keydown handler prevents this double-close behavior. Made-with: Cursor * CHANGELOG * Add tests * Add comment explaining current limitations with testing keyboard events * format
ObliviousHarmony
pushed a commit
that referenced
this pull request
Apr 1, 2026
* Compose: Add stopPropagation to useDialog Escape handler When a legacy Popover (using useDialog) is open inside a Base UI Dialog, pressing Escape should only close the Popover, not bubble up and also dismiss the parent Dialog. Adding event.stopPropagation() to the keydown handler prevents this double-close behavior. Made-with: Cursor * CHANGELOG * Add tests * Add comment explaining current limitations with testing keyboard events * format
adamsilverstein
pushed a commit
that referenced
this pull request
Apr 7, 2026
* Compose: Add stopPropagation to useDialog Escape handler When a legacy Popover (using useDialog) is open inside a Base UI Dialog, pressing Escape should only close the Popover, not bubble up and also dismiss the parent Dialog. Adding event.stopPropagation() to the keydown handler prevents this double-close behavior. Made-with: Cursor * CHANGELOG * Add tests * Add comment explaining current limitations with testing keyboard events * format
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Part of #76487
Extracted from #76837
Add
event.stopPropagation()to the Escape key handler in theuseDialoghook from@wordpress/compose.Why?
When a legacy Popover (which uses
useDialoginternally) is rendered inside a Base UIDialog, pressing Escape currently bubbles up and closes both the Popover and the parent Dialog in a single keystroke.This happens because the Popover's keydown handler calls
event.preventDefault()but notevent.stopPropagation(), so the Escape event continues to propagate up the DOM tree where Base UI's Dialog picks it up and also dismisses itself.The expected behavior is that Escape closes only the innermost overlay — pressing Escape once should close the Popover, and pressing it a second time should close the Dialog.
How?
Added
event.stopPropagation()immediately afterevent.preventDefault()in thecloseOnEscapeRefcallback withinuseDialog. This prevents the Escape keydown event from bubbling past the Popover to parent overlay listeners.Regression risk assessment
The
stopPropagation()only fires whenonCloseis providedThe guard clause (
currentOptions.current?.onClose) ensures that if noonClosehandler is set (e.g., the DataViews dropdown), the Escape event still propagates normally — no behavior change.Legacy Modal already checks
defaultPreventedThe
@wordpress/componentsModal'shandleEscapeKeyDownalready checks!event.defaultPrevented, so the existingpreventDefault()call already prevents it from double-closing. However,stopPropagation()adds a stronger guarantee — the event never even reaches the Modal's handler. All 27 Modal unit tests still pass, including the nested modal Escape stacking test.All 91 Popover tests still pass
Confirming no regression for the primary consumer of
useDialog.Base UI Dialog doesn't check
defaultPreventedIt listens for Escape independently. This is precisely the scenario the fix addresses: without
stopPropagation(), a Popover inside a Base UI Dialog would see both close on a single Escape. With it, only the innermost overlay closes.Nested
useDialoginstances work correctlyIf the inner dialog has
onClose, it handles Escape and stops propagation so the outer dialog doesn't fire. If the inner dialog has noonClose, the event propagates naturally to the outer one.Testing Instructions
Testing Instructions for Keyboard
Same as above — the entire test is keyboard-driven.
Use of AI Tools
Cursor + Claude Opus 4.6