feat(eui): add a focus trap pubsub service

[weronikaolejniczak]

Summary

Changes:

• adds an internal focus trap PubSub service,
• publishes the event from EuiPopover on opening, closing and unmounting the component,
• subscribes to the event in EuiFlyout, re-queries and updates the focusTrapShards.

Why are we making this change?

Resolves #9099

tl;dr;

To fix keyboard navigation for elements that show up later than the flyout renders and so are not queried statically with includeSelectorInFocusTrap.

More context

The includeSelectorInFocusTrap option was originally introduced to support the grid layout and ensure it works correctly with the flyout:

• [EuiFlyout] add includeSelectorInFocusTrap #8849

The issue is that the Side Nav popovers appear after the flyout renders and evaluates includeSelectorInFocusTrap. As a result, the selector isn’t accounted for in the focus trap, leading to focus getting lost:

500894863-d25031df-c550-4291-91d3-a4efdc3df717.mp4

Implementation details

This is the core of includeSelectorInFocusTrap functionality:

eui/packages/eui/src/components/flyout/flyout.tsx

Lines 387 to 422 in d788096

	const focusTrapSelectors = useMemo(() => {
	let selectors: string[] = [];
	if (includeSelectorInFocusTrap) {
	selectors = Array.isArray(includeSelectorInFocusTrap)
	? includeSelectorInFocusTrap
	: [includeSelectorInFocusTrap];
	}
	if (includeFixedHeadersInFocusTrap) {
	selectors.push('.euiHeader[data-fixed-header]');
	}
	return selectors;
	}, [includeSelectorInFocusTrap, includeFixedHeadersInFocusTrap]);
	useEffect(() => {
	if (focusTrapSelectors.length > 0) {
	const shardsEls = focusTrapSelectors.flatMap((selector) =>
	Array.from(document.querySelectorAll<HTMLElement>(selector))
	);
	setFocusTrapShards(Array.from(shardsEls));
	// Flyouts that are toggled from shards do not have working
	// focus trap autoFocus, so we need to focus the flyout wrapper ourselves
	shardsEls.forEach((shard) => {
	if (shard.contains(flyoutToggle.current)) {
	resizeRef?.focus();
	}
	});
	} else {
	// Clear existing shards if necessary, e.g. switching to `false`
	setFocusTrapShards((shards) => (shards.length ? [] : shards));
	}
	}, [focusTrapSelectors, resizeRef]);

The update reuses the same logic, passing it as a callback to the PubSub subscribe function. This lets the flyout detect when a publisher (e.g. EuiPopover) prompts a re-query of elements with includeSelectorInFocusTrap.

PubSub assures clear separation between EuiPopover and EuiFlyout without linking them in any way together.

Alternative approaches

1. onKeyDown:
   Causes a race condition. Key events fire before the popover renders, making focus updates asynchronous and unreliable (in Storybook it required a double Enter, in Kibana it lost focus). It could be made better by delaying the re-queries by popover transition but that is a fragile patch.

2. MutationObserver:
   Detects DOM changes but is costly and can hurt performance if not debounced. Can significantly impact the page load.

3. React Context API:
   Works but adds unnecessary complexity and scope issues (requires global EuiProvider, otherwise is limited to a certain subtree).

Always publish

Always publishing from EuiPopover is the correct design here. A conditional prop would create a leaky abstraction, forcing developers to understand and manually connect the internal mechanics of EuiPopover and EuiFlyout.

The current approach keeps the components decoupled: the popover announces its state and the flyout listens. The performance cost of this is negligible and ensures the components work together automatically.

Impact to users

🟢 This should not cause regression to Flyout trap focus and keyboard navigation. It should improve keyboard navigation in side nav and flyout composition in Kibana. No API changes.

QA

Specific checklist

• Reproduction story works as expected:
  • Popover not included in the focus trap loses focus
  • Popover included in the focus trap handles focus correctly

