Dropdown: Close popover when focus moves into an unrelated dialog - #82170
Dropdown: Close popover when focus moves into an unrelated dialog#82170wprashed wants to merge 9 commits into
Conversation
When the accordion content style was changed to support in-browser search via `hidden="until-found"` (WordPress#74744), the browser applies `content-visibility: hidden`. Because `content-visibility: hidden` hides contents without removing the container box model, any user- or theme-defined padding on the Accordion Panel block produced an empty gap below the title when the accordion was closed. This resets `padding-block: 0 !important` on `.wp-block-accordion-panel[hidden]` so the closed panel occupies no vertical space while preserving the full padding when the panel is open. Fixes WordPress#65900 (Trac WordPress#65900).
Addresses review feedback by ensuring borders and box shadows are also reset when the accordion panel is hidden, along with padding.
Fixes WordPress#82124 When a Dropdown popover is open and focus moves into a `[role="dialog"]`, Dropdown previously preserved presence for any dialog on the page, even if the dialog was launched from an unrelated element (such as clicking 'Edit code' on the Custom HTML toolbar). This updates Dropdown to track whether the last user interaction (pointerdown or focusin) occurred inside the Dropdown container/popover. If focus moves into a dialog that was NOT launched from inside the dropdown, the popover closes cleanly.
|
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. |
dca3257 to
4f3fb40
Compare
| const popoverRef = useRef< HTMLDivElement >( null ); | ||
| const mergedPopoverRef = useMergeRefs( [ popoverRef, popoverProps?.ref ] ); |
There was a problem hiding this comment.
popoverProps.ref is excluded by the public type, so this currently fails type checking. It should not be added solely for internal bookkeeping. The interaction origin can instead be tracked through capture handlers on the Dropdown wrapper, which also receive events from the portaled Popover. This removes the need for popoverRef and useMergeRefs.
| // Ignore focusin events landing inside a [role="dialog"], because | ||
| // focus moving into a dialog (e.g. on modal mount) is the consequence | ||
| // of an action, not the user interaction that triggered it. | ||
| if ( | ||
| event.type === 'focusin' && | ||
| ( target as Element ).closest?.( '[role="dialog"]' ) | ||
| ) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The handler ignores all focus entering a dialog. But a keyboard user can therefore Tab into an existing unrelated non-modal dialog and leave the dropdown open.
We should preserve the dropdown only when the dialog focus follows an activation from inside it (and ideally cover this keyboard path in the tests)
| expect( screen.queryByText( 'test' ) ).not.toBeInTheDocument(); | ||
| } ); | ||
|
|
||
| it( 'should close the dropdown when focus moves into an unrelated dialog', async () => { |
There was a problem hiding this comment.
some feedback:
- I believe this external-dialog test also would also pass on
trunkwithout this PR's fix. includeHiddenis unsupported and fails type checking. We could usehidden: trueor assertonCloseand DOM removal.- We should also verify that the related dialog returns focus to its dropdown trigger after closing.
After these changes, let's confirm the test fails on the base and passes with the fix.
|
|
||
| ### Bug Fixes | ||
|
|
||
| - `Dropdown`: Close popover when focus moves to an unrelated dialog outside the dropdown ([#82124](https://github.com/WordPress/gutenberg/issues/82124)). |
There was a problem hiding this comment.
THe changelog should link to this PR
| - `Dropdown`: Close popover when focus moves to an unrelated dialog outside the dropdown ([#82124](https://github.com/WordPress/gutenberg/issues/82124)). | |
| - `Dropdown`: Close popover when focus moves to an unrelated dialog outside the dropdown ([#82170](https://github.com/WordPress/gutenberg/issues/82170)). |
|
@wprashed let me know if you're still able to work on this PR, otherwise I'll take over. |
|
Continued in #82371 with all review feedback addressed (capture event handlers, keyboard Tab support, complete test suite in |
Fixes #82124
Description
When a
Dropdownpopover is open (e.g. "Transform to" block switcher popover on Custom HTML block) and the user opens an unrelated dialog (e.g. clicking "Edit code" on the toolbar), theDropdownpreviously kept the popover stuck open behind the modal dialog.Cause
Dropdown'scloseIfFocusOutsidechecked if focus moved into any[role="dialog"], treating any dialog on the page as a reason to preserve presence. However, it couldn't distinguish whether the dialog was launched from inside the dropdown itself (e.g. Preferences modal from Options menu) vs launched by an unrelated element elsewhere on the page.Solution
This PR updates
Dropdownto:pointerdownorfocusin) occurred inside theDropdowncontainer or popover.focusinevents landing directly inside a[role="dialog"], because focus moving into a dialog on mount is the consequence of an action, not the user interaction that triggered it.Testing Instructions
<h1>Test</h1>).