fix(popover) Close popover on Esc when focus is outside content - #17439
Open
999purple999 wants to merge 1 commit into
Open
fix(popover) Close popover on Esc when focus is outside content#17439999purple999 wants to merge 1 commit into
999purple999 wants to merge 1 commit into
Conversation
The Popover component already had an Esc handler, but it was attached only to the .popover-content div (which renders into a portal). For hover-triggered popovers (reactions menu, etc.) and for any case where focus remains on the trigger after opening, the Esc keydown never reaches the content div, so the popover cannot be closed by keyboard. Move the Esc handler to the outer container so it fires whether focus is on the trigger element or inside the popover content. Tighten the guard so the preventDefault/stopPropagation only runs when the popover is actually visible, to avoid swallowing Esc for unrelated handlers. Fixes jitsi#17374 (reactions menu Esc-after-hover) Fixes jitsi#17373 (three-dot menu Esc-after-hover)
This was referenced May 27, 2026
|
Hi, thanks for your contribution! |
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.
Summary
The
Popovercomponent declares an Esc-key handler (_onEscKey) but attaches it only to the inner.popover-contentdiv. That div renders inside aDialogPortal, so for hover-triggered popovers — and for any case where focus remains on the trigger after the popover opens — the Esckeydownevent never reaches the content handler and the popover cannot be closed by keyboard.The fix attaches the same handler to the outer container so Esc bubbles up from the trigger (or anywhere inside the container) and closes the popover. The internal guard is tightened so
preventDefault/stopPropagationonly runs when the popover is actually visible — avoiding any chance of swallowing Esc for unrelated handlers when the popover is closed.Closes
[Accessibility] ESC doesn't close the reactions menu after hover[Accessibility] ESC doesn't close the three-dot menu after hoverRelated (out of scope, but same cluster)
The same accessibility audit also reported four issues that need separate fixes in other components (
SettingsDialog,ParticipantsPane, focus-into-menu on open). I'm happy to follow up with separate PRs if this one is acceptable.participants-pane/components/web/ParticipantsPane.tsxreact-focus-onintegrationisElementInTheViewportreturns false when toolbox auto-hidesTest plan
tsc --noEmit -p tsconfig.web.json— cleaneslint react/features/base/popover/components/Popover.web.tsx— cleanmeet.jit.sibuild: hover reactions button → press Esc → menu closes (and triggering element retains focus, matching audit recommendation "closable without moving focus").Disclosure
This patch was drafted with AI assistance and reviewed by me before submission.