Popover keyboard navigation:

• Enter to enter the popover on trigger focus
• Esc to escape the popover on content focus

Before merge:

• chore: add repro story is removed

Testing in Kibana

• [EUI] Test local EUI with flyout focus trap fix kibana#239500 ✅

General checklist

• Browser QA
  • Checked in both light and dark modes
  • Checked in both MacOS and Windows high contrast modes
    • (emulate forced colors if you do not have access to a Windows machine.)
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Checked for accessibility including keyboard-only and screenreader modes
• Docs site QA
  • Added documentation
  • Props have proper autodocs (using @default if default values are missing) and playground toggles
  • Checked Code Sandbox works for any docs examples
• Code quality checklist
  • Added or updated jest and cypress tests
  • Updated visual regression tests
• Release checklist
  • A changelog entry exists and is marked appropriately.
  • If applicable, added the breaking change issue label (and filled out the breaking change checklist)
• Designer checklist
  • If applicable, file an issue to update EUI's Figma library with any corresponding UI changes. (This is an internal repo, if you are external to Elastic, ask a maintainer to submit this request)

[weronikaolejniczak]

WILL BE REVERTED

_{sorry for screaming}

[acstll]

I went through the QA steps provided and it's working as expected. The changes look good and the implementation is neat and clean (agree it's probably the best compared to the other possible approaches). Left a couple of comments.

We agreed with @weronikaolejniczak that it'll make sense to test this with the latest Flyout System changes as well, and that @tkajtoch should also take a look as well 🙂

[acstll]

JSDoc FTW, thanks for adding this 💚

[acstll]

Thanks again for taking the time to address my comments @weronikaolejniczak

I'm approving knowing that we agree

