Skip to content
Closed
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
Binary file added eui_112.2.0-get-flyout-manager-store-002.tgz
Binary file not shown.
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
@@ -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<Props> = ({ overlays }) => {
const openParentFlyout = () => {
const parentFlyout = overlays.openSystemFlyout(
<ParentFlyoutContent overlays={overlays} onClose={() => parentFlyout.close()} />,
{
id: 'parent-flyout-demo',
title: 'Parent Flyout (session: start)',
session: 'start',
size: 'm',
type: 'overlay',
ownFocus: true,
outsideClickCloses: false,
}
);
};

return (
<EuiPanel hasBorder>
<EuiTitle size="s">
<h3>Parent-Child Session Lifecycle Bug</h3>
</EuiTitle>
<EuiSpacer size="m" />
<EuiCallOut title="Bug Demonstration" color="warning" iconType="bug">
<p>
This example demonstrates that child flyouts with{' '}
<EuiCode>session: &quot;inherit&quot;</EuiCode>
do not automatically close when their parent flyout closes.
</p>
</EuiCallOut>
<EuiSpacer size="m" />
<EuiText size="s">
<h4>Steps to reproduce:</h4>
<ol>
<li>Click the button below to open the parent flyout</li>
<li>Inside the parent flyout, click &quot;Open Child Flyout&quot;</li>
<li>
Notice the child flyout appears (correctly) with{' '}
<EuiCode>session: &quot;inherit&quot;</EuiCode>
</li>
<li>Close the parent flyout by clicking the &quot;Close Parent&quot; button</li>
<li>
<strong>BUG:</strong> The child flyout remains open!
</li>
</ol>
<EuiSpacer size="s" />
<h4>Expected behavior:</h4>
<p>
When a flyout with <EuiCode>session: &quot;start&quot;</EuiCode> closes, all child flyouts
with <EuiCode>session: &quot;inherit&quot;</EuiCode> should automatically close as they
are part of the same session lifecycle.
</p>
<EuiSpacer size="s" />
<h4>Root cause:</h4>
<p>
The <EuiCode>SystemFlyoutService</EuiCode> tracks active flyouts but does not implement
session-based lifecycle management. It needs to:
</p>
<ul>
<li>Track parent-child relationships based on session parameter</li>
<li>
When closing a parent (session: start), find and close all children (session: inherit)
</li>
</ul>
</EuiText>
<EuiSpacer size="m" />
<EuiButton onClick={openParentFlyout} fill>
Open Parent Flyout (Reproduce Bug)
</EuiButton>
</EuiPanel>
);
};

const ParentFlyoutContent: React.FC<{ overlays: OverlayStart; onClose: () => void }> = ({
overlays,
onClose,
}) => {
const openChildFlyout = () => {
overlays.openSystemFlyout(<ChildFlyoutContent />, {
id: 'child-flyout-demo',
title: 'Child Flyout (session: inherit)',
session: 'inherit',
size: 's',
type: 'overlay',
outsideClickCloses: false,
});
};

return (
<>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="m">
<h2>Parent Flyout</h2>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
<EuiCallOut title="This is the parent flyout" color="primary" iconType="iInCircle">
<p>
This flyout was opened with <EuiCode>session: &quot;start&quot;</EuiCode>.
</p>
</EuiCallOut>
<EuiSpacer size="l" />
<EuiText>
<p>
Now click the button below to open a child flyout. The child will use{' '}
<EuiCode>session: &quot;inherit&quot;</EuiCode> which means it should be part of this
parent&apos;s session.
</p>
</EuiText>
<EuiSpacer size="m" />
<EuiButton onClick={openChildFlyout} iconType="arrowRight">
Open Child Flyout
</EuiButton>
<EuiSpacer size="l" />
<EuiCallOut title="Now close this parent flyout" color="warning">
<p>
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&apos;t (this is the bug).
</p>
</EuiCallOut>
<EuiSpacer size="m" />
<EuiText size="s">
<p>
<strong>Check the browser console</strong> for debug logs showing:
</p>
<ul>
<li>When flyouts are opened (with their session type)</li>
<li>Active flyouts list</li>
<li>When flyouts are closed</li>
</ul>
<p>
You&apos;ll see the child flyout remains in the active flyouts list after the parent
closes.
</p>
</EuiText>
</EuiFlyoutBody>
<EuiFlyoutFooter>
<EuiFlexGroup justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<EuiButtonEmpty onClick={onClose} iconType="cross">
Close Parent (Trigger Bug)
</EuiButtonEmpty>
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlyoutFooter>
</>
);
};

const ChildFlyoutContent: React.FC = () => {
return (
<>
<EuiFlyoutHeader hasBorder>
<EuiTitle size="s">
<h3>Child Flyout</h3>
</EuiTitle>
</EuiFlyoutHeader>
<EuiFlyoutBody>
<EuiCallOut title="This is the child flyout" color="success" iconType="check">
<p>
This flyout was opened with <EuiCode>session: &quot;inherit&quot;</EuiCode>.
</p>
<p>It correctly appears as a child (stacked on top of the parent).</p>
</EuiCallOut>
<EuiSpacer size="l" />
<EuiCallOut
title="🐛 Bug: This flyout should close automatically"
color="danger"
iconType="alert"
>
<p>
When you close the parent flyout, this child flyout should automatically close since
it&apos;s part of the same session (<EuiCode>session: &quot;inherit&quot;</EuiCode>).
</p>
<p>
<strong>But it doesn&apos;t!</strong> The child stays open even after the parent closes.
</p>
</EuiCallOut>
<EuiSpacer size="m" />
<EuiText size="s">
<p>Go back to the parent flyout and click &quot;Close Parent&quot; to see the bug.</p>
<p>This child flyout will remain open and you&apos;ll need to close it manually.</p>
</EuiText>
</EuiFlyoutBody>
<EuiFlyoutFooter>
<EuiText size="xs" color="subdued">
<p>This flyout should have closed automatically when the parent closed.</p>
</EuiText>
</EuiFlyoutFooter>
</>
);
};
5 changes: 5 additions & 0 deletions examples/flyout_system/public/components/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,6 +42,10 @@ const AppContent: React.FC<AppContentDeps> = ({ overlays, rendering }) => {
>
<EuiPageTemplate.Header iconType="logoElastic" pageTitle="Flyout System Example" />

<EuiPageTemplate.Section grow={false} alignment="top">
<ParentChildSessionBug overlays={overlays} />
</EuiPageTemplate.Section>

<EuiPageTemplate.Section grow={false} alignment="top">
<FlyoutWithComponent />
</EuiPageTemplate.Section>
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -2118,4 +2119,4 @@
"yarn-deduplicate": "6.0.2"
},
"packageManager": "yarn@1.22.21"
}
}
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);
}

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
Loading
Loading