Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/eui/changelogs/upcoming/9378.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Updated `EuiFlyout` manager to close all flyouts when a parent flyout is closed.
13 changes: 11 additions & 2 deletions packages/eui/src/components/flyout/manager/__mocks__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,30 @@
*/

import { LEVEL_MAIN } from '../const';
import { FlyoutManagerApi } from '../types';

/**
* Centralized test utilities for flyout manager tests.
*/

export const mockCloseFlyout = jest.fn();
export const mockCloseAllFlyouts = jest.fn();

export const createMockFunctions = () => ({
export const createMockFunctions = (): Omit<
FlyoutManagerApi,
'state' | 'historyItems'
> => ({
dispatch: jest.fn(),
addFlyout: jest.fn(),
closeFlyout: mockCloseFlyout,
closeAllFlyouts: mockCloseAllFlyouts,
setActiveFlyout: jest.fn(),
setFlyoutWidth: jest.fn(),
goBack: jest.fn(),
goToFlyout: jest.fn(),
getHistoryItems: jest.fn(() => []),
addUnmanagedFlyout: jest.fn(),
closeUnmanagedFlyout: jest.fn(),
setPushPadding: jest.fn(),
});

export const createMockState = () => ({
Expand All @@ -36,6 +44,7 @@ export const createMockState = () => ({
*/
export const createFlyoutManagerMock = () => ({
state: createMockState(),
historyItems: [],
...createMockFunctions(),
});

Expand Down
12 changes: 12 additions & 0 deletions packages/eui/src/components/flyout/manager/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@
import {
addFlyout,
closeFlyout,
closeAllFlyouts,
setActiveFlyout,
setFlyoutWidth,
setLayoutMode,
setActivityStage,
ACTION_ADD,
ACTION_CLOSE,
ACTION_CLOSE_ALL,
ACTION_SET_ACTIVE,
ACTION_SET_WIDTH,
ACTION_SET_LAYOUT_MODE,
Expand Down Expand Up @@ -130,6 +132,16 @@ describe('flyout manager actions', () => {
});
});

describe('closeAllFlyouts', () => {
it('should create close all flyouts action', () => {
const action = closeAllFlyouts();

expect(action).toEqual({
type: ACTION_CLOSE_ALL,
});
});
});

describe('setActiveFlyout', () => {
it('should create set active flyout action with flyout ID', () => {
const action = setActiveFlyout('child-1');
Expand Down
13 changes: 13 additions & 0 deletions packages/eui/src/components/flyout/manager/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ interface BaseAction {
export const ACTION_ADD = `${PREFIX}/add` as const;
/** Dispatched to remove a flyout from the manager (usually on close/unmount). */
export const ACTION_CLOSE = `${PREFIX}/close` as const;
/** Dispatched to remove all flyouts from the manager. */
export const ACTION_CLOSE_ALL = `${PREFIX}/closeAll` as const;
/** Dispatched to set which flyout is currently active within the session. */
export const ACTION_SET_ACTIVE = `${PREFIX}/setActive` as const;
/** Dispatched when an active flyout's pixel width changes (for responsive layout). */
Expand Down Expand Up @@ -60,6 +62,11 @@ export interface CloseFlyoutAction extends BaseAction {
flyoutId: string;
}

/** Remove all flyouts from manager state. */
export interface CloseAllFlyoutsAction extends BaseAction {
type: typeof ACTION_CLOSE_ALL;
}

/** Set the active flyout within the current session (or clear with `null`). */
export interface SetActiveFlyoutAction extends BaseAction {
type: typeof ACTION_SET_ACTIVE;
Expand Down Expand Up @@ -118,6 +125,7 @@ export interface CloseUnmanagedFlyoutAction extends BaseAction {
export type Action =
| AddFlyoutAction
| CloseFlyoutAction
| CloseAllFlyoutsAction
| SetActiveFlyoutAction
| SetWidthAction
| SetLayoutModeAction
Expand Down Expand Up @@ -153,6 +161,11 @@ export const closeFlyout = (flyoutId: string): CloseFlyoutAction => ({
flyoutId,
});

/** Unregister all flyouts. */
export const closeAllFlyouts = (): CloseAllFlyoutsAction => ({
type: ACTION_CLOSE_ALL,
});

/** Set or clear the active flyout for the current session. */
export const setActiveFlyout = (
flyoutId: string | null
Expand Down
15 changes: 4 additions & 11 deletions packages/eui/src/components/flyout/manager/flyout_main.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { render } from '../../../test/rtl';
import { EuiFlyoutMain } from './flyout_main';
import { EuiFlyoutManager } from './provider';
import { LEVEL_MAIN, PROPERTY_LEVEL } from './const';
import { createFlyoutManagerMock } from './__mocks__';

// Mock managed flyout so we can observe props passed through
jest.mock('./flyout_managed', () => ({
Expand All @@ -27,19 +28,11 @@ jest.mock('./flyout_managed', () => ({
),
}));

const mockUseFlyoutManager = createFlyoutManagerMock();

// Keep layout/ID hooks deterministic
jest.mock('./hooks', () => ({
useFlyoutManager: () => ({
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: [],
}),
useFlyoutManager: () => mockUseFlyoutManager,
useHasChildFlyout: () => false,
useFlyoutId: (id?: string) => id ?? 'generated-id',
}));
Expand Down
111 changes: 89 additions & 22 deletions packages/eui/src/components/flyout/manager/flyout_managed.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ jest.mock('../flyout.component', () => {

// Shared mock functions - must be defined in module scope for Jest
const mockCloseFlyout = jest.fn();
const mockCloseAllFlyouts = jest.fn();

// Create mock state and functions once at module scope to avoid redundant object creation
const mockState = {
Expand All @@ -70,6 +71,7 @@ const mockFunctions = {
dispatch: jest.fn(),
addFlyout: jest.fn(),
closeFlyout: mockCloseFlyout,
closeAllFlyouts: mockCloseAllFlyouts,
setActiveFlyout: jest.fn(),
setFlyoutWidth: jest.fn(),
goBack: jest.fn(),
Expand Down Expand Up @@ -152,7 +154,7 @@ describe('EuiManagedFlyout', () => {
expect(el).toHaveAttribute(PROPERTY_LEVEL, LEVEL_MAIN);
});

it('calls the unregister callback prop when onClose', () => {
it('calls closeAllFlyouts during cleanup when main flyout unmounts', () => {
const onClose = jest.fn();

const { getByTestSubject, unmount } = renderInProvider(
Expand All @@ -166,12 +168,36 @@ describe('EuiManagedFlyout', () => {
// The onClose should be called when the flyout is clicked
expect(onClose).toHaveBeenCalled();

// The closeAllFlyouts should be called when the component unmounts (cleanup)
act(() => {
unmount();
});

expect(mockCloseAllFlyouts).toHaveBeenCalled();
expect(mockCloseFlyout).not.toHaveBeenCalled();
});

it('calls closeFlyout during cleanup when child flyout unmounts', () => {
const onClose = jest.fn();

const { getByTestSubject, unmount } = renderInProvider(
<EuiManagedFlyout id="close-me" level={LEVEL_CHILD} onClose={onClose} />
);

act(() => {
userEvent.click(getByTestSubject('managed-flyout'));
});

// The onClose should be called when the flyout is clicked
expect(onClose).toHaveBeenCalled();

// The closeFlyout should be called when the component unmounts (cleanup)
act(() => {
unmount();
});

expect(mockCloseFlyout).toHaveBeenCalledWith('close-me');
expect(mockCloseAllFlyouts).not.toHaveBeenCalled();
});

it('registers child flyout and sets data-level child', () => {
Expand Down Expand Up @@ -463,7 +489,7 @@ describe('EuiManagedFlyout', () => {
expect(onClose).not.toHaveBeenCalled();

// Clear any calls from mount
mockCloseFlyout.mockClear();
mockCloseAllFlyouts.mockClear();

// Unmount the component to trigger cleanup
act(() => {
Expand Down Expand Up @@ -504,13 +530,13 @@ describe('EuiManagedFlyout', () => {
});

describe('manager state update ordering', () => {
it('calls closeFlyout before parent onClose callback', () => {
it('calls closeAllFlyouts before parent onClose callback', () => {
const onClose = jest.fn();
const callOrder: string[] = [];

// Track call order
mockCloseFlyout.mockImplementation(() => {
callOrder.push('closeFlyout');
mockCloseAllFlyouts.mockImplementation(() => {
callOrder.push('closeAllFlyouts');
});
onClose.mockImplementation(() => {
callOrder.push('onClose');
Expand All @@ -530,13 +556,13 @@ describe('EuiManagedFlyout', () => {
userEvent.click(getByTestSubject('managed-flyout'));
});

// Verify closeFlyout was called BEFORE onClose
expect(callOrder).toEqual(['closeFlyout', 'onClose']);
expect(mockCloseFlyout).toHaveBeenCalledWith('ordering-test');
// Verify closeAllFlyouts was called BEFORE onClose
expect(callOrder).toEqual(['closeAllFlyouts', 'onClose']);
expect(mockCloseAllFlyouts).toHaveBeenCalled();
expect(onClose).toHaveBeenCalled();
});

it('prevents duplicate closeFlyout calls when closing via user interaction', () => {
it('prevents duplicate closeAllFlyouts calls when closing via user interaction', () => {
const onClose = jest.fn();

const { getByTestSubject, unmount } = renderInProvider(
Expand All @@ -549,23 +575,23 @@ describe('EuiManagedFlyout', () => {
);

// Clear any setup calls
mockCloseFlyout.mockClear();
mockCloseAllFlyouts.mockClear();

// User closes the flyout
act(() => {
userEvent.click(getByTestSubject('managed-flyout'));
});

// closeFlyout should be called once from the onClose handler
expect(mockCloseFlyout).toHaveBeenCalledTimes(1);
// closeAllFlyouts should be called once from the onClose handler
expect(mockCloseAllFlyouts).toHaveBeenCalledTimes(1);

// Manual, duplicate cleanup call
act(() => {
unmount();
});

// Should still be called only once total
expect(mockCloseFlyout).toHaveBeenCalledTimes(1);
expect(mockCloseAllFlyouts).toHaveBeenCalledTimes(1);
});

it('handles cascade close correctly when main flyout closes', () => {
Expand Down Expand Up @@ -603,10 +629,51 @@ describe('EuiManagedFlyout', () => {
});

// Manager should be notified to handle cascade close
expect(mockCloseFlyout).toHaveBeenCalledWith('main-flyout');
expect(mockCloseAllFlyouts).toHaveBeenCalled();
expect(onCloseMain).toHaveBeenCalled();
});

it('calls closeFlyout when closing a child flyout', () => {
const onCloseMain = jest.fn();
const onCloseChild = jest.fn();

// Simulate a main flyout with child
const { container } = renderInProvider(
<>
<EuiManagedFlyout
id="main-flyout"
level={LEVEL_MAIN}
onClose={onCloseMain}
flyoutMenuProps={{ title: 'Main Flyout' }}
data-test-subj="main-flyout-element"
/>
<EuiManagedFlyout
id="child-flyout"
level={LEVEL_CHILD}
onClose={onCloseChild}
data-test-subj="child-flyout-element"
/>
</>
);

// Find the child flyout specifically
const childFlyout = container.querySelector('[id="child-flyout"]');
expect(childFlyout).toBeInTheDocument();

// Close the child flyout
act(() => {
if (childFlyout) {
userEvent.click(childFlyout);
}
});

// Child flyouts should call closeFlyout, not closeAllFlyouts
expect(mockCloseFlyout).toHaveBeenCalledWith('child-flyout');
expect(mockCloseFlyout).toHaveBeenCalledTimes(1);
expect(mockCloseAllFlyouts).not.toHaveBeenCalled();
expect(onCloseChild).toHaveBeenCalled();
});

it('uses flushSync to ensure synchronous state update before DOM cleanup', () => {
const onClose = jest.fn();

Expand All @@ -621,7 +688,7 @@ describe('EuiManagedFlyout', () => {

// Clear any setup calls
mockFlushSync.mockClear();
mockCloseFlyout.mockClear();
mockCloseAllFlyouts.mockClear();

// Trigger close via user interaction
act(() => {
Expand All @@ -632,14 +699,14 @@ describe('EuiManagedFlyout', () => {
expect(mockFlushSync).toHaveBeenCalledTimes(1);
expect(mockFlushSync).toHaveBeenCalledWith(expect.any(Function));

// Verify closeFlyout was called (inside flushSync)
expect(mockCloseFlyout).toHaveBeenCalledWith('flush-sync-test');
// Verify closeAllFlyouts was called (inside flushSync)
expect(mockCloseAllFlyouts).toHaveBeenCalled();

// Verify onClose was called after the synchronous state update
expect(onClose).toHaveBeenCalled();
});

it('calls closeFlyout inside flushSync callback', () => {
it('calls closeAllFlyouts inside flushSync callback', () => {
const onClose = jest.fn();
const callOrder: string[] = [];

Expand All @@ -650,8 +717,8 @@ describe('EuiManagedFlyout', () => {
callOrder.push('flushSync-end');
});

mockCloseFlyout.mockImplementation(() => {
callOrder.push('closeFlyout');
mockCloseAllFlyouts.mockImplementation(() => {
callOrder.push('closeAllFlyouts');
});

onClose.mockImplementation(() => {
Expand All @@ -675,10 +742,10 @@ describe('EuiManagedFlyout', () => {
userEvent.click(getByTestSubject('managed-flyout'));
});

// Verify closeFlyout is called INSIDE flushSync, and onClose is called AFTER
// Verify closeAllFlyouts is called INSIDE flushSync, and onClose is called AFTER
expect(callOrder).toEqual([
'flushSync-start',
'closeFlyout',
'closeAllFlyouts',
'flushSync-end',
'onClose',
]);
Expand Down
Loading