[wip] flyout manager #8876

clintandrewhall · 2025-07-17T06:42:18Z

Summary

WIP

elasticmachine · 2025-07-17T20:27:47Z

💚 Build Succeeded

Buildkite Build
Commit: aa98d9b

History

💚 Build #4387 succeeded ca1f838

tsullivan · 2025-07-17T21:25:00Z

packages/eui/src/components/flyout/managed/eui_flyout.tsx

+  // Generate or reuse a flyout id for this managed instance
+  const flyoutId = useRef(props.flyoutId || generateManagedFlyoutId());
+  // Check if this flyout is already rendered in the tree
+  const isRendered = useIsManagedFlyoutRendered();


How does useIsManagedFlyoutRendered know if the flyout is already rendered?

Check out the simple provider that wraps each rendered flyout, marked my comment with ⬆️

tsullivan · 2025-07-17T21:38:20Z

packages/eui/src/components/flyout/managed/reducer.ts

+      // Avoid adding duplicate flyouts
+      if (state.flyouts.find((f) => f.id === action.id)) return state;
+
+      // Add new flyout as active, set all others inactive, preserve order


We will have to make sure that opening and closing flyouts have their own actions, as opening and closing child flyouts should have no effect on the history.

Open a main flyout: adds an entry to the managed history

Close a main flyout: pops an entry off the managed history

Open or close a child flyout: doesn't change the number of items in history

Open a main flyout, then open a child flyout, then navigate to a subsequent flyout: the subsequent flyout should show without the child flyout from the previous flyout. If you go back to the previous flyout, the child flyout should re-open.

This is definitely something to cover with the concept of a "session". I'm working on that as we speak.

tsullivan · 2025-07-17T21:41:11Z

packages/eui/src/components/flyout/managed/types.ts

+import type { EuiManagedFlyoutBaseProps } from './eui_flyout_managed';
+
+export interface ManagedFlyoutState {
+  id: string;


Maybe this should have mainId and childId instead of a single id, for the reasons stated here: https://github.com/elastic/eui/pull/8876/files#r2214360182

tsullivan · 2025-07-17T21:53:43Z

packages/eui/src/components/flyout/managed/managed_flyouts.stories.tsx

+/**
+ *
+ * The flyout system allows custom meta data to be provided by the consumer, in the "EuiManagedFlyoutSessionOpen*Options"
+ * interfaces. In the advanced use case, (WithHistoryApp), we're using metadata within the renderMainManagedFlyoutContent
+ * function as a conditional to determine which component to render in the main flyout.
+ */
+// interface WithHistoryAppMeta {
+//   ecommerceMainManagedFlyoutKey?: 'shoppingCart' | 'reviewOrder';
+// }


This comment looks like a carryover from a different architecture. I see that the open flyout action can be dispatched with metadata, but I don't see why we would need it in this system.

tsullivan · 2025-07-17T22:10:20Z

packages/eui/src/components/flyout/managed/managed_flyouts.stories.tsx

+    onCloseProp(event);
+  };
+
+  const renderer = useCreateManagedFlyoutRenderer();


It seems like we would want a separate childFlyoutRenderer, because the props for main flyouts and child flyouts are sufficiently different. For example:

main flyout:

type can be overlay or push

If there is a child flyout, the main flyout size can't be l

child flyout:

type is hardcoded to overlay

backgroundStyle prop exists for "shaded" and "default" style variants

If the main flyout size is m, the child flyout can't also be m

The sizing will definitely become more flexible as we allow resizable and full-screen child flyouts, but I think the above restrictions are pretty basic and will still need to be in effect.

So I would think we would want something like

const { renderer, childRenderer } = useCreateManagedFlyoutRenderer();

clintandrewhall · 2025-07-18T02:53:21Z

packages/eui/src/components/flyout/managed/context.tsx

