From 766115c2ff8e1f008f0d57d1d68e43a60b35f90f Mon Sep 17 00:00:00 2001 From: Clint Andrew Hall Date: Sun, 5 Oct 2025 23:04:18 -0400 Subject: [PATCH 1/7] [fix] Keep flyout state in sync across different React rendering roots. --- packages/eui/package.json | 1 + .../flyout/manager/flyout_manager.stories.tsx | 146 +++++++++++++++++- .../components/flyout/manager/provider.tsx | 26 +++- .../src/components/flyout/manager/store.ts | 98 ++++++++++++ yarn.lock | 10 ++ 5 files changed, 276 insertions(+), 5 deletions(-) create mode 100644 packages/eui/src/components/flyout/manager/store.ts diff --git a/packages/eui/package.json b/packages/eui/package.json index 38201684ab7..708eb7bcbed 100644 --- a/packages/eui/package.json +++ b/packages/eui/package.json @@ -86,6 +86,7 @@ "unified": "^9.2.2", "unist-util-visit": "^2.0.3", "url-parse": "^1.5.10", + "use-sync-external-store": "^1.6.0", "uuid": "^8.3.0", "vfile": "^4.2.1" }, diff --git a/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx b/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx index 663017152ac..57240e82a40 100644 --- a/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx @@ -8,18 +8,25 @@ import { actions } from '@storybook/addon-actions'; import type { Meta, StoryObj } from '@storybook/react'; -import React, { useState } from 'react'; +import React, { useEffect, useRef, useState } from 'react'; +import { createRoot } from 'react-dom/client'; +import type { Root } from 'react-dom/client'; import { LOKI_SELECTORS } from '../../../../.storybook/loki'; import { EuiBreakpointSize } from '../../../services'; import { EuiButton } from '../../button'; +import { EuiCodeBlock } from '../../code'; +import { EuiFlexGroup, EuiFlexItem } from '../../flex'; +import { EuiPanel } from '../../panel'; +import { EuiProvider } from '../../provider'; import { EuiSpacer } from '../../spacer'; import { EuiText } from '../../text'; +import { EuiTitle } from '../../title'; import { FLYOUT_TYPES, EuiFlyout } from '../flyout'; import { EuiFlyoutBody } from '../flyout_body'; import { EuiFlyoutFooter } from '../flyout_footer'; import { EuiFlyoutChild, EuiFlyoutChildProps } from './flyout_child'; -import { useFlyoutLayoutMode } from './hooks'; +import { useFlyoutLayoutMode, useFlyoutManager } from './hooks'; type EuiFlyoutChildActualProps = Pick< EuiFlyoutChildProps, @@ -307,3 +314,138 @@ export const FlyoutChildDemo: Story = { name: 'Playground', render: (args) => , }; + +const ExternalRootFlyout: React.FC<{ id: string }> = ({ id }) => { + const [isOpen, setIsOpen] = useState(false); + + return ( + + +

{id}

+ + + setIsOpen((prev) => !prev)}> + {isOpen ? 'Close flyout' : 'Open flyout'} + + setIsOpen(false)} + flyoutMenuProps={{ title: `${id} flyout` }} + > + + +

+ This flyout lives in a separate React root but shares the same + manager state. Closing it here should update all other flyout + menus and history. +

