Skip to content

[9.2] [SideNav] Fix popover vs flyout trap focus (#238230)#238925

Merged
kibanamachine merged 1 commit intoelastic:9.2from
kibanamachine:backport/9.2/pr-238230
Oct 14, 2025
Merged

[9.2] [SideNav] Fix popover vs flyout trap focus (#238230)#238925
kibanamachine merged 1 commit intoelastic:9.2from
kibanamachine:backport/9.2/pr-238230

Conversation

@kibanamachine
Copy link
Copy Markdown
Contributor

Backport

This will backport the following commits from main to 9.2:

Questions ?

Please refer to the Backport tool documentation

## Summary

Currently, there is an issue with the popover interactiveness when
there's a flyout with a focus trap on the screen.

https://github.com/user-attachments/assets/9a94d3da-d468-43df-b16e-bb544f318e9c

The changes on this PR patch this behavior in the side nav by only
calling the blur callback if the focus is not trapped by the flyout.

> [!WARNING]
> This is not a **long-term** solution. It's a quick and dirty fix so
that things work but it gives us more time to fix the issue meaningfully
without affecting the users.

## Details

`EuiFlyout` wraps its contents in an [internal focus
trap](https://github.com/elastic/eui/blob/ed0c2c0b73239c06766eca1225101032d2a6287b/packages/eui/src/components/flyout/flyout.tsx#L529-L552).

The moment you click into the popover, that trap yanks focus back into
the flyout. Because `handleBlur` runs on the wrapper div, it sees the
blur event, notices that `relatedTarget` is no longer inside the trigger
or the popover, and immediately calls `handleClose`. From the
user perspective the popover isn't interactive because any click
sends focus to the flyout -> the blur handler fires -> the popover
closes.

When we remove `handleBlur`, we stop clearing `anyPopoverOpen` on blur.
That global flag then stays `true` and so the delayed `handleMouseEnter`
/ `tryOpen` logic bails out (`tryOpen` won’t reopen while
it thinks another popover is active; it's purpose is to avoid toggling
several popovers at once when navigating with a keyboard). As a result
the popover never appears when you focus the trigger via keyboard.

So with `handleBlur` present the focus trap keeps closing the popover.
Without it the open state never resets, blocking keyboard-triggered
opens.

`euiIncludeSelectorInFocusTrap` doesn't work (specifically
`panelProps={euiIncludeSelectorInFocusTrap.prop}` on `EuiPopover`). It
looks like the focus trap captures matched elements only when the flyout
opens but the popover panel is added later via a portal so it’s missed.

Making the popover be inserted right next to the trigger and not
portalled makes the popover a) not visible at all in the fixed layout,
b) not position correctly in the grid layout.

## QA

After the changes were applied:

https://github.com/user-attachments/assets/769b07c3-0657-457f-87fd-8e6410ce7e4d

### Checklist

- [ ] Verify that the keyboard navigation works as expected
  - [ ] I can navigate with tab and arrow keys
  - [ ] The popover opens on trigger focus
- [ ] ~I can press "Enter" to move focus to the popover~ (these do not
work in this case)
- [ ] ~I can press "Escape" to move focus back to the trigger~ (these do
not work in this case)
- [ ] Verify that on a page with a flyout (e.g. Discover > Open session
- folder icon) mouse clicks work as expected
  - [ ] I can hover over a trigger and the popover shows
  - [ ] I can click on an item in the popover and it's triggered
- [ ] I can click on an item with submenu in "More" menu and open the
nested panel

(cherry picked from commit 9ab2151)
@kibanamachine kibanamachine requested a review from a team as a code owner October 14, 2025 14:47
@kibanamachine kibanamachine added the backport This PR is a backport of another PR label Oct 14, 2025
@kibanamachine kibanamachine enabled auto-merge (squash) October 14, 2025 14:47
@kibanamachine kibanamachine merged commit fddb1eb into elastic:9.2 Oct 14, 2025
16 checks passed
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
core 132.7KB 132.8KB +58.0B

cc @weronikaolejniczak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants