Jitsi (8x8) · jitsi-meet #17439 · Popover Esc-after-hover fix · Francesco

01 · The problem

An Esc handler that never fires

The reactions button and the three-dot overflow button each open a popover. On desktop the reactions popover opens on hover (default trigger in ToolboxButtonWithPopup), the overflow popover opens on click. In both cases, focus stays on the trigger element after open, for hover that's by design, for click it's because focus-lock activation goes through a 100 ms resize-observer dance and the user can press Esc before it kicks in.

The component had an Esc handler all along, _onEscKey, but it was attached to the .popover-content div, which gets mounted into a separate DialogPortal subtree. The trigger lives in the outer container; the content lives in the portal. Esc bubbles up from the trigger and never enters the portal's DOM, so the handler simply never fires for keyboard users.

Where the handler was wired

// Popover.web.tsx - render(), trigger side
<div
    className = { className }
    onClick = { this._onClick }
    onKeyPress = { this._onKeyPress }    // Space/Enter only
    onTouchStart = { this._onTouchStart }
    onMouseEnter = { this._onShowDialog } // hover open
    onMouseLeave = { this._onHideDialog }
    role = "button"
    tabIndex = { 0 }
    ref = { this._containerRef }>
    { visible && (
        <DialogPortal>
            <FocusOn>
                <div
                    className = "popover-content"
                    onKeyDown = { this._onEscKey }   // Esc handler is HERE,
                    ...>                              // in a separate portal subtree
                    { content }
                </div>
            </FocusOn>
        </DialogPortal>
    )}
    { children }
</div>
      

Focus on the trigger button → Esc bubbles through the outer container → no handler → event dies. The accessibility audit caught it cleanly: "Contents that open on hover, should be closable without moving focus (e.g. by pressing Esc). This is currently not the case for this menu."

02 · The fix

Move the handler one level up

Three lines change in react/features/base/popover/components/Popover.web.tsx: attach _onEscKey to the outer container so Esc bubbles up from the trigger and closes the popover; tighten the visibility guard so preventDefault/stopPropagation only runs when the popover is actually visible, so the handler doesn't swallow Esc for unrelated handlers when the popover is closed.

Diff

<div
    className = { className }
    onClick = { this._onClick }
+   onKeyDown = { this._onEscKey }
    onKeyPress = { this._onKeyPress }
    ...
    ref = { this._containerRef }>

  _onEscKey(e: React.KeyboardEvent) {
-   if (e.key === 'Escape') {
-       e.preventDefault();
-       e.stopPropagation();
-       if (this.props.visible) {
-           this._onHideDialog();
-       }
-   }
+   if (e.key === 'Escape' && this.props.visible) {
+       e.preventDefault();
+       e.stopPropagation();
+       this._onHideDialog();
+   }
  }
      

Same number of statements, just reordered so the visibility check gates the side-effecting calls instead of guarding only _onHideDialog. The outer onKeyDown on the container catches Esc whether focus is on the trigger, on the container itself (it has tabIndex=0 when trigger is hover + focusable), or anywhere inside the popover content.

Why this scope and not bigger

The same audit also filed #17368 (participants pane focus on open), #17367 + #17365 (focus into menu on open), and #17366 (focus return after closing settings). Those need work in different components, ParticipantsPane.tsx, the react-focus-on activation timing inside the popover, and BaseDialog's isElementInTheViewport guard which returns false when the toolbox auto-hides. Bundling them into this PR would conflate four root causes; the PR body lists them explicitly and offers follow-up patches.

03 · The outreach

Where this stands in the conversation funnel

PR open against master, GitHub-noreply identity from the start (CLA-friendly), tsc --noEmit -p tsconfig.web.json + eslint Popover.web.tsx both clean. Two issues explicitly Closes-tagged, four cluster siblings cross-linked.

Done

Investigation across 6 related issues

Read all six audit reports, walked OverflowMenuButton.tsx → ToolboxButtonWithPopup.tsx → Popover.web.tsx → DialogPortal.ts → BaseDialog.tsx → ParticipantsPane.tsx. Identified that #17374 + #17373 share one root cause (Popover Esc-handler location), and the other four need separate fixes in separate components. Scoped the PR honestly.

Done

Surgical patch + CI-clean

Three-line change in Popover.web.tsx. TypeScript clean on the full tsconfig.web.json project, ESLint clean on the patched file, no behaviour change for the click-trigger popovers where Esc already worked.

Done

PR open with Closes-tag + maintainer notes

Two Closes # lines, the other four issues listed as same-cluster-different-root-cause with offer to follow up. Reporter (@emrahcom) cross-linked from the patched issues so they see the fix without having to find it on the PR list.

CI run + maintainer triage

External-fork CI on jitsi-meet requires a maintainer to approve the workflow run before the lint + build pipelines fire for real. Area-owner candidates from git log -- <file>: raduanastase8x8 (most recent WCAG commit), Emmanuel Pelletier (author of #12969 "Globally improve accessibility for screen reader users"), Andrei Gavrilescu (latest Popover touch-interaction fix). Typical jitsi-meet turnaround on small a11y PRs: 3-10 days.

Goal

Merge + follow-up patches on the four siblings

Merge odds on this PR: ~70% given the +3/-4 footprint, the precise root-cause analysis, the cluster-scoping discipline, and the company-funded audit framing the reports. If this lands, the four follow-up issues become a natural conversation opener, each is a self-contained fix demonstrating the same approach.

04 · How to verify

Reproduce every claim on this page

# 1. Pull the branch from the fork
git clone https://github.com/999purple999/jitsi-meet --branch fix/popover-esc-after-hover

# 2. Open the changed file - the change is fully contained in one block
git diff master -- react/features/base/popover/components/Popover.web.tsx
# expect: +1 line onKeyDown on container, -3/+3 reordering inside _onEscKey

# 3. TS + lint on the patched file
./node_modules/.bin/tsc --noEmit -p tsconfig.web.json    # clean
./node_modules/.bin/eslint react/features/base/popover/components/Popover.web.tsx   # clean

# 4. Manual on a local build / staging deployment:
#    - hover the reactions button → popover opens
#    - press Esc → popover closes, focus stays on trigger (audit's recommended behaviour)
#    - tab to reactions button, Space to open via keyboard, Esc to close (regression check)
#    - click the three-dot menu (overflow), Esc to close (cluster sibling)
#    - press Esc with no popover open → other Esc handlers (dialog close etc) still fire
    

No unit tests added, jitsi-meet's test suite is end-to-end (wdio + a running Prosody/Jicofo/JVB stack), not unit-level, so manual verification on a deployment is the project's idiomatic test plan for UI patches of this size.