+    >
+      {children}
+      {renderedManagedFlyouts.map(({ id, element }) => (
+        <ManagedFlyoutIsRenderedProvider key={id}>


@tsullivan ⬆️

tkajtoch · 2025-07-18T15:13:45Z

packages/eui/src/components/flyout/managed/context.tsx

+
+      // After closing, activate the last flyout if any remain
+      setTimeout(() => {
+        if (state.flyouts.length > 1) {


I think there's a slight possibility of a race condition here if we're unlucky. state.flyouts could technically change between event cycles

Yeah, this was Windsurf's idea. I've removed it since.

Copilot

Pull Request Overview

This is a work-in-progress pull request that introduces a flyout manager system to the EUI component library. The changes implement a managed flyout system that allows multiple flyouts to be stacked and coordinated through a centralized context provider.

Adds a new ManagedFlyoutProvider to the main EuiProvider for global flyout management
Implements a complete flyout management system with context, reducer, and hooks
Creates a new managed flyout API that can be used alongside the existing unmanaged flyouts

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
packages/eui/src/components/provider/provider.tsx	Integrates ManagedFlyoutProvider into the main EuiProvider component tree
packages/eui/src/components/provider/component_defaults/component_defaults.tsx	Adds commented-out type definitions for future managed flyout component defaults
packages/eui/src/components/flyout/managed/types.ts	Defines TypeScript interfaces and types for the managed flyout system
packages/eui/src/components/flyout/managed/reducer.ts	Implements the state management reducer for flyout operations
packages/eui/src/components/flyout/managed/managed_flyouts.stories.tsx	Provides Storybook examples demonstrating managed flyout usage
packages/eui/src/components/flyout/managed/index.ts	Exports the public API for the managed flyout system
packages/eui/src/components/flyout/managed/hooks.ts	Implements React hooks for interacting with the flyout manager
packages/eui/src/components/flyout/managed/eui_flyout_managed.tsx	Core managed flyout component with registration and lifecycle management
packages/eui/src/components/flyout/managed/eui_flyout.tsx	Unified flyout component that supports both managed and unmanaged modes
packages/eui/src/components/flyout/managed/context.tsx	React context provider and related components for flyout state management
packages/eui/src/components/flyout/index.ts	Updates flyout exports with commented references to managed flyouts
packages/eui/src/components/flyout/flyout.tsx	Exports internal props interface for use in managed flyouts
packages/eui/src/components/flyout/flyout.stories.tsx	Adds commented-out examples for component defaults integration

Copilot · 2025-07-19T12:49:58Z

packages/eui/src/components/flyout/managed/context.tsx

+      setTimeout(() => {
+        if (state.flyouts.length > 1) {
+          const last = state.flyouts.filter((f) => f.id !== id).at(-1);
+          if (last) {
+            dispatch({ type: 'SET_ACTIVE', id: last.id });
+          }
+        }
+      });


Using setTimeout without a delay can cause race conditions and unpredictable behavior. The flyout state update should be handled synchronously or with proper async handling.

Suggested change

setTimeout(() => {

if (state.flyouts.length > 1) {

const last = state.flyouts.filter((f) => f.id !== id).at(-1);

if (last) {

dispatch({ type: 'SET_ACTIVE', id: last.id });

}

}

});

const remainingFlyouts = state.flyouts.filter((f) => f.id !== id);

if (remainingFlyouts.length > 0) {

const last = remainingFlyouts.at(-1);

if (last) {

dispatch({ type: 'SET_ACTIVE', id: last.id });

}

}

Copilot · 2025-07-19T12:49:58Z

packages/eui/src/components/flyout/managed/eui_flyout_managed.tsx

+      isOpen.current = true;
+    }
+    if (isOpen.current && !isRegistered) {
+      onClose?.({ type: 'external', flyoutId } as any);


Using 'as any' bypasses TypeScript's type checking. Define a proper type for the onClose event parameter instead of using type assertion.

Suggested change

onClose?.({ type: 'external', flyoutId } as any);

onClose?.({ type: 'external', flyoutId });

Copilot · 2025-07-19T12:49:59Z

packages/eui/src/components/flyout/managed/context.tsx

+        setRenderedManagedFlyouts((prev) => prev.filter((f) => f.id !== id));
+        dispatch({ type: 'CLOSE_FLYOUT', id });
+        if (userOnClose) {
+          userOnClose({ type: 'internal', flyoutId: id } as any);


Using 'as any' bypasses TypeScript's type checking. Define a proper type for the onClose event parameter instead of using type assertion.

Copilot · 2025-07-19T12:49:59Z

packages/eui/src/components/flyout/managed/managed_flyouts.stories.tsx

+        </EuiButton>
+      </EuiFlyoutBody>
+      <EuiFlyoutFooter>
+        <EuiButton onClick={(e: any) => onClose(e)} color="danger">


Using 'any' type for event parameter reduces type safety. Use the proper event type from React instead.

Suggested change

<EuiButton onClick={(e: any) => onClose(e)} color="danger">

<EuiButton onClick={(e: React.MouseEvent<HTMLButtonElement>) => onClose(e)} color="danger">

Copilot · 2025-07-19T12:49:59Z

packages/eui/src/components/flyout/managed/managed_flyouts.stories.tsx

+            Go back
+          </EuiButton>
+        )}{' '}
+        <EuiButton onClick={(e: any) => props.onClose(e)} color="danger">


Using 'any' type for event parameter reduces type safety. Use the proper event type from React instead.

Suggested change

<EuiButton onClick={(e: any) => props.onClose(e)} color="danger">

<EuiButton onClick={(e: React.MouseEvent<HTMLButtonElement>) => props.onClose(e)} color="danger">

Copilot · 2025-07-19T12:50:00Z

packages/eui/src/components/flyout/managed/managed_flyouts.stories.tsx

+        </EuiText>
+      </EuiFlyoutBody>
+      <EuiFlyoutFooter>
+        <EuiButton onClick={(e: any) => onClose(e)} color="danger">


Using 'any' type for event parameter reduces type safety. Use the proper event type from React instead.

Suggested change

<EuiButton onClick={(e: any) => onClose(e)} color="danger">

<EuiButton onClick={(e: React.MouseEvent<HTMLButtonElement>) => onClose(e)} color="danger">

Copilot · 2025-07-19T12:50:00Z

packages/eui/src/components/flyout/managed/eui_flyout_managed.tsx

+function isChildManagedFlyout(
+  props: EuiManagedFlyoutBaseProps
+): props is ChildFlyoutProps {
+  return (props as any).level === 'child';


Using 'as any' bypasses TypeScript's type checking. Use proper type narrowing or define a discriminated union for better type safety.

Suggested change

return (props as any).level === 'child';

return props.level === 'child';

elasticmachine · 2025-07-23T01:10:52Z

💚 Build Succeeded

History

💚 Build #805 succeeded aa98d9b
💚 Build #786 succeeded ca1f838

tsullivan reviewed Jul 17, 2025

View reviewed changes

clintandrewhall commented Jul 18, 2025

View reviewed changes

tkajtoch reviewed Jul 18, 2025

View reviewed changes

clintandrewhall requested a review from Copilot July 19, 2025 12:48

Copilot AI reviewed Jul 19, 2025

View reviewed changes

clintandrewhall added 3 commits July 22, 2025 20:49

[wip] flyout manager

8bedc7f

Separate and clarify managed flyouts.

be40dbf

Relocate concept of active flyout to the top-level state

3a467a8

clintandrewhall force-pushed the exploration/flyout_manager branch from aa98d9b to 3a467a8 Compare July 23, 2025 00:52

	onClose?.({ type: 'external', flyoutId } as any);
	onClose?.({ type: 'external', flyoutId });

	<EuiButton onClick={(e: any) => onClose(e)} color="danger">
	<EuiButton onClick={(e: React.MouseEvent<HTMLButtonElement>) => onClose(e)} color="danger">

	<EuiButton onClick={(e: any) => props.onClose(e)} color="danger">
	<EuiButton onClick={(e: React.MouseEvent<HTMLButtonElement>) => props.onClose(e)} color="danger">

	return (props as any).level === 'child';
	return props.level === 'child';

[wip] flyout manager #8876

Are you sure you want to change the base?

[wip] flyout manager #8876

Conversation

clintandrewhall commented Jul 17, 2025

Summary

Uh oh!

elasticmachine commented Jul 17, 2025

💚 Build Succeeded

History

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tsullivan Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clintandrewhall Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jul 23, 2025

💚 Build Succeeded

History

Uh oh!

Uh oh!

tsullivan Jul 17, 2025 •

edited

Loading

clintandrewhall Jul 18, 2025 •

edited

Loading