Skip to content

fix(modal): Skip ESC close when event is already handled by child#109556

Merged
TkDodo merged 2 commits intomasterfrom
tkdodo/fix/modal-closing-on-esc
Feb 27, 2026
Merged

fix(modal): Skip ESC close when event is already handled by child#109556
TkDodo merged 2 commits intomasterfrom
tkdodo/fix/modal-closing-on-esc

Conversation

@TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented Feb 27, 2026

When a CompactSelect dropdown is open inside a modal, pressing ESC closes the entire modal instead of just closing the dropdown. In drawers, this already works correctly — ESC first closes the dropdown, then on a subsequent press, closes the drawer.

The modal uses a raw document.addEventListener('keydown', ...) that unconditionally closes on any ESC press. React Aria's useOverlay (used by CompactSelect) handles ESC by calling e.preventDefault() on the native event before it reaches the document-level listener. Adding an e.defaultPrevented check to the modal's handler makes it respect events already consumed by child overlays.

When a CompactSelect dropdown is open inside a modal, pressing ESC
should only close the dropdown, not the entire modal. React Aria's
useOverlay calls preventDefault() on the native ESC event, so check
defaultPrevented before closing the modal.

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Feb 27, 2026
@TkDodo TkDodo marked this pull request as ready for review February 27, 2026 13:36
@TkDodo TkDodo requested a review from a team February 27, 2026 13:37
@TkDodo TkDodo merged commit 98349bd into master Feb 27, 2026
60 checks passed
@TkDodo TkDodo deleted the tkdodo/fix/modal-closing-on-esc branch February 27, 2026 17:12
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants