Skip to content

Commit 88d9d68

Browse files
tsullivantsullivan
committed
Render child flyout if hasActiveSession and session === undefined 
1 parent f904630 commit 88d9d68

3 files changed

Lines changed: 122 additions & 17 deletions

File tree

‎packages/eui/src/components/flyout/flyout.test.tsx‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,4 +437,116 @@ describe('EuiFlyout', () => {
437437
).not.toBeTruthy();
438438
});
439439
});
440+
441+
describe('flyout routing logic', () => {
442+
// Mock the manager hooks to control routing behavior
443+
const mockUseHasActiveSession = jest.fn();
444+
const mockUseIsInManagedFlyout = jest.fn();
445+
446+
beforeEach(() => {
447+
jest.clearAllMocks();
448+
// Mock the manager hooks
449+
jest.doMock('./manager', () => ({
450+
...jest.requireActual('./manager'),
451+
useHasActiveSession: mockUseHasActiveSession,
452+
useIsInManagedFlyout: mockUseIsInManagedFlyout,
453+
}));
454+
});
455+
456+
afterEach(() => {
457+
jest.dontMock('./manager');
458+
});
459+
460+
it('routes to child flyout when session is undefined and there is an active session', () => {
461+
// Setup: There's an active session but flyout is not in managed context
462+
mockUseHasActiveSession.mockReturnValue(true);
463+
mockUseIsInManagedFlyout.mockReturnValue(false);
464+
465+
const { getByTestSubject } = render(
466+
<EuiFlyout
467+
onClose={() => {}}
468+
data-test-subj="child-flyout"
469+
// session is undefined (not explicitly set)
470+
/>
471+
);
472+
473+
// Should render as child flyout (EuiFlyoutChild)
474+
const flyout = getByTestSubject('child-flyout');
475+
expect(flyout).toBeInTheDocument();
476+
});
477+
478+
it('routes to main flyout when session is explicitly true', () => {
479+
// Setup: There's an active session and flyout is not in managed context
480+
mockUseHasActiveSession.mockReturnValue(true);
481+
mockUseIsInManagedFlyout.mockReturnValue(false);
482+
483+
const { getByTestSubject } = render(
484+
<EuiFlyout
485+
onClose={() => {}}
486+
data-test-subj="main-flyout"
487+
session={true} // Explicitly creating a new session
488+
flyoutMenuProps={{ title: 'Test Main Flyout' }} // Required for managed flyouts
489+
/>
490+
);
491+
492+
// Should render as main flyout (EuiFlyoutMain)
493+
const flyout = getByTestSubject('main-flyout');
494+
expect(flyout).toBeInTheDocument();
495+
});
496+
497+
it('routes to main flyout when session is explicitly false and there is an active session', () => {
498+
// Setup: There's an active session and flyout is not in managed context
499+
mockUseHasActiveSession.mockReturnValue(true);
500+
mockUseIsInManagedFlyout.mockReturnValue(false);
501+
502+
const { getByTestSubject } = render(
503+
<EuiFlyout
504+
onClose={() => {}}
505+
data-test-subj="main-flyout"
506+
session={false} // Explicitly not creating a new session, but still routes to main
507+
flyoutMenuProps={{ title: 'Test Main Flyout' }} // Required for managed flyouts
508+
/>
509+
);
510+
511+
// Should render as main flyout (EuiFlyoutMain)
512+
const flyout = getByTestSubject('main-flyout');
513+
expect(flyout).toBeInTheDocument();
514+
});
515+
516+
it('routes to child flyout when in managed context and there is an active session', () => {
517+
// Setup: There's an active session and flyout is in managed context
518+
mockUseHasActiveSession.mockReturnValue(true);
519+
mockUseIsInManagedFlyout.mockReturnValue(true);
520+
521+
const { getByTestSubject } = render(
522+
<EuiFlyout
523+
onClose={() => {}}
524+
data-test-subj="child-flyout"
525+
session={undefined} // Not explicitly set
526+
/>
527+
);
528+
529+
// Should render as child flyout (EuiFlyoutChild)
530+
const flyout = getByTestSubject('child-flyout');
531+
expect(flyout).toBeInTheDocument();
532+
});
533+
534+
it('routes to standard flyout when there is no active session', () => {
535+
// Setup: No active session
536+
mockUseHasActiveSession.mockReturnValue(false);
537+
mockUseIsInManagedFlyout.mockReturnValue(false);
538+
539+
const { getByTestSubject } = render(
540+
<EuiFlyout
541+
onClose={() => {}}
542+
data-test-subj="standard-flyout"
543+
session={undefined} // Not explicitly set
544+
/>
545+
);
546+
547+
// Should render as standard flyout (EuiFlyoutComponent)
548+
const flyout = getByTestSubject('standard-flyout');
549+
expect(flyout).toBeInTheDocument();
550+
});
551+
});
440552
});

