diff --git a/eui_112.2.0-get-flyout-manager-store-002.tgz b/eui_112.2.0-get-flyout-manager-store-002.tgz new file mode 100644 index 0000000000000..c41772cd003a6 Binary files /dev/null and b/eui_112.2.0-get-flyout-manager-store-002.tgz differ diff --git a/examples/flyout_system/public/components/_flyout_with_overlays.tsx b/examples/flyout_system/public/components/_flyout_with_overlays.tsx index 7c9a9d0399157..71d3b551b0e39 100644 --- a/examples/flyout_system/public/components/_flyout_with_overlays.tsx +++ b/examples/flyout_system/public/components/_flyout_with_overlays.tsx @@ -260,17 +260,6 @@ const SessionFlyout: React.FC = React.memo((props) => { }, [title]); const handleCloseFlyout = useCallback(() => { - // Close child flyout first if it's open - if (childFlyoutRefA.current) { - childFlyoutRefA.current.close(); - childFlyoutRefA.current = null; - } - if (childFlyoutRefB.current) { - childFlyoutRefB.current.close(); - childFlyoutRefB.current = null; - } - - // Then close main flyout if (flyoutRef.current) { flyoutRef.current.close(); flyoutRef.current = null; diff --git a/examples/flyout_system/public/components/_parent_child_session_bug.tsx b/examples/flyout_system/public/components/_parent_child_session_bug.tsx new file mode 100644 index 0000000000000..08640c07377b2 --- /dev/null +++ b/examples/flyout_system/public/components/_parent_child_session_bug.tsx @@ -0,0 +1,228 @@ +/* + * 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", the "GNU Affero General Public License v3.0 only", 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", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +import React from 'react'; +import { + EuiButton, + EuiButtonEmpty, + EuiCallOut, + EuiCode, + EuiFlexGroup, + EuiFlexItem, + EuiFlyoutBody, + EuiFlyoutFooter, + EuiFlyoutHeader, + EuiPanel, + EuiSpacer, + EuiText, + EuiTitle, +} from '@elastic/eui'; +import type { OverlayStart } from '@kbn/core/public'; + +interface Props { + overlays: OverlayStart; +} + +/** + * This component demonstrates a bug in the SystemFlyoutService: + * When a parent flyout (session: 'start') closes, child flyouts (session: 'inherit') + * remain open instead of being automatically closed. + */ +export const ParentChildSessionBug: React.FC = ({ overlays }) => { + const openParentFlyout = () => { + const parentFlyout = overlays.openSystemFlyout( + parentFlyout.close()} />, + { + id: 'parent-flyout-demo', + title: 'Parent Flyout (session: start)', + session: 'start', + size: 'm', + type: 'overlay', + ownFocus: true, + outsideClickCloses: false, + } + ); + }; + + return ( + + +

Parent-Child Session Lifecycle Bug

+ + + +

+ This example demonstrates that child flyouts with{' '} + session: "inherit" + do not automatically close when their parent flyout closes. +

+ + + +

Steps to reproduce:

+
    +
  1. Click the button below to open the parent flyout
    2. +
  2. Inside the parent flyout, click "Open Child Flyout"
    3. +
  3. + Notice the child flyout appears (correctly) with{' '} + session: "inherit" +
    4. +
  4. Close the parent flyout by clicking the "Close Parent" button
    5. +
  5. + BUG: The child flyout remains open! +
    6. +
+ +

Expected behavior:

+

+ When a flyout with session: "start" closes, all child flyouts + with session: "inherit" should automatically close as they + are part of the same session lifecycle. +

+ +

Root cause:

+

+ The SystemFlyoutService tracks active flyouts but does not implement + session-based lifecycle management. It needs to: +

+
    +
  • Track parent-child relationships based on session parameter
    • +
  • + When closing a parent (session: start), find and close all children (session: inherit) +
    • +
+ + + + Open Parent Flyout (Reproduce Bug) + + + ); +}; + +const ParentFlyoutContent: React.FC<{ overlays: OverlayStart; onClose: () => void }> = ({ + overlays, + onClose, +}) => { + const openChildFlyout = () => { + overlays.openSystemFlyout(, { + id: 'child-flyout-demo', + title: 'Child Flyout (session: inherit)', + session: 'inherit', + size: 's', + type: 'overlay', + outsideClickCloses: false, + }); + }; + + return ( + <> + + +

Parent Flyout

+ + + + +

+ This flyout was opened with session: "start". +

+ + + +

+ Now click the button below to open a child flyout. The child will use{' '} + session: "inherit" which means it should be part of this + parent's session. +

+ + + + Open Child Flyout + + + +

+ After opening the child flyout above, close this parent flyout using the button below. + Watch what happens to the child flyout - it should close automatically but it + doesn't (this is the bug). +

+ + + +

+ Check the browser console for debug logs showing: +

+
    +
  • When flyouts are opened (with their session type)
    • +
  • Active flyouts list
    • +
  • When flyouts are closed
    • +
+

+ You'll see the child flyout remains in the active flyouts list after the parent + closes. +

+ + + + + + + Close Parent (Trigger Bug) + + + + + + ); +}; + +const ChildFlyoutContent: React.FC = () => { + return ( + <> + + +

Child Flyout

+ + + + +

+ This flyout was opened with session: "inherit". +

+

It correctly appears as a child (stacked on top of the parent).

+ + + +

+ When you close the parent flyout, this child flyout should automatically close since + it's part of the same session (session: "inherit"). +

+

+ But it doesn't! The child stays open even after the parent closes. +

+ + + +

Go back to the parent flyout and click "Close Parent" to see the bug.

+

This child flyout will remain open and you'll need to close it manually.

+ + + + +

This flyout should have closed automatically when the parent closed.

+ + + + ); +}; diff --git a/examples/flyout_system/public/components/app.tsx b/examples/flyout_system/public/components/app.tsx index 0a7584bc27885..f40b654b3c141 100644 --- a/examples/flyout_system/public/components/app.tsx +++ b/examples/flyout_system/public/components/app.tsx @@ -16,6 +16,7 @@ import { BrowserRouter as Router } from '@kbn/shared-ux-router'; import { FlyoutWithComponent } from './_flyout_with_component'; import { FlyoutWithOverlays } from './_flyout_with_overlays'; +import { ParentChildSessionBug } from './_parent_child_session_bug'; interface AppDeps { basename: string; @@ -41,6 +42,10 @@ const AppContent: React.FC = ({ overlays, rendering }) => { > + + + + diff --git a/package.json b/package.json index bc2e90167d45a..483844f5c2d37 100644 --- a/package.json +++ b/package.json @@ -132,8 +132,9 @@ "@elastic/ecs": "9.0.0", "@elastic/elasticsearch": "9.2.0", "@elastic/ems-client": "8.6.3", - "@elastic/eui": "112.2.0", + "@elastic/eui": "file:./eui_112.2.0-get-flyout-manager-store-002.tgz", "@elastic/eui-theme-borealis": "5.4.0", + "@elastic/eui-theme-common": "7.3.0", "@elastic/filesaver": "1.1.2", "@elastic/kibana-d3-color": "npm:@elastic/kibana-d3-color@2.0.1", "@elastic/monaco-esql": "3.1.16", @@ -2118,4 +2119,4 @@ "yarn-deduplicate": "6.0.2" }, "packageManager": "yarn@1.22.21" -} \ No newline at end of file +} diff --git a/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_ref.ts b/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_ref.ts index 7035671e960d7..b4a14e881aadc 100644 --- a/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_ref.ts +++ b/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_ref.ts @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { Subject } from 'rxjs'; +import { Subject, firstValueFrom } from 'rxjs'; import { unmountComponentAtNode } from 'react-dom'; import type { OverlayRef } from '@kbn/core-mount-utils-browser'; @@ -17,18 +17,22 @@ import type { OverlayRef } from '@kbn/core-mount-utils-browser'; */ export class SystemFlyoutRef implements OverlayRef { public readonly onClose: Promise; + private _isClosed = false; private closeSubject = new Subject(); private container: HTMLElement; - private isClosed = false; constructor(container: HTMLElement) { this.container = container; - this.onClose = this.closeSubject.toPromise(); + this.onClose = firstValueFrom(this.closeSubject); + } + + public get isClosed(): boolean { + return this._isClosed; } public close(): Promise { - if (!this.isClosed) { - this.isClosed = true; + if (!this._isClosed) { + this._isClosed = true; unmountComponentAtNode(this.container); this.container.remove(); diff --git a/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_service.test.tsx b/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_service.test.tsx index 8a14d84a688e6..c22034ec2130b 100644 --- a/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_service.test.tsx +++ b/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_service.test.tsx @@ -14,10 +14,36 @@ import { i18nServiceMock } from '@kbn/core-i18n-browser-mocks'; import { themeServiceMock } from '@kbn/core-theme-browser-mocks'; import { userProfileServiceMock } from '@kbn/core-user-profile-browser-mocks'; import { SystemFlyoutService } from './system_flyout_service'; +import type { SystemFlyoutRef } from './system_flyout_ref'; import type { OverlayRef } from '@kbn/core-mount-utils-browser'; import type { OverlaySystemFlyoutStart } from '@kbn/core-overlays-browser'; import React from 'react'; +interface FlyoutManagerEvent { + type: 'CLOSE_SESSION'; + session: { mainFlyoutId: string; childFlyoutId: string | null }; +} + +const eventListeners = new Set<(event: FlyoutManagerEvent) => void>(); +const mockSubscribeToEvents = jest.fn((listener: (event: FlyoutManagerEvent) => void) => { + eventListeners.add(listener); + return () => { + eventListeners.delete(listener); + }; +}); + +const emitEvent = (event: FlyoutManagerEvent) => { + eventListeners.forEach((listener) => listener(event)); +}; + +jest.mock('@elastic/eui', () => { + const actual = jest.requireActual('@elastic/eui'); + return { + ...actual, + getFlyoutManagerStore: jest.fn(() => ({ subscribeToEvents: mockSubscribeToEvents })), + }; +}); + const analyticsMock = analyticsServiceMock.createAnalyticsServiceStart(); const i18nMock = i18nServiceMock.createStartContract(); const themeMock = themeServiceMock.createStartContract(); @@ -26,6 +52,8 @@ const userProfileMock = userProfileServiceMock.createStart(); beforeEach(() => { mockReactDomRender.mockClear(); mockReactDomUnmount.mockClear(); + mockSubscribeToEvents.mockClear(); + eventListeners.clear(); }); afterEach(() => { @@ -58,6 +86,26 @@ describe('SystemFlyoutService', () => { }); describe('openSystemFlyout()', () => { + it('closes the flyout when a CLOSE_SESSION event removes its session', () => { + const ref = systemFlyouts.open(
System flyout content
, { + id: 'parent-flyout-demo', + session: 'start', + }); + + expect(mockReactDomUnmount).not.toHaveBeenCalled(); + + emitEvent({ + type: 'CLOSE_SESSION', + session: { + mainFlyoutId: 'parent-flyout-demo', + childFlyoutId: null, + }, + }); + + expect((ref as SystemFlyoutRef).isClosed).toBe(true); + expect(mockReactDomUnmount).toHaveBeenCalledTimes(1); + }); + it('renders a system flyout to the DOM', () => { expect(mockReactDomRender).not.toHaveBeenCalled(); systemFlyouts.open(
System flyout content
); diff --git a/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_service.tsx b/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_service.tsx index 966f94646ee3d..46fcf35dea963 100644 --- a/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_service.tsx +++ b/src/core/packages/overlays/browser-internal/src/flyout/system_flyout_service.tsx @@ -8,19 +8,20 @@ */ import React from 'react'; -import { v4 as uuidV4 } from 'uuid'; import { render } from 'react-dom'; +import { v4 as uuidV4 } from 'uuid'; + +import { EuiFlyout, getFlyoutManagerStore, type EuiFlyoutMenuProps } from '@elastic/eui'; import type { AnalyticsServiceStart } from '@kbn/core-analytics-browser'; -import type { ThemeServiceStart } from '@kbn/core-theme-browser'; -import type { UserProfileService } from '@kbn/core-user-profile-browser'; import type { I18nStart } from '@kbn/core-i18n-browser'; import type { OverlayRef } from '@kbn/core-mount-utils-browser'; import type { OverlaySystemFlyoutOpenOptions, OverlaySystemFlyoutStart, } from '@kbn/core-overlays-browser'; +import type { ThemeServiceStart } from '@kbn/core-theme-browser'; +import type { UserProfileService } from '@kbn/core-user-profile-browser'; import { KibanaRenderContextProvider } from '@kbn/react-kibana-context-render'; -import { EuiFlyout, type EuiFlyoutMenuProps } from '@elastic/eui'; import { SystemFlyoutRef } from './system_flyout_ref'; interface SystemFlyoutStartDeps { @@ -82,8 +83,37 @@ export class SystemFlyoutService { mergedFlyoutMenuProps = { title, ...flyoutMenuProps }; } - // Render the flyout content using EuiFlyout with session="start" - // This ensures full EUI Flyout System integration as a new MAIN flyout. + // Subscribe to EUI flyout manager store to detect cascade closes + // This ensures child flyouts close when their parent closes, even across separate React roots + if (session !== 'never') { + // Use the EUI flyout ID (from options.id) for store lookups, not our internal container ID + const euiFlyoutId = options.id || flyoutId; + + const { subscribeToEvents } = getFlyoutManagerStore(); + + const unsubscribe = subscribeToEvents((event) => { + if (event.type !== 'CLOSE_SESSION') { + return; + } + + const { mainFlyoutId, childFlyoutId } = event.session; + const shouldClose = euiFlyoutId === mainFlyoutId || euiFlyoutId === childFlyoutId; + + if (shouldClose && !flyoutRef.isClosed) { + flyoutRef.close(); + unsubscribe(); + this.activeFlyouts.delete(flyoutId); + } + }); + + // Clean up subscription when flyout closes normally + flyoutRef.onClose.then(() => { + unsubscribe(); + }); + } + + // Render the flyout content using EuiFlyout with session management + // This ensures full EUI Flyout System integration render(