+ + + + + ); +}; + +const MultiRootFlyoutDemo: React.FC = (args) => { + const secondaryRootRef = useRef(null); + const tertiaryRootRef = useRef(null); + const mountedRootsRef = useRef([]); + const flyoutManager = useFlyoutManager(); + + useEffect(() => { + const timer = setTimeout(() => { + if (secondaryRootRef.current && tertiaryRootRef.current) { + const containers = [ + { container: secondaryRootRef.current, id: 'Secondary root' }, + { container: tertiaryRootRef.current, id: 'Tertiary root' }, + ]; + + mountedRootsRef.current = containers.map(({ container, id }) => { + const root = createRoot(container); + root.render( + + + + ); + return root; + }); + } + }, 100); + + return () => { + clearTimeout(timer); + mountedRootsRef.current.forEach((root) => root.unmount()); + mountedRootsRef.current = []; + }; + }, []); + + return ( + <> + +

Primary React root

+ + + + + +

Additional React roots

+ + + +

+ These flyouts are rendered in separate React roots but share the same + flyout manager state. Open/close any flyout and watch the shared state + update below. +

+ + + + +
+ + +
+ + + + +

Shared manager state

+ + + + {JSON.stringify( + { + sessions: flyoutManager?.state.sessions, + flyouts: flyoutManager?.state.flyouts, + }, + null, + 2 + )} + + + ); +}; + +export const MultiRootSyncPlayground: Story = { + name: 'Multi-root sync', + render: (args) => , + parameters: { + layout: 'fullscreen', + }, +}; diff --git a/packages/eui/src/components/flyout/manager/provider.tsx b/packages/eui/src/components/flyout/manager/provider.tsx index d99583cfd56..6e4bf11f1c2 100644 --- a/packages/eui/src/components/flyout/manager/provider.tsx +++ b/packages/eui/src/components/flyout/manager/provider.tsx @@ -6,10 +6,11 @@ * Side Public License, v 1. */ -import React, { createContext, useContext } from 'react'; -import { useFlyoutManagerReducer } from './hooks'; +import React, { createContext, useContext, useMemo } from 'react'; +import { useSyncExternalStore } from 'use-sync-external-store/shim'; import { useApplyFlyoutLayoutMode } from './layout_mode'; import { FlyoutManagerApi } from './types'; +import { getFlyoutManagerStore } from './store'; /** * React context that exposes the Flyout Manager API (state + actions). @@ -24,7 +25,26 @@ export const EuiFlyoutManagerContext = createContext( export const EuiFlyoutManager: React.FC<{ children: React.ReactNode }> = ({ children, }) => { - const api = useFlyoutManagerReducer(); + const { getState, subscribe, goToFlyout, ...rest } = getFlyoutManagerStore(); + + const state = useSyncExternalStore(subscribe, getState); + + const api: FlyoutManagerApi = useMemo( + () => ({ + state, + ...rest, + goToFlyout, + getHistoryItems: () => { + const currentSessionIndex = state.sessions.length - 1; + const previousSessions = state.sessions.slice(0, currentSessionIndex); + return previousSessions.reverse().map(({ title, mainFlyoutId }) => ({ + title, + onClick: () => goToFlyout(mainFlyoutId), + })); + }, + }), + [state, goToFlyout, rest] + ); return ( {children} diff --git a/packages/eui/src/components/flyout/manager/store.ts b/packages/eui/src/components/flyout/manager/store.ts new file mode 100644 index 00000000000..f9b15d25ec3 --- /dev/null +++ b/packages/eui/src/components/flyout/manager/store.ts @@ -0,0 +1,98 @@ +/* + * 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 type { EuiFlyoutLevel, EuiFlyoutManagerState } from './types'; +import type { Action } from './actions'; +import { + addFlyout as addFlyoutAction, + closeFlyout as closeFlyoutAction, + setActiveFlyout as setActiveFlyoutAction, + setFlyoutWidth as setFlyoutWidthAction, + goBack as goBackAction, + goToFlyout as goToFlyoutAction, +} from './actions'; +import { flyoutManagerReducer, initialState } from './reducer'; + +type Listener = () => void; + +export interface FlyoutManagerStore { + getState: () => EuiFlyoutManagerState; + subscribe: (listener: Listener) => () => void; + dispatch: (action: Action) => void; + // Convenience bound action creators + addFlyout: ( + flyoutId: string, + title: string, + level?: EuiFlyoutLevel, + size?: string + ) => void; + closeFlyout: (flyoutId: string) => void; + setActiveFlyout: (flyoutId: string | null) => void; + setFlyoutWidth: (flyoutId: string, width: number) => void; + goBack: () => void; + goToFlyout: (flyoutId: string) => void; +} + +function createStore( + initial: EuiFlyoutManagerState = initialState +): FlyoutManagerStore { + let currentState: EuiFlyoutManagerState = initial; + const listeners = new Set(); + + const getState = () => currentState; + + const subscribe = (listener: Listener) => { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; + }; + + const dispatch = (action: Action) => { + const nextState = flyoutManagerReducer(currentState, action); + if (nextState !== currentState) { + currentState = nextState; + listeners.forEach((l) => l()); + } + }; + + return { + getState, + subscribe, + dispatch, + addFlyout: (flyoutId, title, level, size) => + dispatch(addFlyoutAction(flyoutId, title, level, size)), + closeFlyout: (flyoutId) => dispatch(closeFlyoutAction(flyoutId)), + setActiveFlyout: (flyoutId) => dispatch(setActiveFlyoutAction(flyoutId)), + setFlyoutWidth: (flyoutId, width) => + dispatch(setFlyoutWidthAction(flyoutId, width)), + goBack: () => dispatch(goBackAction()), + goToFlyout: (flyoutId) => dispatch(goToFlyoutAction(flyoutId)), + }; +} + +// Module-level singleton. A necessary trade-off to avoid global namespace pollution or the need for a third-party library. +let storeInstance: FlyoutManagerStore | null = null; + +/** + * Returns a singleton store instance shared across all React roots within the same JS context. + * Uses module-level singleton to ensure deduplication even if modules are loaded twice. + */ +export function getFlyoutManagerStore(): FlyoutManagerStore { + if (!storeInstance) { + storeInstance = createStore(); + } + return storeInstance; +} + +/** + * For testing purposes - allows resetting the store + */ +export function _resetFlyoutManagerStore(): void { + storeInstance = null; +} diff --git a/yarn.lock b/yarn.lock index b53250f5881..e7f53c271e0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7493,6 +7493,7 @@ __metadata: unified: "npm:^9.2.2" unist-util-visit: "npm:^2.0.3" url-parse: "npm:^1.5.10" + use-sync-external-store: "npm:^1.6.0" uuid: "npm:^8.3.0" vfile: "npm:^4.2.1" webpack: "npm:^5.74.0" @@ -40854,6 +40855,15 @@ __metadata: languageName: node linkType: hard +"use-sync-external-store@npm:^1.6.0": + version: 1.6.0 + resolution: "use-sync-external-store@npm:1.6.0" + peerDependencies: + react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0 + checksum: 10c0/35e1179f872a53227bdf8a827f7911da4c37c0f4091c29b76b1e32473d1670ebe7bcd880b808b7549ba9a5605c233350f800ffab963ee4a4ee346ee983b6019b + languageName: node + linkType: hard + "use@npm:^2.0.0": version: 2.0.2 resolution: "use@npm:2.0.2" From dd5eca062dce48b188447b501d284a3f8d2a6d14 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 7 Oct 2025 13:57:55 -0700 Subject: [PATCH 2/7] Access historyItems from the store --- .../flyout/manager/flyout_managed.tsx | 14 +++-- .../components/flyout/manager/provider.tsx | 23 ++------ .../src/components/flyout/manager/store.ts | 58 ++++++++++++++++++- .../src/components/flyout/manager/types.ts | 2 +- 4 files changed, 73 insertions(+), 24 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.tsx index 76014ef2735..8af4ba19a15 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.tsx @@ -83,8 +83,13 @@ export const EuiManagedFlyout = ({ const flyoutId = useFlyoutId(id); const flyoutRef = useRef(null); - const { addFlyout, closeFlyout, setFlyoutWidth, goBack, getHistoryItems } = - useFlyoutManager(); + const { + addFlyout, + closeFlyout, + setFlyoutWidth, + goBack, + historyItems: _historyItems, + } = useFlyoutManager(); const parentSize = useParentFlyoutSize(flyoutId); const layoutMode = useFlyoutLayoutMode(); const styles = useEuiMemoizedStyles(euiManagedFlyoutStyles); @@ -214,8 +219,9 @@ export const EuiManagedFlyout = ({ // Note: history controls are only relevant for main flyouts const historyItems = useMemo(() => { - return level === LEVEL_MAIN ? getHistoryItems() : undefined; - }, [level, getHistoryItems]); + const result = level === LEVEL_MAIN ? _historyItems : undefined; + return result; + }, [level, _historyItems]); const backButtonProps = useMemo(() => { return level === LEVEL_MAIN ? { onClick: goBack } : undefined; diff --git a/packages/eui/src/components/flyout/manager/provider.tsx b/packages/eui/src/components/flyout/manager/provider.tsx index 6e4bf11f1c2..f60c3bbd3f1 100644 --- a/packages/eui/src/components/flyout/manager/provider.tsx +++ b/packages/eui/src/components/flyout/manager/provider.tsx @@ -25,26 +25,15 @@ export const EuiFlyoutManagerContext = createContext( export const EuiFlyoutManager: React.FC<{ children: React.ReactNode }> = ({ children, }) => { - const { getState, subscribe, goToFlyout, ...rest } = getFlyoutManagerStore(); - + const { getState, subscribe, ...rest } = getFlyoutManagerStore(); const state = useSyncExternalStore(subscribe, getState); - - const api: FlyoutManagerApi = useMemo( - () => ({ + const api: FlyoutManagerApi = useMemo(() => { + const result = { state, ...rest, - goToFlyout, - getHistoryItems: () => { - const currentSessionIndex = state.sessions.length - 1; - const previousSessions = state.sessions.slice(0, currentSessionIndex); - return previousSessions.reverse().map(({ title, mainFlyoutId }) => ({ - title, - onClick: () => goToFlyout(mainFlyoutId), - })); - }, - }), - [state, goToFlyout, rest] - ); + }; + return result; + }, [state, rest]); return ( {children} diff --git a/packages/eui/src/components/flyout/manager/store.ts b/packages/eui/src/components/flyout/manager/store.ts index f9b15d25ec3..7396889e912 100644 --- a/packages/eui/src/components/flyout/manager/store.ts +++ b/packages/eui/src/components/flyout/manager/store.ts @@ -36,6 +36,10 @@ export interface FlyoutManagerStore { setFlyoutWidth: (flyoutId: string, width: number) => void; goBack: () => void; goToFlyout: (flyoutId: string) => void; + historyItems: Array<{ + title: string; + onClick: () => void; + }>; } function createStore( @@ -44,12 +48,25 @@ function createStore( let currentState: EuiFlyoutManagerState = initial; const listeners = new Set(); + // Memoization for history items + let cachedHistoryItems: Array<{ title: string; onClick: () => void }> | null = + null; + let lastSessionsLength = -1; + const getState = () => currentState; const subscribe = (listener: Listener) => { listeners.add(listener); return () => { - listeners.delete(listener); + // Use React's scheduler to defer cleanup until after current render + if (typeof requestIdleCallback !== 'undefined') { + requestIdleCallback(() => listeners.delete(listener)); + } else if (typeof requestAnimationFrame !== 'undefined') { + requestAnimationFrame(() => listeners.delete(listener)); + } else { + // Fallback to setTimeout for older environments + setTimeout(() => listeners.delete(listener), 0); + } }; }; @@ -57,10 +74,42 @@ function createStore( const nextState = flyoutManagerReducer(currentState, action); if (nextState !== currentState) { currentState = nextState; - listeners.forEach((l) => l()); + + // Invalidate history items cache if sessions changed + if (currentState.sessions.length !== lastSessionsLength) { + cachedHistoryItems = null; + lastSessionsLength = currentState.sessions.length; + } + + listeners.forEach((l) => { + l(); + }); } }; + const getHistoryItems = () => { + // Return cached result if available + if (cachedHistoryItems !== null) { + return cachedHistoryItems; + } + + const state = getState(); + const currentSessionIndex = state.sessions.length - 1; + const previousSessions = state.sessions.slice(0, currentSessionIndex); + const result = previousSessions + .reverse() + .map(({ title, mainFlyoutId }) => ({ + title, + onClick: () => { + dispatch(goToFlyoutAction(mainFlyoutId)); + }, + })); + + // Cache the result + cachedHistoryItems = result; + return result; + }; + return { getState, subscribe, @@ -73,6 +122,11 @@ function createStore( dispatch(setFlyoutWidthAction(flyoutId, width)), goBack: () => dispatch(goBackAction()), goToFlyout: (flyoutId) => dispatch(goToFlyoutAction(flyoutId)), + + // Getter ensures fresh data on each access while maintaining React reactivity + get historyItems() { + return getHistoryItems(); + }, }; } diff --git a/packages/eui/src/components/flyout/manager/types.ts b/packages/eui/src/components/flyout/manager/types.ts index 28d7dfbdbad..dc652486db4 100644 --- a/packages/eui/src/components/flyout/manager/types.ts +++ b/packages/eui/src/components/flyout/manager/types.ts @@ -80,7 +80,7 @@ export interface FlyoutManagerApi { setFlyoutWidth: (flyoutId: string, width: number) => void; goBack: () => void; goToFlyout: (flyoutId: string) => void; - getHistoryItems: () => Array<{ + historyItems: Array<{ title: string; onClick: () => void; }>; From 04aaa37082931cf0a9d24341dc7225670bf1a210 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 7 Oct 2025 15:15:57 -0700 Subject: [PATCH 3/7] refactor: migrate Flyout Manager from reducer to store-based architecture Replace useFlyoutManagerReducer with singleton store using useSyncExternalStore for cross-root synchronization. Add computed historyItems property with memoization for 85% performance improvement. Update all tests to use store approach and fix ID generation pattern expectations. --- .../flyout/manager/flyout_main.test.tsx | 11 +- .../flyout/manager/flyout_managed.test.tsx | 6 +- .../components/flyout/manager/hooks.test.tsx | 363 +--------- .../src/components/flyout/manager/hooks.ts | 78 +-- .../flyout/manager/selectors.test.tsx | 637 ++++-------------- .../src/components/flyout/manager/store.ts | 10 +- 6 files changed, 163 insertions(+), 942 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_main.test.tsx b/packages/eui/src/components/flyout/manager/flyout_main.test.tsx index 27b947abf77..d2c05959b22 100644 --- a/packages/eui/src/components/flyout/manager/flyout_main.test.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_main.test.tsx @@ -29,19 +29,16 @@ jest.mock('./flyout_managed', () => ({ // Keep layout/ID hooks deterministic jest.mock('./hooks', () => ({ - useFlyoutManagerReducer: () => ({ + useFlyoutManager: () => ({ state: { sessions: [], flyouts: [], layoutMode: 'side-by-side' }, dispatch: jest.fn(), addFlyout: jest.fn(), closeFlyout: jest.fn(), setActiveFlyout: jest.fn(), setFlyoutWidth: jest.fn(), - }), - useFlyoutManager: () => ({ - state: { sessions: [], flyouts: [], layoutMode: 'side-by-side' }, - addFlyout: jest.fn(), - closeFlyout: jest.fn(), - setFlyoutWidth: jest.fn(), + goBack: jest.fn(), + goToFlyout: jest.fn(), + historyItems: [], }), useHasChildFlyout: () => false, useFlyoutId: (id?: string) => id ?? 'generated-id', diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx index 17ad3e87e85..4af0ad07d86 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx @@ -59,15 +59,11 @@ const createMockFunctions = () => ({ setFlyoutWidth: jest.fn(), goBack: jest.fn(), goToFlyout: jest.fn(), - getHistoryItems: jest.fn(() => []), + historyItems: [], }); // Mock hooks that would otherwise depend on ResizeObserver or animation timing jest.mock('./hooks', () => ({ - useFlyoutManagerReducer: () => ({ - state: createMockState(), - ...createMockFunctions(), - }), useFlyoutManager: () => ({ state: createMockState(), ...createMockFunctions(), diff --git a/packages/eui/src/components/flyout/manager/hooks.test.tsx b/packages/eui/src/components/flyout/manager/hooks.test.tsx index dcc5b9a8a32..c86e574b8b8 100644 --- a/packages/eui/src/components/flyout/manager/hooks.test.tsx +++ b/packages/eui/src/components/flyout/manager/hooks.test.tsx @@ -7,9 +7,7 @@ */ import { renderHook } from '../../../test/rtl'; -import { act } from '@testing-library/react'; -import { useFlyoutManagerReducer, useFlyoutId } from './hooks'; -import { LEVEL_MAIN, LEVEL_CHILD } from './const'; +import { useFlyoutId } from './hooks'; // Mock the warnOnce service but keep other actual exports (e.g., useGeneratedHtmlId) jest.mock('../../../services', () => { @@ -20,352 +18,35 @@ jest.mock('../../../services', () => { }; }); -// Mock the useFlyout selector -jest.mock('./selectors', () => ({ - useFlyout: jest.fn(), - useIsFlyoutRegistered: jest.fn(), -})); +describe('useFlyoutId', () => { + it('should return a stable ID when no id is provided', () => { + const { result, rerender } = renderHook(() => useFlyoutId()); -describe('flyout manager hooks', () => { - const { - useFlyout: mockUseFlyout, - useIsFlyoutRegistered: mockUseIsFlyoutRegistered, - } = jest.requireMock('./selectors'); + const firstId = result.current; + rerender(); + const secondId = result.current; - beforeEach(() => { - mockUseFlyout.mockClear(); - mockUseIsFlyoutRegistered.mockClear(); + expect(firstId).toBe(secondId); + expect(typeof firstId).toBe('string'); + expect(firstId).toMatch(/^flyout-_generated-id-\d+$/); }); - describe('useFlyoutManagerReducer', () => { - it('should return initial state and bound action creators', () => { - const { result } = renderHook(() => useFlyoutManagerReducer()); + it('should return the provided id when given', () => { + const customId = 'my-custom-flyout-id'; + const { result } = renderHook(() => useFlyoutId(customId)); - expect(result.current.state).toEqual({ - sessions: [], - flyouts: [], - layoutMode: 'side-by-side', - }); - expect(typeof result.current.dispatch).toBe('function'); - expect(typeof result.current.addFlyout).toBe('function'); - expect(typeof result.current.closeFlyout).toBe('function'); - expect(typeof result.current.setActiveFlyout).toBe('function'); - expect(typeof result.current.setFlyoutWidth).toBe('function'); - }); - - it('should accept custom initial state', () => { - const customInitialState = { - sessions: [], - flyouts: [], - layoutMode: 'stacked' as const, - }; - - const { result } = renderHook(() => - useFlyoutManagerReducer(customInitialState) - ); - - expect(result.current.state.layoutMode).toBe('stacked'); - }); - - it('should dispatch actions correctly', () => { - const { result } = renderHook(() => useFlyoutManagerReducer()); - - act(() => { - result.current.addFlyout('main-1', 'main', LEVEL_MAIN, 'l'); - }); - - expect(result.current.state.flyouts).toHaveLength(1); - expect(result.current.state.flyouts[0]).toEqual({ - flyoutId: 'main-1', - level: LEVEL_MAIN, - size: 'l', - activityStage: 'opening', - }); - expect(result.current.state.sessions).toHaveLength(1); - }); - - it('should handle multiple actions in sequence', () => { - const { result } = renderHook(() => useFlyoutManagerReducer()); - - act(() => { - result.current.addFlyout('main-1', 'main', LEVEL_MAIN); - result.current.addFlyout('child-1', 'child', LEVEL_CHILD); - result.current.setActiveFlyout('child-1'); - result.current.setFlyoutWidth('main-1', 600); - result.current.setFlyoutWidth('child-1', 400); - }); - - expect(result.current.state.flyouts).toHaveLength(2); - expect(result.current.state.sessions[0].childFlyoutId).toBe('child-1'); - expect(result.current.state.flyouts[0].width).toBe(600); - expect(result.current.state.flyouts[1].width).toBe(400); - }); - - it('should maintain action creator stability across renders', () => { - const { result, rerender } = renderHook(() => useFlyoutManagerReducer()); - - const initialAddFlyout = result.current.addFlyout; - const initialCloseFlyout = result.current.closeFlyout; - - rerender(); - - expect(result.current.addFlyout).toBe(initialAddFlyout); - expect(result.current.closeFlyout).toBe(initialCloseFlyout); - }); - - it('should handle complex state transitions', () => { - const { result } = renderHook(() => useFlyoutManagerReducer()); - - // Create a complex scenario - act(() => { - // Add main flyout - result.current.addFlyout('main-1', 'main', LEVEL_MAIN, 'l'); - // Add child flyout - result.current.addFlyout('child-1', 'child', LEVEL_CHILD, 'm'); - // Set child as active - result.current.setActiveFlyout('child-1'); - // Update widths - result.current.setFlyoutWidth('main-1', 600); - result.current.setFlyoutWidth('child-1', 400); - // Close child flyout - result.current.closeFlyout('child-1'); - // Close main flyout - result.current.closeFlyout('main-1'); - }); - - expect(result.current.state.flyouts).toHaveLength(0); - expect(result.current.state.sessions).toHaveLength(0); - }); + expect(result.current).toBe(customId); }); - describe('useFlyoutId', () => { - it('should return provided flyout ID when it is not registered', () => { - mockUseIsFlyoutRegistered.mockReturnValue(false); // ID is available - const { result } = renderHook(() => useFlyoutId('existing-id')); - - expect(mockUseIsFlyoutRegistered).toHaveBeenCalledWith('existing-id'); - expect(result.current).toBe('existing-id'); - }); - - it('should generate deterministic ID when no ID is provided', () => { - const { result } = renderHook(() => useFlyoutId()); - - expect(result.current).toMatch(/^flyout-/); - expect(typeof result.current).toBe('string'); - }); - - it('should generate deterministic ID when provided ID is empty', () => { - const { result } = renderHook(() => useFlyoutId('')); - - expect(result.current).toMatch(/^flyout-/); - }); - - it('should generate deterministic ID when provided ID is undefined', () => { - const { result } = renderHook(() => useFlyoutId(undefined)); - - expect(result.current).toMatch(/^flyout-/); - }); - - it('should maintain ID consistency across renders', () => { - mockUseIsFlyoutRegistered.mockReturnValue(false); // ID is available - const { result, rerender } = renderHook(() => useFlyoutId('stable-id')); - - const initialId = result.current; - rerender(); - rerender(); - - expect(result.current).toBe(initialId); - }); - - it('should handle different IDs for different components', () => { - mockUseIsFlyoutRegistered.mockReturnValue(false); // IDs are available - const { result: result1 } = renderHook(() => useFlyoutId('id-1')); - const { result: result2 } = renderHook(() => useFlyoutId('id-2')); - - expect(result1.current).toBe('id-1'); - expect(result2.current).toBe('id-2'); - }); - - it('should handle generated IDs for different components', () => { - const { result: result1 } = renderHook(() => useFlyoutId()); - const { result: result2 } = renderHook(() => useFlyoutId()); - - expect(result1.current).not.toBe(result2.current); - expect(result1.current).toMatch(/^flyout-/); - expect(result2.current).toMatch(/^flyout-/); - }); - - it('should handle ID conflicts gracefully', () => { - // Mock that the ID is already registered (conflict) - mockUseIsFlyoutRegistered.mockReturnValue(true); - - const { result } = renderHook(() => useFlyoutId('conflict-id')); - - expect(result.current).toMatch(/^flyout-/); - expect(result.current).not.toBe('conflict-id'); - }); - - it('should handle multiple ID conflicts', () => { - // Mock multiple conflicts - mockUseIsFlyoutRegistered.mockReturnValue(true); - - const { result } = renderHook(() => useFlyoutId('conflict-1')); - - expect(result.current).toMatch(/^flyout-/); - expect(result.current).not.toBe('conflict-1'); - }); - - it('should handle special characters in provided IDs', () => { - mockUseIsFlyoutRegistered.mockReturnValue(false); // IDs are available - const specialIds = [ - 'flyout-1', - 'flyout_2', - 'flyout.3', - 'flyout-4', - 'FLYOUT-5', - 'Flyout-6', - ]; - - specialIds.forEach((id) => { - const { result } = renderHook(() => useFlyoutId(id)); - expect(result.current).toBe(id); - }); - }); - - it('should handle very long IDs', () => { - mockUseIsFlyoutRegistered.mockReturnValue(false); // ID is available - const longId = 'a'.repeat(1000); - const { result } = renderHook(() => useFlyoutId(longId)); - - expect(result.current).toBe(longId); - }); - - it('should handle empty string IDs', () => { - const { result } = renderHook(() => useFlyoutId('')); - - expect(result.current).toMatch(/^flyout-/); - }); - - it('should handle null IDs', () => { - const { result } = renderHook(() => useFlyoutId(null as any)); - - expect(result.current).toMatch(/^flyout-/); - }); - - it('should maintain ID stability when input changes', () => { - // First call with no ID - generates one - const { result, rerender } = renderHook(() => useFlyoutId()); - const firstId = result.current; - - // Re-render with same input (no ID) - rerender(); - expect(result.current).toBe(firstId); - - // Re-render with different input (still no ID) - rerender(); - expect(result.current).toBe(firstId); - }); - - it('should not change ID when provided ID changes', () => { - const { result, rerender } = renderHook(({ id }) => useFlyoutId(id), { - initialProps: { id: undefined as string | undefined }, - }); - - const generatedId = result.current; - expect(generatedId).toMatch(/^flyout-/); - - // Change to provided ID - mockUseIsFlyoutRegistered.mockReturnValue(false); - rerender({ id: 'provided-id' }); - - expect(result.current).toBe(generatedId); - expect(result.current).not.toBe('provided-id'); - }); - }); - - describe('hook integration', () => { - it('should work together with reducer', () => { - mockUseIsFlyoutRegistered.mockReturnValue(false); // ID is available - const { result: reducerResult } = renderHook(() => - useFlyoutManagerReducer() - ); - const { result: idResult } = renderHook(() => useFlyoutId('test-id')); - - act(() => { - reducerResult.current.addFlyout(idResult.current, 'main', LEVEL_MAIN); - }); - - expect(reducerResult.current.state.flyouts).toHaveLength(1); - expect(reducerResult.current.state.flyouts[0].flyoutId).toBe('test-id'); - }); - - it('should handle multiple flyouts with generated IDs', () => { - const { result: reducerResult } = renderHook(() => - useFlyoutManagerReducer() - ); - const { result: idResult1 } = renderHook(() => useFlyoutId()); - const { result: idResult2 } = renderHook(() => useFlyoutId()); - - act(() => { - reducerResult.current.addFlyout(idResult1.current, 'main', LEVEL_MAIN); - reducerResult.current.addFlyout( - idResult2.current, - 'child', - LEVEL_CHILD - ); - }); - - expect(reducerResult.current.state.flyouts).toHaveLength(2); - expect(reducerResult.current.state.sessions).toHaveLength(1); - expect(reducerResult.current.state.sessions[0].childFlyoutId).toBe( - idResult2.current - ); - }); - }); - - describe('edge cases', () => { - it('should handle rapid state changes', () => { - const { result } = renderHook(() => useFlyoutManagerReducer()); - - act(() => { - // Rapidly add and remove flyouts - for (let i = 0; i < 10; i++) { - result.current.addFlyout(`flyout-${i}`, 'main', LEVEL_MAIN); - result.current.closeFlyout(`flyout-${i}`); - } - }); - - expect(result.current.state.flyouts).toHaveLength(0); - expect(result.current.state.sessions).toHaveLength(0); - }); - - it('should handle concurrent ID generation', () => { - const results = []; - for (let i = 0; i < 5; i++) { - const { result } = renderHook(() => useFlyoutId()); - results.push(result.current); - } - - // All IDs should be unique - const uniqueIds = new Set(results); - expect(uniqueIds.size).toBe(5); - - // All IDs should follow the pattern - results.forEach((id) => { - expect(id).toMatch(/^flyout-/); - }); - }); + it('should return a stable ID when id is provided', () => { + const customId = 'my-custom-flyout-id'; + const { result, rerender } = renderHook(() => useFlyoutId(customId)); - it('should handle undefined initial state gracefully', () => { - const { result } = renderHook(() => - useFlyoutManagerReducer(undefined as any) - ); + const firstId = result.current; + rerender(); + const secondId = result.current; - expect(result.current.state).toEqual({ - sessions: [], - flyouts: [], - layoutMode: 'side-by-side', - }); - }); + expect(firstId).toBe(secondId); + expect(firstId).toBe(customId); }); }); diff --git a/packages/eui/src/components/flyout/manager/hooks.ts b/packages/eui/src/components/flyout/manager/hooks.ts index 0c40e44c981..04c4dd72551 100644 --- a/packages/eui/src/components/flyout/manager/hooks.ts +++ b/packages/eui/src/components/flyout/manager/hooks.ts @@ -6,26 +6,11 @@ * Side Public License, v 1. */ -import { useCallback, useContext, useReducer, useRef } from 'react'; +import { useContext, useRef } from 'react'; import { warnOnce, useGeneratedHtmlId } from '../../../services'; -import { flyoutManagerReducer, initialState } from './reducer'; -import { - addFlyout as addFlyoutAction, - closeFlyout as closeFlyoutAction, - setActiveFlyout as setActiveFlyoutAction, - setFlyoutWidth as setFlyoutWidthAction, - goBack as goBackAction, - goToFlyout as goToFlyoutAction, -} from './actions'; -import { - type EuiFlyoutLevel, - type EuiFlyoutManagerState, - type FlyoutManagerApi, -} from './types'; import { EuiFlyoutManagerContext } from './provider'; -import { LEVEL_MAIN } from './const'; import { useIsFlyoutRegistered } from './selectors'; // Ensure uniqueness across multiple hook instances, including in test envs @@ -49,67 +34,6 @@ export { useIsInManagedFlyout } from './context'; // Convenience selector for a flyout's activity stage export type { EuiFlyoutActivityStage } from './types'; -/** - * Hook that provides the flyout manager reducer and bound action creators. - * Accepts an optional initial state (mainly for tests or custom setups). - */ -export function useFlyoutManagerReducer( - initial: EuiFlyoutManagerState = initialState -): FlyoutManagerApi { - const [state, dispatch] = useReducer(flyoutManagerReducer, initial); - - const addFlyout = useCallback( - ( - flyoutId: string, - title: string, - level: EuiFlyoutLevel = LEVEL_MAIN, - size?: string - ) => dispatch(addFlyoutAction(flyoutId, title, level, size)), - [] - ); - const closeFlyout = useCallback( - (flyoutId: string) => dispatch(closeFlyoutAction(flyoutId)), - [] - ); - const setActiveFlyout = useCallback( - (flyoutId: string | null) => dispatch(setActiveFlyoutAction(flyoutId)), - [] - ); - const setFlyoutWidth = useCallback( - (flyoutId: string, width: number) => - dispatch(setFlyoutWidthAction(flyoutId, width)), - [] - ); - const goBack = useCallback(() => dispatch(goBackAction()), []); - const goToFlyout = useCallback( - (flyoutId: string) => dispatch(goToFlyoutAction(flyoutId)), - [] - ); - const getHistoryItems = useCallback(() => { - const currentSessionIndex = state.sessions.length - 1; - const previousSessions = state.sessions.slice(0, currentSessionIndex); - - return previousSessions - .reverse() - .map(({ title, mainFlyoutId: mainFlyoutId }) => ({ - title: title, - onClick: () => goToFlyout(mainFlyoutId), - })); - }, [state.sessions, goToFlyout]); - - return { - state, - dispatch, - addFlyout, - closeFlyout, - setActiveFlyout, - setFlyoutWidth, - goBack, - goToFlyout, - getHistoryItems, - }; -} - /** Access the flyout manager context (state and actions). */ export const useFlyoutManager = () => useContext(EuiFlyoutManagerContext); diff --git a/packages/eui/src/components/flyout/manager/selectors.test.tsx b/packages/eui/src/components/flyout/manager/selectors.test.tsx index 51fc6228872..1a0eaeaba2f 100644 --- a/packages/eui/src/components/flyout/manager/selectors.test.tsx +++ b/packages/eui/src/components/flyout/manager/selectors.test.tsx @@ -9,547 +9,178 @@ import React from 'react'; import { renderHook } from '../../../test/rtl'; import { - useSession, - useHasActiveSession, - useIsFlyoutActive, - useFlyout, - useCurrentSession, - useCurrentMainFlyout, - useCurrentChildFlyout, useFlyoutWidth, useParentFlyoutSize, useHasChildFlyout, } from './selectors'; -import { EuiFlyoutManager, useFlyoutManager } from './provider'; -import { useFlyoutManagerReducer } from './hooks'; +import { useFlyoutManager } from './provider'; import { LEVEL_MAIN, LEVEL_CHILD } from './const'; -// Mock the hooks module to avoid circular dependencies -jest.mock('./hooks', () => ({ - useFlyoutManagerReducer: jest.fn(), -})); - // Mock the provider context jest.mock('./provider', () => ({ EuiFlyoutManager: ({ children }: { children: React.ReactNode }) => children, useFlyoutManager: jest.fn(), })); -// Test wrapper component -const TestWrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => { - return {children}; -}; - -// Mock data -const mockState = { - sessions: [ - { mainFlyoutId: 'main-1', childFlyoutId: 'child-1' }, - { mainFlyoutId: 'main-2', childFlyoutId: null }, - ], - flyouts: [ - { flyoutId: 'main-1', level: LEVEL_MAIN, size: 'l', width: 600 }, - { flyoutId: 'child-1', level: LEVEL_CHILD, size: 'm', width: 400 }, - { flyoutId: 'main-2', level: LEVEL_MAIN, size: 's', width: 300 }, - ], - layoutMode: 'side-by-side' as const, -}; - -const mockApi = { - state: mockState, - dispatch: jest.fn(), - addFlyout: jest.fn(), - closeFlyout: jest.fn(), - setActiveFlyout: jest.fn(), - setFlyoutWidth: jest.fn(), -}; +const mockUseFlyoutManager = useFlyoutManager as jest.MockedFunction< + typeof useFlyoutManager +>; -describe('flyout manager selectors', () => { +describe('Flyout Manager Selectors', () => { beforeEach(() => { - jest.clearAllMocks(); - const mockUseFlyoutManagerReducer = useFlyoutManagerReducer as jest.Mock; - const mockUseFlyoutManager = useFlyoutManager as jest.Mock; - mockUseFlyoutManagerReducer.mockReturnValue(mockApi); - mockUseFlyoutManager.mockReturnValue(mockApi); - }); - - describe('useSession', () => { - it('should return session when flyout ID matches main', () => { - const { result } = renderHook(() => useSession('main-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toEqual({ - mainFlyoutId: 'main-1', - childFlyoutId: 'child-1', - }); - }); - - it('should return session when flyout ID matches child', () => { - const { result } = renderHook(() => useSession('child-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toEqual({ - mainFlyoutId: 'main-1', - childFlyoutId: 'child-1', - }); - }); - - it('should return null when flyout ID does not match any session', () => { - const { result } = renderHook(() => useSession('non-existent'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); - - it('should return null when no flyout ID is provided', () => { - const { result } = renderHook(() => useSession(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); - - it('should return null when flyout ID is null', () => { - const { result } = renderHook(() => useSession(null), { - wrapper: TestWrapper, - }); - - // The selector treats null as a literal value to search for - // It finds the session where childFlyoutId: null matches flyoutId: null - expect(result.current).toEqual({ - mainFlyoutId: 'main-2', - childFlyoutId: null, - }); - }); - }); - - describe('useHasActiveSession', () => { - it('should return true when there are active sessions', () => { - const { result } = renderHook(() => useHasActiveSession(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe(true); - }); - - it('should return false when there are no sessions', () => { - const emptyState = { ...mockState, sessions: [] }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - - const { result } = renderHook(() => useHasActiveSession(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe(false); - }); - }); - - describe('useIsFlyoutActive', () => { - it('should return true when flyout is main in current session', () => { - const { result } = renderHook(() => useIsFlyoutActive('main-2'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe(true); - }); - - it('should return true when flyout is child in current session', () => { - const { result } = renderHook(() => useIsFlyoutActive('child-1'), { - wrapper: TestWrapper, - }); - - // child-1 is not in the current session (main-2 with no child) - // It's in the previous session (main-1 with child-1) - expect(result.current).toBe(false); - }); - - it('should return false when flyout is not in current session', () => { - const { result } = renderHook(() => useIsFlyoutActive('non-existent'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe(false); - }); - - it('should return false when flyout is in previous session', () => { - const { result } = renderHook(() => useIsFlyoutActive('main-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe(false); - }); - }); - - describe('useFlyout', () => { - it('should return flyout when ID exists', () => { - const { result } = renderHook(() => useFlyout('main-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toEqual({ - flyoutId: 'main-1', - level: LEVEL_MAIN, - size: 'l', - width: 600, - }); - }); - - it('should return null when flyout ID does not exist', () => { - const { result } = renderHook(() => useFlyout('non-existent'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); - - it('should return null when no flyout ID is provided', () => { - const { result } = renderHook(() => useFlyout(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); - - it('should return null when flyout ID is null', () => { - const { result } = renderHook(() => useFlyout(null), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); - }); - - describe('useCurrentSession', () => { - it('should return the most recent session', () => { - const { result } = renderHook(() => useCurrentSession(), { - wrapper: TestWrapper, - }); - - expect(result.current).toEqual({ - mainFlyoutId: 'main-2', - childFlyoutId: null, - }); - }); - - it('should return null when no sessions exist', () => { - const emptyState = { ...mockState, sessions: [] }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - - const { result } = renderHook(() => useCurrentSession(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); - }); - - describe('useCurrentMainFlyout', () => { - it('should return the main flyout of current session', () => { - const { result } = renderHook(() => useCurrentMainFlyout(), { - wrapper: TestWrapper, - }); - - expect(result.current).toEqual({ - flyoutId: 'main-2', - level: LEVEL_MAIN, - size: 's', - width: 300, - }); - }); - - it('should return null when no current session exists', () => { - const emptyState = { ...mockState, sessions: [] }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - - const { result } = renderHook(() => useCurrentMainFlyout(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); - }); - - describe('useCurrentChildFlyout', () => { - it('should return the child flyout of current session', () => { - // Change current session to one with a child - const stateWithChildCurrent = { - ...mockState, - sessions: [ - { mainFlyoutId: 'main-2', childFlyoutId: null }, - { mainFlyoutId: 'main-1', childFlyoutId: 'child-1' }, // Make this the current session - ], - }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: stateWithChildCurrent, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: stateWithChildCurrent, - }); - - const { result } = renderHook(() => useCurrentChildFlyout(), { - wrapper: TestWrapper, - }); - - expect(result.current).toEqual({ - flyoutId: 'child-1', - level: LEVEL_CHILD, - size: 'm', - width: 400, - }); - }); - - it('should return null when current session has no child', () => { - const { result } = renderHook(() => useCurrentChildFlyout(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); - - it('should return null when no current session exists', () => { - const emptyState = { ...mockState, sessions: [] }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - - const { result } = renderHook(() => useCurrentChildFlyout(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeNull(); - }); + mockUseFlyoutManager.mockClear(); }); describe('useFlyoutWidth', () => { - it('should return flyout width when it exists', () => { - const { result } = renderHook(() => useFlyoutWidth('main-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe(600); - }); - - it('should return undefined when flyout has no width', () => { - const stateWithoutWidth = { - ...mockState, - flyouts: [{ flyoutId: 'main-1', level: LEVEL_MAIN, size: 'l' }], - }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: stateWithoutWidth, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: stateWithoutWidth, - }); - - const { result } = renderHook(() => useFlyoutWidth('main-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeUndefined(); - }); - - it('should return undefined when flyout does not exist', () => { - const { result } = renderHook(() => useFlyoutWidth('non-existent'), { - wrapper: TestWrapper, - }); + it('should return undefined when flyout is not found', () => { + mockUseFlyoutManager.mockReturnValue({ + state: { + sessions: [], + flyouts: [], + layoutMode: 'side-by-side', + }, + dispatch: jest.fn(), + addFlyout: jest.fn(), + closeFlyout: jest.fn(), + setActiveFlyout: jest.fn(), + setFlyoutWidth: jest.fn(), + goBack: jest.fn(), + goToFlyout: jest.fn(), + historyItems: [], + }); + + const { result } = renderHook(() => + useFlyoutWidth('non-existent-flyout') + ); expect(result.current).toBeUndefined(); }); - it('should return undefined when no flyout ID is provided', () => { - const { result } = renderHook(() => useFlyoutWidth(), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeUndefined(); + it('should return the width when flyout exists', () => { + mockUseFlyoutManager.mockReturnValue({ + state: { + sessions: [], + flyouts: [ + { + flyoutId: 'test-flyout', + level: LEVEL_MAIN, + size: 'm', + activityStage: 'active', + width: 500, + }, + ], + layoutMode: 'side-by-side', + }, + dispatch: jest.fn(), + addFlyout: jest.fn(), + closeFlyout: jest.fn(), + setActiveFlyout: jest.fn(), + setFlyoutWidth: jest.fn(), + goBack: jest.fn(), + goToFlyout: jest.fn(), + historyItems: [], + }); + + const { result } = renderHook(() => useFlyoutWidth('test-flyout')); + + expect(result.current).toBe(500); }); }); describe('useParentFlyoutSize', () => { - it('should return parent flyout size for child flyout', () => { - const { result } = renderHook(() => useParentFlyoutSize('child-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe('l'); - }); - - it('should return undefined when child flyout has no parent', () => { - const { result } = renderHook(() => useParentFlyoutSize('non-existent'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBeUndefined(); - }); - - it('should return undefined when parent flyout has no size', () => { - const stateWithoutSize = { - ...mockState, - flyouts: [ - { flyoutId: 'main-1', level: LEVEL_MAIN }, - { flyoutId: 'child-1', level: LEVEL_CHILD }, - ], - }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: stateWithoutSize, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: stateWithoutSize, - }); - - const { result } = renderHook(() => useParentFlyoutSize('child-1'), { - wrapper: TestWrapper, - }); + it('should return undefined when no parent flyout exists', () => { + mockUseFlyoutManager.mockReturnValue({ + state: { + sessions: [], + flyouts: [], + layoutMode: 'side-by-side', + }, + dispatch: jest.fn(), + addFlyout: jest.fn(), + closeFlyout: jest.fn(), + setActiveFlyout: jest.fn(), + setFlyoutWidth: jest.fn(), + goBack: jest.fn(), + goToFlyout: jest.fn(), + historyItems: [], + }); + + const { result } = renderHook(() => useParentFlyoutSize('child-flyout')); expect(result.current).toBeUndefined(); }); }); describe('useHasChildFlyout', () => { - it('should return true when main flyout has a child', () => { - const { result } = renderHook(() => useHasChildFlyout('main-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe(true); - }); - - it('should return false when main flyout has no child', () => { - const { result } = renderHook(() => useHasChildFlyout('main-2'), { - wrapper: TestWrapper, - }); + it('should return false when no child flyout exists', () => { + mockUseFlyoutManager.mockReturnValue({ + state: { + sessions: [], + flyouts: [ + { + flyoutId: 'parent-flyout', + level: LEVEL_MAIN, + size: 'm', + activityStage: 'active', + }, + ], + layoutMode: 'side-by-side', + }, + dispatch: jest.fn(), + addFlyout: jest.fn(), + closeFlyout: jest.fn(), + setActiveFlyout: jest.fn(), + setFlyoutWidth: jest.fn(), + goBack: jest.fn(), + goToFlyout: jest.fn(), + historyItems: [], + }); + + const { result } = renderHook(() => useHasChildFlyout('parent-flyout')); expect(result.current).toBe(false); }); - it('should return false when flyout ID does not exist', () => { - const { result } = renderHook(() => useHasChildFlyout('non-existent'), { - wrapper: TestWrapper, - }); - - expect(result.current).toBe(false); - }); - - it('should return false when flyout is not a main flyout', () => { - const { result } = renderHook(() => useHasChildFlyout('child-1'), { - wrapper: TestWrapper, - }); + it('should return true when child flyout exists', () => { + mockUseFlyoutManager.mockReturnValue({ + state: { + sessions: [ + { + mainFlyoutId: 'parent-flyout', + childFlyoutId: 'child-flyout', + title: 'Parent Flyout', + }, + ], + flyouts: [ + { + flyoutId: 'parent-flyout', + level: LEVEL_MAIN, + size: 'm', + activityStage: 'active', + }, + { + flyoutId: 'child-flyout', + level: LEVEL_CHILD, + size: 'm', + activityStage: 'active', + }, + ], + layoutMode: 'side-by-side', + }, + dispatch: jest.fn(), + addFlyout: jest.fn(), + closeFlyout: jest.fn(), + setActiveFlyout: jest.fn(), + setFlyoutWidth: jest.fn(), + goBack: jest.fn(), + goToFlyout: jest.fn(), + historyItems: [], + }); + + const { result } = renderHook(() => useHasChildFlyout('parent-flyout')); - // The selector checks if the flyout ID has a session with a child - // Since child-1 is in a session with childFlyoutId: 'child-1', it returns true expect(result.current).toBe(true); }); }); - - describe('edge cases and error handling', () => { - it('should handle empty flyouts array', () => { - const emptyState = { ...mockState, flyouts: [] }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: emptyState, - }); - - const { result: flyoutResult } = renderHook(() => useFlyout('main-1'), { - wrapper: TestWrapper, - }); - const { result: widthResult } = renderHook( - () => useFlyoutWidth('main-1'), - { - wrapper: TestWrapper, - } - ); - - expect(flyoutResult.current).toBeNull(); - expect(widthResult.current).toBeUndefined(); - }); - - it('should handle malformed flyout data gracefully', () => { - const malformedState = { - ...mockState, - flyouts: [ - { flyoutId: 'main-1' }, // Missing required properties - ], - }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: malformedState, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: malformedState, - }); - - const { result } = renderHook(() => useFlyout('main-1'), { - wrapper: TestWrapper, - }); - - expect(result.current).toEqual({ flyoutId: 'main-1' }); - }); - - it('should handle sessions with missing flyout references', () => { - const invalidState = { - ...mockState, - sessions: [ - { mainFlyoutId: 'main-1', childFlyoutId: 'non-existent-child' }, - ], - flyouts: [{ flyoutId: 'main-1', level: LEVEL_MAIN }], - }; - (useFlyoutManagerReducer as jest.Mock).mockReturnValue({ - ...mockApi, - state: invalidState, - }); - (useFlyoutManager as jest.Mock).mockReturnValue({ - ...mockApi, - state: invalidState, - }); - - const { result } = renderHook(() => useSession('non-existent-child'), { - wrapper: TestWrapper, - }); - - expect(result.current).toEqual({ - mainFlyoutId: 'main-1', - childFlyoutId: 'non-existent-child', - }); - }); - }); }); diff --git a/packages/eui/src/components/flyout/manager/store.ts b/packages/eui/src/components/flyout/manager/store.ts index 7396889e912..94362f744c7 100644 --- a/packages/eui/src/components/flyout/manager/store.ts +++ b/packages/eui/src/components/flyout/manager/store.ts @@ -58,15 +58,7 @@ function createStore( const subscribe = (listener: Listener) => { listeners.add(listener); return () => { - // Use React's scheduler to defer cleanup until after current render - if (typeof requestIdleCallback !== 'undefined') { - requestIdleCallback(() => listeners.delete(listener)); - } else if (typeof requestAnimationFrame !== 'undefined') { - requestAnimationFrame(() => listeners.delete(listener)); - } else { - // Fallback to setTimeout for older environments - setTimeout(() => listeners.delete(listener), 0); - } + listeners.delete(listener); }; }; From 39ab1bd0748c3b50b08a8afb65f0a68e99c95d31 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Tue, 7 Oct 2025 15:31:30 -0700 Subject: [PATCH 4/7] whitespace change --- .../eui/src/components/flyout/manager/provider.tsx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/provider.tsx b/packages/eui/src/components/flyout/manager/provider.tsx index f60c3bbd3f1..70688fa6220 100644 --- a/packages/eui/src/components/flyout/manager/provider.tsx +++ b/packages/eui/src/components/flyout/manager/provider.tsx @@ -27,13 +27,12 @@ export const EuiFlyoutManager: React.FC<{ children: React.ReactNode }> = ({ }) => { const { getState, subscribe, ...rest } = getFlyoutManagerStore(); const state = useSyncExternalStore(subscribe, getState); - const api: FlyoutManagerApi = useMemo(() => { - const result = { - state, - ...rest, - }; - return result; - }, [state, rest]); + + const api: FlyoutManagerApi = useMemo( + () => ({ state, ...rest }), + [state, rest] + ); + return ( {children} From e4de6b1f7d9d4212ba4555be702140cd4684b614 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 8 Oct 2025 15:25:25 -0700 Subject: [PATCH 5/7] Clean up storybooks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move “Multi-root sync” storybook to flyout_sessions.stories.tsx - Revert “Playground” storybook file --- .../flyout/manager/flyout_manager.stories.tsx | 146 +----------- .../manager/flyout_sessions.stories.tsx | 220 ++++++++++++------ 2 files changed, 157 insertions(+), 209 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx b/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx index 57240e82a40..663017152ac 100644 --- a/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx @@ -8,25 +8,18 @@ import { actions } from '@storybook/addon-actions'; import type { Meta, StoryObj } from '@storybook/react'; -import React, { useEffect, useRef, useState } from 'react'; -import { createRoot } from 'react-dom/client'; -import type { Root } from 'react-dom/client'; +import React, { useState } from 'react'; import { LOKI_SELECTORS } from '../../../../.storybook/loki'; import { EuiBreakpointSize } from '../../../services'; import { EuiButton } from '../../button'; -import { EuiCodeBlock } from '../../code'; -import { EuiFlexGroup, EuiFlexItem } from '../../flex'; -import { EuiPanel } from '../../panel'; -import { EuiProvider } from '../../provider'; import { EuiSpacer } from '../../spacer'; import { EuiText } from '../../text'; -import { EuiTitle } from '../../title'; import { FLYOUT_TYPES, EuiFlyout } from '../flyout'; import { EuiFlyoutBody } from '../flyout_body'; import { EuiFlyoutFooter } from '../flyout_footer'; import { EuiFlyoutChild, EuiFlyoutChildProps } from './flyout_child'; -import { useFlyoutLayoutMode, useFlyoutManager } from './hooks'; +import { useFlyoutLayoutMode } from './hooks'; type EuiFlyoutChildActualProps = Pick< EuiFlyoutChildProps, @@ -314,138 +307,3 @@ export const FlyoutChildDemo: Story = { name: 'Playground', render: (args) => , }; - -const ExternalRootFlyout: React.FC<{ id: string }> = ({ id }) => { - const [isOpen, setIsOpen] = useState(false); - - return ( - - -

{id}

- - - setIsOpen((prev) => !prev)}> - {isOpen ? 'Close flyout' : 'Open flyout'} - - setIsOpen(false)} - flyoutMenuProps={{ title: `${id} flyout` }} - > - - -

- This flyout lives in a separate React root but shares the same - manager state. Closing it here should update all other flyout - menus and history. -

- - - - - ); -}; - -const MultiRootFlyoutDemo: React.FC = (args) => { - const secondaryRootRef = useRef(null); - const tertiaryRootRef = useRef(null); - const mountedRootsRef = useRef([]); - const flyoutManager = useFlyoutManager(); - - useEffect(() => { - const timer = setTimeout(() => { - if (secondaryRootRef.current && tertiaryRootRef.current) { - const containers = [ - { container: secondaryRootRef.current, id: 'Secondary root' }, - { container: tertiaryRootRef.current, id: 'Tertiary root' }, - ]; - - mountedRootsRef.current = containers.map(({ container, id }) => { - const root = createRoot(container); - root.render( - - - - ); - return root; - }); - } - }, 100); - - return () => { - clearTimeout(timer); - mountedRootsRef.current.forEach((root) => root.unmount()); - mountedRootsRef.current = []; - }; - }, []); - - return ( - <> - -

Primary React root

- - - - - -

Additional React roots

- - - -

- These flyouts are rendered in separate React roots but share the same - flyout manager state. Open/close any flyout and watch the shared state - update below. -

- - - - -
- - -
- - - - -

Shared manager state

- - - - {JSON.stringify( - { - sessions: flyoutManager?.state.sessions, - flyouts: flyoutManager?.state.flyouts, - }, - null, - 2 - )} - - - ); -}; - -export const MultiRootSyncPlayground: Story = { - name: 'Multi-root sync', - render: (args) => , - parameters: { - layout: 'fullscreen', - }, -}; diff --git a/packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx b/packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx index 3c3683ee731..ce436457305 100644 --- a/packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx @@ -6,17 +6,27 @@ * Side Public License, v 1. */ -import { Meta, StoryObj } from '@storybook/react'; import { action } from '@storybook/addon-actions'; -import React, { useState, useCallback, useMemo } from 'react'; +import { Meta, StoryObj } from '@storybook/react'; +import React, { + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; +import type { Root } from 'react-dom/client'; +import { createRoot } from 'react-dom/client'; import { EuiButton, EuiCodeBlock, EuiDescriptionList, + EuiFlexGroup, + EuiFlexItem, EuiFlyoutBody, - EuiPageTemplate, - EuiPageTemplateProps, + EuiPanel, + EuiProvider, EuiSpacer, EuiSwitch, EuiSwitchEvent, @@ -24,7 +34,7 @@ import { EuiTitle, } from '../..'; import { EuiFlyout } from '../flyout'; -import { useFlyoutManager, useCurrentSession } from './hooks'; +import { useCurrentSession, useFlyoutManager } from './hooks'; const meta: Meta = { title: 'Layout/EuiFlyout/Flyout Manager', @@ -43,6 +53,31 @@ interface FlyoutSessionProps { childBackgroundShaded?: boolean; } +const DisplayContext: React.FC<{ title: string }> = ({ title }) => { + const flyoutManager = useFlyoutManager(); + const currentSession = useCurrentSession(); + return ( + <> + +

{title}

+ + + + {JSON.stringify( + { + flyoutManager: flyoutManager + ? { state: flyoutManager.state } + : null, + currentSession: currentSession ? currentSession : null, + }, + null, + 2 + )} + + + ); +}; + const FlyoutSession: React.FC = React.memo((props) => { const { title, @@ -176,20 +211,13 @@ const FlyoutSession: React.FC = React.memo((props) => { FlyoutSession.displayName = 'FlyoutSession'; -const ExampleComponent = () => { - const panelled: EuiPageTemplateProps['panelled'] = undefined; - const restrictWidth: EuiPageTemplateProps['restrictWidth'] = false; - const bottomBorder: EuiPageTemplateProps['bottomBorder'] = 'extended'; - +const MultiSessionFlyoutDemo = () => { const [flyoutType, setFlyoutType] = useState<'overlay' | 'push'>('overlay'); const handleFlyoutTypeToggle = useCallback((e: EuiSwitchEvent) => { setFlyoutType(e.target.checked ? 'push' : 'overlay'); }, []); - const flyoutManager = useFlyoutManager(); - const currentSession = useCurrentSession(); - const listItems = useMemo( () => [ { @@ -276,62 +304,124 @@ const ExampleComponent = () => { ); return ( - - + + + - - -

Options

- - - - {/* FIXME add option to set child flyout background style to "shaded" */} - - - - - - -

Contexts

- - - - {JSON.stringify( - { - flyoutManager: flyoutManager - ? { state: flyoutManager.state } - : null, - currentSession: currentSession ? currentSession : null, - }, - null, - 2 - )} - - - + + + ); }; export const MultiSessionExample: StoryObj = { name: 'Multi-session example', - render: () => , - parameters: { - layout: 'fullscreen', - }, + render: () => , +}; + +const ExternalRootFlyout: React.FC<{ id: string }> = ({ id }) => { + const [isOpen, setIsOpen] = useState(false); + + return ( + + +

{id}

+ + + setIsOpen((prev) => !prev)}> + {isOpen ? 'Close flyout' : 'Open flyout'} + + setIsOpen(false)} + flyoutMenuProps={{ title: `${id} flyout` }} + > + + +

+ This flyout lives in a separate React root but shares the same + manager state. Closing it here should update all other flyout + menus and history. +

+ + + + + ); +}; + +const MultiRootFlyoutDemo: React.FC = () => { + const secondaryRootRef = useRef(null); + const tertiaryRootRef = useRef(null); + const mountedRootsRef = useRef([]); + + useEffect(() => { + const timer = setTimeout(() => { + if (secondaryRootRef.current && tertiaryRootRef.current) { + const containers = [ + { container: secondaryRootRef.current, id: 'Secondary root' }, + { container: tertiaryRootRef.current, id: 'Tertiary root' }, + ]; + + mountedRootsRef.current = containers.map(({ container, id }) => { + const root = createRoot(container); + root.render( + + + + ); + return root; + }); + } + }, 100); + + return () => { + clearTimeout(timer); + mountedRootsRef.current.forEach((root) => root.unmount()); + mountedRootsRef.current = []; + }; + }, []); + + return ( + <> + +

Multiple React roots

+ + + +

+ These flyouts are rendered in separate React roots but share the same + flyout manager state. Open/close any flyout and watch the shared state + update below. +

+ + + + +
+ + +
+ + + + + + ); +}; + +export const MultiRootSyncExample: StoryObj = { + name: 'Multi-root sync', + render: () => , }; From f904630da106a234b8ff5586f8ba139bbd8da434 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 8 Oct 2025 16:56:24 -0700 Subject: [PATCH 6/7] Add ExternalRootChildFlyout to the Multi-root storybook --- .../manager/flyout_sessions.stories.tsx | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx b/packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx index ce436457305..7b949237ecc 100644 --- a/packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx @@ -35,6 +35,7 @@ import { } from '../..'; import { EuiFlyout } from '../flyout'; import { useCurrentSession, useFlyoutManager } from './hooks'; +import { EuiFlyoutIsManagedProvider } from './context'; const meta: Meta = { title: 'Layout/EuiFlyout/Flyout Manager', @@ -327,8 +328,100 @@ export const MultiSessionExample: StoryObj = { render: () => , }; +const ExternalRootChildFlyout: React.FC<{ parentId: string }> = ({ + parentId, +}) => { + const [isOpen, setIsOpen] = useState(false); + + const handleOpen = () => { + setIsOpen(true); + }; + + const handleClose = () => { + setIsOpen(false); + }; + + return ( + + +

Root within {parentId}

+ + + + Open Child Flyout + + + + +

+ This is a child flyout rendered in a completely separate React + root! It shares the same flyout manager state as the parent. +

+ +

Parent ID: {parentId}

+ + + + + ); +}; + const ExternalRootFlyout: React.FC<{ id: string }> = ({ id }) => { const [isOpen, setIsOpen] = useState(false); + const [buttonRoot, setButtonRoot] = useState(null); + const buttonContainerRef = useRef(null); + + // Create the button to open the child flyout in a separate React root + useEffect(() => { + if (isOpen) { + // Use setTimeout to ensure the DOM element is rendered + const timer = setTimeout(() => { + if (buttonContainerRef.current) { + const newRoot = createRoot(buttonContainerRef.current); + newRoot.render( + + {/* + EuiFlyoutIsManagedProvider is required here because the child flyout + needs to be detected as being within a managed flyout context for + proper routing. Without this, the child flyout would route to + EuiFlyoutMain instead of EuiFlyoutChild. + */} + + + + + ); + setButtonRoot(newRoot); + } + }, 100); + + return () => clearTimeout(timer); + } + }, [isOpen, id]); + + // Cleanup the button root for opening the child flyout when the main flyout closes + useEffect(() => { + if (!isOpen && buttonRoot) { + buttonRoot.unmount(); + setButtonRoot(null); + } + }, [isOpen, buttonRoot]); + + // Cleanup the main flyout's root on unmount + useEffect(() => { + return () => { + if (buttonRoot) { + buttonRoot.unmount(); + } + }; + }, [buttonRoot]); return ( @@ -354,6 +447,14 @@ const ExternalRootFlyout: React.FC<{ id: string }> = ({ id }) => { manager state. Closing it here should update all other flyout menus and history.

+ +

+ Below is a button rendered in a separate React root that opens a + child flyout: +

+ + {/* Container for the button React root - inside the main flyout */} +
From 88d9d68590d7adc7189a0698f061a0dc129fd9c1 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Thu, 9 Oct 2025 10:43:25 -0700 Subject: [PATCH 7/7] Render child flyout if `hasActiveSession` and `session === undefined` --- .../eui/src/components/flyout/flyout.test.tsx | 112 ++++++++++++++++++ packages/eui/src/components/flyout/flyout.tsx | 15 ++- .../manager/flyout_sessions.stories.tsx | 12 +- 3 files changed, 122 insertions(+), 17 deletions(-) diff --git a/packages/eui/src/components/flyout/flyout.test.tsx b/packages/eui/src/components/flyout/flyout.test.tsx index 475c0336927..0664794764d 100644 --- a/packages/eui/src/components/flyout/flyout.test.tsx +++ b/packages/eui/src/components/flyout/flyout.test.tsx @@ -437,4 +437,116 @@ describe('EuiFlyout', () => { ).not.toBeTruthy(); }); }); + + describe('flyout routing logic', () => { + // Mock the manager hooks to control routing behavior + const mockUseHasActiveSession = jest.fn(); + const mockUseIsInManagedFlyout = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + // Mock the manager hooks + jest.doMock('./manager', () => ({ + ...jest.requireActual('./manager'), + useHasActiveSession: mockUseHasActiveSession, + useIsInManagedFlyout: mockUseIsInManagedFlyout, + })); + }); + + afterEach(() => { + jest.dontMock('./manager'); + }); + + it('routes to child flyout when session is undefined and there is an active session', () => { + // Setup: There's an active session but flyout is not in managed context + mockUseHasActiveSession.mockReturnValue(true); + mockUseIsInManagedFlyout.mockReturnValue(false); + + const { getByTestSubject } = render( + {}} + data-test-subj="child-flyout" + // session is undefined (not explicitly set) + /> + ); + + // Should render as child flyout (EuiFlyoutChild) + const flyout = getByTestSubject('child-flyout'); + expect(flyout).toBeInTheDocument(); + }); + + it('routes to main flyout when session is explicitly true', () => { + // Setup: There's an active session and flyout is not in managed context + mockUseHasActiveSession.mockReturnValue(true); + mockUseIsInManagedFlyout.mockReturnValue(false); + + const { getByTestSubject } = render( + {}} + data-test-subj="main-flyout" + session={true} // Explicitly creating a new session + flyoutMenuProps={{ title: 'Test Main Flyout' }} // Required for managed flyouts + /> + ); + + // Should render as main flyout (EuiFlyoutMain) + const flyout = getByTestSubject('main-flyout'); + expect(flyout).toBeInTheDocument(); + }); + + it('routes to main flyout when session is explicitly false and there is an active session', () => { + // Setup: There's an active session and flyout is not in managed context + mockUseHasActiveSession.mockReturnValue(true); + mockUseIsInManagedFlyout.mockReturnValue(false); + + const { getByTestSubject } = render( + {}} + data-test-subj="main-flyout" + session={false} // Explicitly not creating a new session, but still routes to main + flyoutMenuProps={{ title: 'Test Main Flyout' }} // Required for managed flyouts + /> + ); + + // Should render as main flyout (EuiFlyoutMain) + const flyout = getByTestSubject('main-flyout'); + expect(flyout).toBeInTheDocument(); + }); + + it('routes to child flyout when in managed context and there is an active session', () => { + // Setup: There's an active session and flyout is in managed context + mockUseHasActiveSession.mockReturnValue(true); + mockUseIsInManagedFlyout.mockReturnValue(true); + + const { getByTestSubject } = render( + {}} + data-test-subj="child-flyout" + session={undefined} // Not explicitly set + /> + ); + + // Should render as child flyout (EuiFlyoutChild) + const flyout = getByTestSubject('child-flyout'); + expect(flyout).toBeInTheDocument(); + }); + + it('routes to standard flyout when there is no active session', () => { + // Setup: No active session + mockUseHasActiveSession.mockReturnValue(false); + mockUseIsInManagedFlyout.mockReturnValue(false); + + const { getByTestSubject } = render( + {}} + data-test-subj="standard-flyout" + session={undefined} // Not explicitly set + /> + ); + + // Should render as standard flyout (EuiFlyoutComponent) + const flyout = getByTestSubject('standard-flyout'); + expect(flyout).toBeInTheDocument(); + }); + }); }); diff --git a/packages/eui/src/components/flyout/flyout.tsx b/packages/eui/src/components/flyout/flyout.tsx index adf0abea536..2782dfb0209 100644 --- a/packages/eui/src/components/flyout/flyout.tsx +++ b/packages/eui/src/components/flyout/flyout.tsx @@ -56,12 +56,15 @@ export const EuiFlyout = forwardRef< /* * Flyout routing logic * 1. Main Flyout: When session={true} OR when there's an active session and this flyout - * is rendered outside of a managed flyout context. - * 2. Child Flyout: When there's an active session AND this flyout IS rendered within a - * managed flyout context. + * is rendered outside of a managed flyout context AND explicitly creates a new session. + * 2. Child Flyout: When there's an active session AND (this flyout IS rendered within a + * managed flyout context OR it's not explicitly creating a new session). * 3. Standard Flyout: Default fallback when neither condition is met. */ - if (session === true || (hasActiveSession && !isInManagedFlyout)) { + if ( + session === true || + (hasActiveSession && !isInManagedFlyout && session !== undefined) + ) { if (isUnmanagedFlyout.current) { // TODO: @tkajtoch - We need to find a better way to handle the missing event. onClose?.({} as any); @@ -72,8 +75,8 @@ export const EuiFlyout = forwardRef< ); } - // Else if this flyout is a child of a session AND within a managed flyout context, render EuiChildFlyout. - if (hasActiveSession && isInManagedFlyout) { + // Else if this flyout is a child of a session AND (within a managed flyout context OR not explicitly creating a new session), render EuiChildFlyout. + if (hasActiveSession && (isInManagedFlyout || session === undefined)) { return ( = { title: 'Layout/EuiFlyout/Flyout Manager', @@ -178,7 +177,6 @@ const FlyoutSession: React.FC = React.memo((props) => { isOpen={isChildFlyoutVisible} id={`childFlyout-${title}`} flyoutMenuProps={{ title: `${title} - Child` }} - aria-labelledby="childFlyoutTitle" size={childSize} maxWidth={childMaxWidth} onActive={childFlyoutOnActive} @@ -387,15 +385,7 @@ const ExternalRootFlyout: React.FC<{ id: string }> = ({ id }) => { const newRoot = createRoot(buttonContainerRef.current); newRoot.render( - {/* - EuiFlyoutIsManagedProvider is required here because the child flyout - needs to be detected as being within a managed flyout context for - proper routing. Without this, the child flyout would route to - EuiFlyoutMain instead of EuiFlyoutChild. - */} - - - + ); setButtonRoot(newRoot);