‎packages/eui/src/components/flyout/flyout.tsx‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,15 @@ export const EuiFlyout = forwardRef<
5656
/*
5757
* Flyout routing logic
5858
* 1. Main Flyout: When session={true} OR when there's an active session and this flyout
59-
* is rendered outside of a managed flyout context.
60-
* 2. Child Flyout: When there's an active session AND this flyout IS rendered within a
61-
* managed flyout context.
59+
* is rendered outside of a managed flyout context AND explicitly creates a new session.
60+
* 2. Child Flyout: When there's an active session AND (this flyout IS rendered within a
61+
* managed flyout context OR it's not explicitly creating a new session).
6262
* 3. Standard Flyout: Default fallback when neither condition is met.
6363
*/
64-
if (session === true || (hasActiveSession && !isInManagedFlyout)) {
64+
if (
65+
session === true ||
66+
(hasActiveSession && !isInManagedFlyout && session !== undefined)
67+
) {
6568
if (isUnmanagedFlyout.current) {
6669
// TODO: @tkajtoch - We need to find a better way to handle the missing event.
6770
onClose?.({} as any);
@@ -72,8 +75,8 @@ export const EuiFlyout = forwardRef<
7275
);
7376
}
7477

75-
// Else if this flyout is a child of a session AND within a managed flyout context, render EuiChildFlyout.
76-
if (hasActiveSession && isInManagedFlyout) {
78+
// 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.
79+
if (hasActiveSession && (isInManagedFlyout || session === undefined)) {
7780
return (
7881
<EuiFlyoutChild
7982
{...rest}

‎packages/eui/src/components/flyout/manager/flyout_sessions.stories.tsx‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import {
3535
} from '../..';
3636
import { EuiFlyout } from '../flyout';
3737
import { useCurrentSession, useFlyoutManager } from './hooks';
38-
import { EuiFlyoutIsManagedProvider } from './context';
3938

4039
const meta: Meta<typeof EuiFlyout> = {
4140
title: 'Layout/EuiFlyout/Flyout Manager',
@@ -178,7 +177,6 @@ const FlyoutSession: React.FC<FlyoutSessionProps> = React.memo((props) => {
178177
isOpen={isChildFlyoutVisible}
179178
id={`childFlyout-${title}`}
180179
flyoutMenuProps={{ title: `${title} - Child` }}
181-
aria-labelledby="childFlyoutTitle"
182180
size={childSize}
183181
maxWidth={childMaxWidth}
184182
onActive={childFlyoutOnActive}
@@ -387,15 +385,7 @@ const ExternalRootFlyout: React.FC<{ id: string }> = ({ id }) => {
387385
const newRoot = createRoot(buttonContainerRef.current);
388386
newRoot.render(
389387
<EuiProvider>
390-
{/*
391-
EuiFlyoutIsManagedProvider is required here because the child flyout
392-
needs to be detected as being within a managed flyout context for
393-
proper routing. Without this, the child flyout would route to
394-
EuiFlyoutMain instead of EuiFlyoutChild.
395-
*/}
396-
<EuiFlyoutIsManagedProvider isManaged={true}>
397-
<ExternalRootChildFlyout parentId={id} />
398-
</EuiFlyoutIsManagedProvider>
388+
<ExternalRootChildFlyout parentId={id} />
399389
</EuiProvider>
400390
);
401391
setButtonRoot(newRoot);

0 commit comments

Comments
 (0)