diff --git a/packages/eui/changelogs/upcoming/9103.md b/packages/eui/changelogs/upcoming/9103.md new file mode 100644 index 00000000000..fb157575100 --- /dev/null +++ b/packages/eui/changelogs/upcoming/9103.md @@ -0,0 +1,3 @@ +**Accessibility** + +- Fixed an issue where portalled components like `EuiPopover` were not included in `EuiFlyout`'s focus trap through `includeSelectorInFocusTrap`, making them inaccessible to keyboard users diff --git a/packages/eui/src/components/flyout/flyout.spec.tsx b/packages/eui/src/components/flyout/flyout.spec.tsx index 8dcb04436c1..a40bbf61d1f 100644 --- a/packages/eui/src/components/flyout/flyout.spec.tsx +++ b/packages/eui/src/components/flyout/flyout.spec.tsx @@ -11,18 +11,27 @@ /// import React, { useRef, useState } from 'react'; +import { css } from '@emotion/react'; +import { EuiButton } from '../button'; +import { EuiCollapsibleNav, EuiCollapsibleNavGroup } from '../collapsible_nav'; +import { EuiCollapsibleNavBeta } from '../collapsible_nav_beta'; +import { EuiFlexGroup } from '../flex'; +import { EuiFlyout } from './flyout'; +import { EuiFlyoutBody } from './flyout_body'; +import { EuiFlyoutHeader } from './flyout_header'; import { EuiGlobalToastList } from '../toast'; import { EuiHeader, EuiHeaderSection, EuiHeaderSectionItemButton, } from '../header'; -import { EuiCollapsibleNavBeta } from '../collapsible_nav_beta'; -import { EuiFlyout } from './flyout'; -import { EuiCollapsibleNav, EuiCollapsibleNavGroup } from '../collapsible_nav'; import { EuiIcon } from '../icon'; -import { EuiButton } from '../button'; +import { EuiPanel } from '../panel'; +import { EuiPopover } from '../popover'; +import { EuiText } from '../text'; +import { EuiTitle } from '../title'; +import { useEuiTheme } from '../../services'; const childrenDefault = ( <> @@ -251,6 +260,7 @@ describe('EuiFlyout', () => { }: { children?: React.ReactNode; collapsibleNavVariant?: 'beta' | 'default'; + includeFixedHeadersInFocusTrap?: boolean; }) => { const [isOpen, setIsOpen] = useState(false); const [navIsOpen, setNavIsOpen] = useState(false); @@ -453,4 +463,107 @@ describe('EuiFlyout', () => { cy.get(':root').cssVar(euiPushFlyoutOffsetInlineEnd).should('not.exist'); }); }); + + describe('Focus trap shards', () => { + const INCLUDE_IN_FLYOUT_TRAP_FOCUS_DATA_ATTRIBUTE = + 'data-include-in-flyout-trap-focus'; + const INCLUDE_IN_FLYOUT_TRAP_FOCUS = { + prop: { + [INCLUDE_IN_FLYOUT_TRAP_FOCUS_DATA_ATTRIBUTE]: 'true', + }, + attribute: INCLUDE_IN_FLYOUT_TRAP_FOCUS_DATA_ATTRIBUTE, + selector: `[${INCLUDE_IN_FLYOUT_TRAP_FOCUS_DATA_ATTRIBUTE}="true"]`, + }; + + const FlyoutWithPopoverShard = () => { + const { euiTheme } = useEuiTheme(); + + const [isFlyoutOpen, setIsFlyoutOpen] = useState(false); + const [isPopoverOpen, setIsPopoverOpen] = useState(false); + + return ( + <> + + + setIsFlyoutOpen(true)}> + Toggle flyout + + setIsPopoverOpen(true)}> + Toggle popover + + } + isOpen={isPopoverOpen} + panelProps={{ + ...INCLUDE_IN_FLYOUT_TRAP_FOCUS.prop, + 'data-test-subj': 'popover-panel', + }} + > + setIsPopoverOpen(false)} + > + Confirm action + + setIsPopoverOpen(false)} + > + Destructive action + + + + + {isFlyoutOpen && ( + setIsFlyoutOpen(false)} + includeSelectorInFocusTrap={[ + INCLUDE_IN_FLYOUT_TRAP_FOCUS.selector, + ]} + > + + +

Popover composition

+
+
+ + +

These popovers are portalled outside the flyout DOM.