• @tkajtoch will also do a review
• it will get tested in Kibana (with a test PR like elastic/kibana#239500 and CI green) after the current work for integrating the flyout system is stable

[tkajtoch]

I like what I see in this PR very much. The solution seems thought out and doesn't open us to any new risks.

My comments are just comments and I don't expect you to change anything. LGTM 🚢

[tkajtoch]

[Not a change request] Semantically, this implements the observer pattern and not the pub-sub pattern, but the differences are minimal. It doesn't change the fact that this solution works here.

[weronikaolejniczak]

I would say this is a hybrid approach, technically it's closer to Observer but conceptually it's closer to Pub/Sub. Why?

• publish() does not know who the listeners are, it just broadcasts,
• focusTrapPubSub acts as a mediator (the "message broker"), components call publish and subscribe on it,
• Flyout and Popover communicate without references to one another,
• the publisher doesn’t maintain observers tied to its own instance, it uses a shared event channel.

In the Observer pattern, the "subject" holds the list of observers. Here, no object owns the list, the listeners Set is a global event bus. This means multiple independent parts of the app can publish and subscribe. For now it's only Flyout being the subscriber and Popover the publisher but we could loop in other components as needed. Isn't that a Pub/Sub?

I'm genuinely curious, please let me know what was your thought process 😄

EDIT: One more thing that came to mind, I always thought of Observer as a tightly-coupling pattern while Pub/Sub as a loosely-coupling pattern, which is why Pub/Sub is great to use in a micro-frontend architecture for cross-module behaviors 👍🏻

EDIT 2: I guess what differs it from a true Pub/Sub is the "topics" (here we don't need them), and external message broker. Also the focusTrapPubSub is an in-memory singleton that synchronously loops through listeners, like an Observer subject. This is so nuanced, I don't think it's wrong to call it either Pub/Sub or Observer but the intention for this skews more for Pub/Sub 😅

[tkajtoch]

Just a thought: Even since we can't do anything with react-focus-on's shards property that accepts type Array<React.RefObject<any> | HTMLElement>, it would still be possible to make it dynamic based on the usage. The library only accesses the data on two events - onMouseDown and the internal onNodeActivation. That means shards is only read then, which makes it a potential good candidate for a Proxy array that would work similarly to HTMLCollection but using querySelectorAll. Whenever the array would be read, it would read a dynamic value based on the output of document.querySelectorAll at the time of read.

I'm not saying this would be a better solution. I'm just mentioning it as a way to bend the rules :D

The best overall solution would likely be to open a feature request and allow shards to be a function or something more dynamic in general for these use cases.

[weronikaolejniczak]

I love the idea of a Proxy array, it's very clever! 🧠 And removes the need for a subscription system.

I quickly whipped it up in a7faab2 and tested with the story:

Kapture.2025-11-07.at.12.03.24.mp4

We can always drop the commit in case we feel this is too risky (e.g. potential performance overhead due to frequent queries or the reliance of how react-focus-on handles shards array). I feel this would have to be re-tested thoroughly because it does affect other parts of the flyout code.

Lmk what you think, personally I'd revert to the pub/sub for potentially better performance and no compatibility risk but Proxy is a more elegant solution with less component coupling and less code to maintain.

[tkajtoch]

Huge props for actually testing this and comparing both solutions! My primary intention was to just mention it and not push you in either direction. Your solution works perfectly fine and thanks to its simplicity I feel confident we can merge it and be able to enhance it if we ever need to in the future. The proxy solution might be useful someday, somewhere else 👍🏻

[weronikaolejniczak]

For prosperity, the diff would be sth like:

const focusTrapSelectors = useMemo(() => {
  let selectors: string[] = [];
  if (includeSelectorInFocusTrap) {
    selectors = Array.isArray(includeSelectorInFocusTrap)
      ? includeSelectorInFocusTrap
      : [includeSelectorInFocusTrap];
  }
  if (includeFixedHeadersInFocusTrap) {
    selectors.push('.euiHeader[data-fixed-header]');
  }
  return selectors;
}, [includeSelectorInFocusTrap, includeFixedHeadersInFocusTrap]);

const dynamicShards = useMemo(() => {
  const userShards = _focusTrapProps?.shards || [];

  if (focusTrapSelectors.length === 0) {
    return userShards;
  }

  const getDynamicNodes = () =>
    focusTrapSelectors.flatMap((selector) =>
      Array.from(document.querySelectorAll<HTMLElement>(selector))
    );

  return new Proxy(userShards, {
    get(target, prop) {
      const allNodes = [
        ...target, // consumer-provided shards
        ...getDynamicNodes(),
      ];
      const value = Reflect.get(allNodes, prop);

      if (typeof value === 'function') {
        return value.bind(allNodes);
      }

      return value;
    },
  });
}, [_focusTrapProps?.shards, focusTrapSelectors]);

const focusTrapProps: EuiFlyoutProps['focusTrapProps'] = useMemo(
  () => ({
    ..._focusTrapProps,
    shards: dynamicShards,
  }),
  [_focusTrapProps, dynamicShards]
);

[acstll]

it was amazing watching this work in a7faab2 💪

I must say it's the first time I see Proxy being practically useful (besides the typical reactivity implementations in UI libraries) 💚

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: eed4fe4
• Documentation website
• Storybook

History

• 💚 Build #1852 succeeded a7faab2
• 💚 Build #1838 succeeded 7785a57
• 💚 Build #1836 succeeded 228bdda
• 💚 Build #1805 succeeded 077d68f
• 💚 Build #1793 succeeded 55de887
• 💚 Build #1792 succeeded 6d3c47c

cc @weronikaolejniczak

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: eed4fe4

History

• 💔 Build #5155 failed a7faab2
• 💚 Build #5143 succeeded 7785a57
• 💚 Build #5141 succeeded 228bdda
• 💚 Build #5123 succeeded 077d68f
• 💚 Build #5111 succeeded 55de887
• 💔 Build #5109 failed 695e5b1

cc @weronikaolejniczak