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
Original file line number Diff line number Diff line change
Expand Up @@ -260,17 +260,6 @@ const SessionFlyout: React.FC<SessionFlyoutProps> = 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -17,18 +17,22 @@ import type { OverlayRef } from '@kbn/core-mount-utils-browser';
*/
export class SystemFlyoutRef implements OverlayRef {
public readonly onClose: Promise<void>;
private _isClosed = false;
private closeSubject = new Subject<void>();
private container: HTMLElement;
private isClosed = false;

constructor(container: HTMLElement) {
this.container = container;
this.onClose = this.closeSubject.toPromise();
this.onClose = firstValueFrom(this.closeSubject);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The firstValueFrom is an unrelated deprecated code cleanup

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: if you want to avoid errors thrown if closeSubject closes the observable before emitting, you can specify a fallback value as a 2nd parameter to firstValueFrom.

}

public get isClosed(): boolean {
return this._isClosed;
}

public close(): Promise<void> {
if (!this.isClosed) {
this.isClosed = true;
if (!this._isClosed) {
this._isClosed = true;
unmountComponentAtNode(this.container);
this.container.remove();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -26,6 +52,8 @@ const userProfileMock = userProfileServiceMock.createStart();
beforeEach(() => {
mockReactDomRender.mockClear();
mockReactDomUnmount.mockClear();
mockSubscribeToEvents.mockClear();
eventListeners.clear();
});

afterEach(() => {
Expand Down Expand Up @@ -58,6 +86,26 @@ describe('SystemFlyoutService', () => {
});

describe('openSystemFlyout()', () => {
it('closes the flyout when a CLOSE_SESSION event removes its session', () => {
const ref = systemFlyouts.open(<div>System flyout content</div>, {
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(<div>System flyout content</div>);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
<KibanaRenderContextProvider
analytics={analytics}
Expand Down
Loading