+
+
+
+ )} + + ); + }; + + it('includes popover panels in the focus trap when opened', () => { + cy.mount(); + + // 1. Open the flyout. + cy.realPress('Tab'); + cy.realPress('Enter'); + cy.get('[data-test-subj="flyoutSpec"]').should('be.focused'); + + // 2. Open the popover. + cy.repeatRealPress('Tab', 4); + cy.realPress('Enter'); + cy.get('[data-test-subj="popover-panel"]').should('exist'); + + // 3. Tab from the popover trigger into the popover content. + cy.realPress('Tab'); + cy.get('[data-test-subj="popover-confirm-action"]').should('be.focused'); + }); + }); }); diff --git a/packages/eui/src/components/flyout/flyout.tsx b/packages/eui/src/components/flyout/flyout.tsx index b8336a8d434..f2cec400546 100644 --- a/packages/eui/src/components/flyout/flyout.tsx +++ b/packages/eui/src/components/flyout/flyout.tsx @@ -33,6 +33,7 @@ import { useEuiMemoizedStyles, useGeneratedHtmlId, useEuiThemeCSSVariables, + focusTrapPubSub, } from '../../services'; import { logicalStyle } from '../../global_styling'; @@ -400,26 +401,45 @@ export const EuiFlyout = forwardRef( return selectors; }, [includeSelectorInFocusTrap, includeFixedHeadersInFocusTrap]); - useEffect(() => { - if (focusTrapSelectors.length > 0) { - const shardsEls = focusTrapSelectors.flatMap((selector) => - Array.from(document.querySelectorAll(selector)) - ); + /** + * Finds the shards to include in the focus trap by querying by `focusTrapSelectors`. + * + * @param shouldAutoFocus Whether to auto-focus the flyout wrapper when the focus trap is activated. + * This is necessary because when a flyout is toggled from within a shard, the focus trap's `autoFocus` + * feature doesn't work. This logic manually focuses the flyout as a workaround. + */ + const findShards = useCallback( + (shouldAutoFocus: boolean = false) => { + if (focusTrapSelectors.length > 0) { + const shardsEls = focusTrapSelectors.flatMap((selector) => + Array.from(document.querySelectorAll(selector)) + ); + + setFocusTrapShards(Array.from(shardsEls)); + + if (shouldAutoFocus) { + 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] + ); - setFocusTrapShards(Array.from(shardsEls)); + useEffect(() => { + // Auto-focus should only happen on initial flyout mount (or when the dependencies change) + // because it snaps focus to the flyout wrapper, which steals it from subsequently focused elements. + findShards(true); - // 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]); + const unsubscribe = focusTrapPubSub.subscribe(() => findShards()); + return unsubscribe; + }, [findShards]); const focusTrapProps: EuiFlyoutProps['focusTrapProps'] = useMemo( () => ({ diff --git a/packages/eui/src/components/popover/popover.tsx b/packages/eui/src/components/popover/popover.tsx index e178f3dcabc..3b7d07fbdac 100644 --- a/packages/eui/src/components/popover/popover.tsx +++ b/packages/eui/src/components/popover/popover.tsx @@ -29,6 +29,7 @@ import { getWaitDuration, performOnFrame, htmlIdGenerator, + focusTrapPubSub, } from '../../services'; import { setMultipleRefs } from '../../services/hooks/useCombinedRefs'; @@ -448,6 +449,7 @@ export class EuiPopover extends Component { this.repositionTimeout = window.setTimeout(() => { this.setState({ isOpenStable: true }, () => { this.positionPopoverFixed(); + focusTrapPubSub.publish(); }); }, durationMatch + delayMatch); }; @@ -492,6 +494,7 @@ export class EuiPopover extends Component { this.setState({ isClosing: false, }); + focusTrapPubSub.publish(); }, closingTransitionTime); } } @@ -502,6 +505,7 @@ export class EuiPopover extends Component { clearTimeout(this.strandedFocusTimeout); clearTimeout(this.closingTransitionTimeout); cancelAnimationFrame(this.closingTransitionAnimationFrame!); + focusTrapPubSub.publish(); } onMutation = (records: MutationRecord[]) => { diff --git a/packages/eui/src/services/focus_trap/focus_trap_pub_sub.test.ts b/packages/eui/src/services/focus_trap/focus_trap_pub_sub.test.ts new file mode 100644 index 00000000000..093ea59a874 --- /dev/null +++ b/packages/eui/src/services/focus_trap/focus_trap_pub_sub.test.ts @@ -0,0 +1,56 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { focusTrapPubSub } from './focus_trap_pub_sub'; + +describe('focusTrapPubSub', () => { + let unsubscribeAll: Array<() => void> = []; + + afterEach(() => { + unsubscribeAll.forEach((unsubscribe) => unsubscribe()); + unsubscribeAll = []; + }); + + it('subscribes a listener and calls it on publish', () => { + const listener = jest.fn(); + + unsubscribeAll.push(focusTrapPubSub.subscribe(listener)); + focusTrapPubSub.publish(); + + expect(listener).toHaveBeenCalledTimes(1); + }); + + it('does not call the listener after it has been unsubscribed', () => { + const listener = jest.fn(); + const unsubscribe = focusTrapPubSub.subscribe(listener); + + unsubscribe(); + focusTrapPubSub.publish(); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('can handle multiple subscribers and unsubscribes them independently', () => { + const listener1 = jest.fn(); + const listener2 = jest.fn(); + + const unsubscribe1 = focusTrapPubSub.subscribe(listener1); + + unsubscribeAll.push(focusTrapPubSub.subscribe(listener2)); + focusTrapPubSub.publish(); + + expect(listener1).toHaveBeenCalledTimes(1); + expect(listener2).toHaveBeenCalledTimes(1); + + unsubscribe1(); + focusTrapPubSub.publish(); + + expect(listener1).toHaveBeenCalledTimes(1); + expect(listener2).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/eui/src/services/focus_trap/focus_trap_pub_sub.ts b/packages/eui/src/services/focus_trap/focus_trap_pub_sub.ts new file mode 100644 index 00000000000..422b3f6aba4 --- /dev/null +++ b/packages/eui/src/services/focus_trap/focus_trap_pub_sub.ts @@ -0,0 +1,73 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +type Listener = () => void; + +const listeners: Set = new Set(); + +/** + * Subscribes a listener function to be called whenever focus trap updates are published. + * + * @param listener The function to be called on updates. + * @returns A function that, when called, will unsubscribe the listener. Please remember + * to call this function for proper cleanup. + * @example + * ```tsx + * useEffect(() => { + * const unsubscribe = focusTrapPubSub.subscribe(() => { + * console.log('focus trap updated'); + * }); + * + * return () => unsubscribe(); + * }, []); + * ``` + */ +const subscribe = (listener: Listener) => { + listeners.add(listener); + + return () => unsubscribe(listener); +}; + +/** + * Unsubscribes a listener from the focus trap PubSub service. + * + * @param listener The function to unsubscribe. + */ +const unsubscribe = (listener: Listener) => { + listeners.delete(listener); +}; + +/** + * Publishes an event to all subscribed listeners, signaling that + * components managing focus traps should re-evaluate their tracked elements. + */ +const publish = () => { + listeners.forEach((listener) => listener()); +}; + +/** + * A lightweight, global PubSub service for loose coupling of components + * that need to interact with the same focus trap. + * + * This allows a component (like `EuiPopover`) to be rendered in a React Portal + * and still be included in the focus trap of another component (like `EuiFlyout`) + * without either component needing a direct reference to the other. + * + * How it works: + * + * 1. A container component (e.g., `EuiFlyout`) `subscribe`s to this service on mount. + * 2. An ephemeral component (e.g., `EuiPopover`) calls `publish` when its state + * changes in a way that affects the DOM (e.g., opening, closing, unmounting). + * 3. The container component's subscribed callback fires, causing it to re-query + * the DOM for any elements it should include in its focus trap. + */ +export const focusTrapPubSub = { + subscribe, + unsubscribe, + publish, +}; diff --git a/packages/eui/src/services/focus_trap/index.ts b/packages/eui/src/services/focus_trap/index.ts new file mode 100644 index 00000000000..4ac02dc9c75 --- /dev/null +++ b/packages/eui/src/services/focus_trap/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +export { focusTrapPubSub } from './focus_trap_pub_sub'; diff --git a/packages/eui/src/services/index.ts b/packages/eui/src/services/index.ts index cc6ac652606..ed2589649de 100644 --- a/packages/eui/src/services/index.ts +++ b/packages/eui/src/services/index.ts @@ -77,6 +77,7 @@ export * from './console'; export * from './copy'; export * from './emotion'; export * from './findElement'; +export { focusTrapPubSub } from './focus_trap'; export { dateFormatAliases, formatAuto,