Skip to content

Fix Popover closing unexpectedly when a Menu inside it is closed. #72376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tellthemachines merged 7 commits into from
Oct 22, 2025
Merged

Fix Popover closing unexpectedly when a Menu inside it is closed. #72376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
353c7d0
Fix Dropdown closing unexpectedly when a Menu inside it is closed.
tellthemachines Oct 16, 2025
55204b9
changelog update
tellthemachines Oct 16, 2025
869d987
Add some stories for testing
tellthemachines Oct 16, 2025
30aaec0
Check blur event in popover instead of dropdown
tellthemachines Oct 17, 2025
892f1d3
Preserve previous logic
tellthemachines Oct 17, 2025
9c3c8e9
Remove story and update changelog.
tellthemachines Oct 21, 2025
eabb81c
update changelog _again_
tellthemachines Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
### Bug Fixes

- `Button`: Ensure that icons don't shrink ([#72474](https://github.com/WordPress/gutenberg/pull/72474)).
- `Popover`: Fix `onDialogClose` logic to only close the popover if the blur event targets the specific popover instance ([#72376](https://github.com/WordPress/gutenberg/pull/72376)).

## 30.6.0 (2025-10-17)

Expand Down
25 changes: 23 additions & 2 deletions packages/components/src/popover/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,30 @@ const UnforwardedPopover = (
onDialogClose = ( type: string | undefined, event: SyntheticEvent ) => {
// Ideally the popover should have just a single onClose prop and
// not three props that potentially do the same thing.
if ( type === 'focus-outside' && onFocusOutside ) {
onFocusOutside( event );
if ( type === 'focus-outside' ) {
// Check if this blur event is actually relevant to this popover
const blurTarget = event?.target as Element;
const referenceElement = refs.reference.current;
const floatingElement = refs.floating.current;

// Check if blur is from this popover's reference element or its floating content
const isBlurFromThisPopover =
( referenceElement &&
'contains' in referenceElement &&
referenceElement.contains( blurTarget ) ) ||
floatingElement?.contains( blurTarget );
// Only proceed if the blur is actually from this popover
if ( ! isBlurFromThisPopover ) {
return;
Copy link
Copy Markdown
Member

@ramonjd ramonjd Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, what should happen to blur events that aren't from this popover? (if anything)

I was just asking myself if the logic should wrap the callbacks. E.g.,

				// Check if blur is from this popover's reference element or its floating content
				const isBlurFromThisPopover =
					( referenceElement &&
						'contains' in referenceElement &&
						referenceElement.contains( blurTarget ) ) ||
					( floatingElement &&
						floatingElement.contains( blurTarget ) );

				// Only proceed if the blur is actually from this popover
				if ( isBlurFromThisPopover ) {
					if ( onFocusOutside ) {
						onFocusOutside( event );
					} else if ( onClose ) {
						onClose();
					}
				}

But if there's no need to, then ignore me.

Copy link
Copy Markdown
Contributor Author

@tellthemachines tellthemachines Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on the assumption that if a blur happens anywhere unrelated to the popover, the popover shouldn't react to it 😅

asking myself if the logic should wrap the callbacks

It should make no difference. I prefer the early return myself as it makes it easier to read.

}
// Call onFocusOutside if defined or call onClose.
if ( onFocusOutside ) {
onFocusOutside( event );
} else if ( onClose ) {
onClose();
}
} else if ( onClose ) {
// onClose should be called for other event types if it exists.
onClose();
}
};
Expand Down
Loading