Components: Popover: don't close when focus moves into the @wordpress/ui compat overlay slot#78407
Open
ciampo wants to merge 6 commits into
Open
Components: Popover: don't close when focus moves into the @wordpress/ui compat overlay slot#78407ciampo wants to merge 6 commits into
@wordpress/ui compat overlay slot#78407ciampo wants to merge 6 commits into
Conversation
…verlay slot `@wordpress/components` `Popover` uses `useFocusOutside` to close itself when focus leaves its subtree. `@wordpress/ui` overlays (e.g. `Select`) portal their popup to the body-level `[data-wp-compat-overlay-slot]` container and move real DOM focus into it (via base-ui's `FloatingFocusManager` + `useListNavigation` with `focusItemOnHover`), which `Popover` was incorrectly interpreting as focus leaving. Treat focus moves whose `relatedTarget` is inside the compat overlay slot as "still inside" so nested `@wordpress/ui` overlays don't dismiss their host `Popover`. Fixes #78406.
|
Size Change: +81 B (0%) Total Size: 7.97 MB 📦 View Changed
ℹ️ View Unchanged
|
When a portaled descendant (e.g. a `@wordpress/ui` `Select` popup) closes and restores DOM focus back to a Popover-internal element (such as its trigger), the blur event has: - `target` = the unmounting portaled element (outside the Popover's DOM subtree) - `relatedTarget` = the Popover-internal element receiving focus The existing focus-outside heuristic only checked containment for `event.target`, so this looked like focus leaving the Popover and dismissed it. Also ignore blur events whose `relatedTarget` is inside the floating element. Refs #78406.
…storation When a portaled descendant closes via an outside-press on a body-level backdrop (e.g. base-ui's `Select` `modal: true` `InternalBackdrop`), the sequence is: 1. mousedown on the (non-focusable) backdrop blurs the currently focused element to `body`, so the captured blur event has `relatedTarget = null` and the active element briefly becomes `body`. 2. The dismissal handler closes the popup and unmounts it. 3. base-ui's `FloatingFocusManager` synchronously restores focus to the popup's trigger inside the host popover. By the time `useFocusOutside`'s deferred blur-check runs, focus is back inside the popover, but the captured `relatedTarget` no longer reflects that. Inspect `ownerDocument.activeElement` at evaluation time as a fallback and bail out if it's inside the floating element. Refs #78406.
|
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. |
|
Flaky tests detected in f5afbec. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26044911538
|
mirka
approved these changes
May 18, 2026
Member
mirka
left a comment
mirka
left a comment
There was a problem hiding this comment.
I also noticed a UX issue with the Autocomplete, where an Escape key press in the input will immediately close the @wordpress/components Popover (correct UX is to first close the Autocomplete popover, then clear the input, then close the parent popover).
Not a huge problem because I don't think it's a common pattern, and probably easier to just use a @wordpress/ui Popover when it comes to it.
| document.body.removeChild( slot ); | ||
| } ); | ||
|
|
||
| it( 'should not call onFocusOutside when focus returns from the slot to a popover descendant', async () => { |
Member
There was a problem hiding this comment.
I think this test may not exercise the intended branch. Since focus starts on slotButton, then user.click( slotButton ) does not move focus out of the popover, and trigger.focus() only moves focus into the popover afterward. That means the popover's focus-outside path may never run here.
Contributor
Author
|
I have to look further into it, but I suspect that's a separate issue, where the escape key press bubbles up and causes the popover to close. |
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?
See #78406. Scoped fix for the most visible manifestation:
@wordpress/uiSelectnested in aPopovercauses thePopoverto close on hover and on any dismissal.Why?
Popovercloses viauseFocusOutside, which only checks DOM containment.Select's popup (and any portaled descendant that takes real DOM focus) lives outside thePopover's DOM subtree, so it looks indistinguishable from focus leaving and thePopovercloses prematurely.The general bug — and existing per-consumer workarounds elsewhere in the codebase — is tracked in #78406.
How?
In
Popover'sfocus-outsidehandler, ignore blur events when any of the following are true:relatedTargetis inside[data-wp-compat-overlay-slot].relatedTargetis inside the popover's floating element.document.activeElement(at evaluation time) is inside the popover's floating element.Why three checks?
Each covers a different dismissal path:
FloatingFocusManagerrestores focus to the trigger, captured asrelatedTarget.bodyfirst (sorelatedTargetisnull), and onlyactiveElementqueried later reflects the restored focus.Autocompleteisn't affected because it keeps DOM focus on its input (combobox +aria-activedescendant).Why scoped, not the general fix?
A general fix needs a way for
useFocusOutsideto know which portaled subtrees are logically inside its target — likely via a React-context registry or an opt-in API. Larger surface to design; tracked in #78406.Testing Instructions
npm run storybook:devPlayground/WP Compat Overlay Slot→Inside Components Popover.@wordpress/uiSelect. Then, in turn: hover items, select one, press Esc, click the modal backdrop. The hostPopovershould stay open in every case.Popoveritself → it should close (regression check).Unit tests:
npx jest -c test/unit/jest.config.js packages/components/src/popover.Testing Instructions for Keyboard
Same flow as above, opening the
Selectwith Space/Enter, highlighting with Arrows, dismissing with Esc / Enter.Screenshots or screencast
Kapture.2026-05-18.at.18.10.33.mp4
Kapture.2026-05-18.at.18.11.41.mp4
Use of AI Tools
Drafted with AI assistance; reviewed and verified manually (storybook reproduction + new unit tests).