-
Notifications
You must be signed in to change notification settings - Fork 861
[flyouts] Developer API + sessions #8939
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
55a9a2f
Add FlyoutSystemMenu component (#8851)
tsullivan c321e49
[wip] New Developer API + sessions
clintandrewhall b4a34d1
Fix imports
tsullivan d9edc91
Iffy TS fix
tsullivan 84d0291
fix iffiness of previous typecheck fixes
tsullivan e53b984
Fix TS and tests
clintandrewhall 3c841bf
Add FIXME/TODO for parent+child context
tsullivan 5a0058b
Refactor managed flyout styles
tsullivan 14689b0
Refactor child flyout storybook
tsullivan 3170aba
Remove previous history management API
tsullivan 304454c
Remove previous EuiFlyoutChild implementation
tsullivan 0227690
Move TODOs to more appropriate context
tsullivan f300dd5
Don’t allow `hideCloseButton` since its not supported in the EuiFlyou…
tsullivan 31c3828
Refine flyout_child.stories.tsx
tsullivan 316c537
Allow close from EuiFlyoutMenu in EuiManagedFlyout
tsullivan 749ae78
Fix Escape key rules
tsullivan a5cba85
Add flyout/README.md
tsullivan af1cd88
Improve storybook
tsullivan 66906db
Validate combination of parent and child sizes
tsullivan 231b1bc
Add note about left-hand shadow
tsullivan 96ec2ea
Fix animations, flickers, overlay styles, ownFocus changes.
clintandrewhall 635b53f
size string validation cleanup
tsullivan 726e778
Flyout layout mode and add `useCurrentChildFlyout` helper
tsullivan cf521d5
update flyout/README
tsullivan 410cea0
Fix animation problems; resolve PR feedback
clintandrewhall 0c17fbc
Adjust stacked detection and behavior.
clintandrewhall e3b6388
Debug
tsullivan 03c03db
remove feedback loop, layout fix to follow
tsullivan 32789c8
Set main to inactive in stacked mode - WIP
tsullivan 15df295
Remove debugging styles
tsullivan edc9f83
Fix the race condition issue
tsullivan 56fd536
Allow main flyout to have ownFocus even if there is a child
tsullivan 462ae12
Cleanup and clarification
clintandrewhall 03b50e7
Docs shouldn't include child.
clintandrewhall 6fbf76e
Fix animation bugs; extract state hook
clintandrewhall e228f73
Fix left/right incompatibility, timing issues with transitions
clintandrewhall e1dd28a
Merge feat/flyout-system into exploration/flyout/manager
clintandrewhall f58837b
Update packages/eui/src/components/flyout/manager/activity_stage.ts
clintandrewhall ca1bafd
Address feedback.
clintandrewhall 330e186
Address further feedback
clintandrewhall 34670d5
Adding tests for hooks, state, etc
clintandrewhall f5feb52
Fix React 17 test failures.
clintandrewhall 905437a
Update Flyout Menu story
tsullivan a8295e9
Update Flyout Child stories to use session
tsullivan b02c50f
Handle attempt to render child out of context
tsullivan e31a1e8
Do not export EuiFlyoutMain, EuiFlyoutChild for direct consumption
tsullivan a530e45
fix test using spyOn
tsullivan 3071ce5
Add React component tests.
clintandrewhall File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,3 +32,5 @@ yarn-error.log* | |
| !.yarn/sdks | ||
| !.yarn/versions | ||
| yarn-error.log | ||
| .cursorrules | ||
| WARP.md | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| # EUI Flyout System | ||
|
|
||
| ## Core Flyout Components | ||
|
|
||
| ### `src/components/flyout/flyout.tsx` | ||
| The main flyout component that serves as the entry point for all flyout functionality. It intelligently renders different flyout types based on context: | ||
| - **Session flyouts**: When `session={true}` or within an active session, renders `EuiFlyoutMain` | ||
| - **Child flyouts**: When within a managed flyout context, renders `EuiFlyoutChild` | ||
| - **Standard flyouts**: Default behavior renders `EuiFlyoutComponent` | ||
| - **Resizable flyouts**: `EuiFlyoutResizable` component exists but is not integrated into main routing logic | ||
|
|
||
| ### `src/components/flyout/flyout.component.tsx` | ||
| The core flyout implementation with comprehensive functionality: | ||
| - **Props**: Extensive configuration options including size, padding, positioning, focus management | ||
| - **Types**: Support for `push` and `overlay` types, left/right sides, various sizes (s/m/l) | ||
| - **Accessibility**: Built-in screen reader support, focus trapping, keyboard navigation with sophisticated ESC key handling | ||
| - **Styling**: Dynamic width handling, responsive behavior, theme integration | ||
| - **Portal/Overlay**: Conditional portal rendering and overlay mask management | ||
| - **Session Logic**: Complex routing logic that determines flyout type based on session state and managed context | ||
| - **Responsive Behavior**: Adaptive layout switching for managed flyouts based on viewport width and flyout size combinations | ||
|
|
||
| ### `src/components/flyout/flyout.styles.ts` | ||
| Contains the emotion-based styling for the flyout component, including: | ||
| - Base flyout styles | ||
| - Size-specific styles (s/m/l) | ||
| - Padding size variations | ||
| - Push vs overlay type styles | ||
| - Side-specific positioning (left/right) | ||
| - Animation and transition styles | ||
|
|
||
| ## Flyout Management System | ||
|
|
||
| ### `src/components/flyout/manager/flyout_manager.tsx` | ||
| The central state management system for flyout sessions: | ||
| - **Context Provider**: `EuiFlyoutManager` provides global flyout state | ||
| - **Session Management**: Tracks main and child flyout relationships with complex state transitions | ||
| - **State Reducer**: Handles flyout lifecycle (add, close, set active, set width) | ||
| - **Hooks**: Provides utilities like `useHasActiveSession`, `useCurrentSession`, `useFlyoutWidth` | ||
| - **Actions**: `addFlyout`, `closeFlyout`, `setActiveFlyout`, `setFlyoutWidth` | ||
| - **Responsive Layout**: `useFlyoutLayoutMode` hook manages responsive behavior for managed flyouts with 90% viewport width rule for switching between `side-by-side` and `stacked` layouts | ||
|
|
||
| ### `src/components/flyout/manager/flyout_main.tsx` | ||
| Renders the primary flyout in a session. Currently a simple wrapper around `EuiManagedFlyout` with `session={true}`. TODO items include handling child flyout presence and adjusting focus/shadow behavior. | ||
|
|
||
| ### `src/components/flyout/manager/flyout_child.tsx` | ||
| Renders child flyouts within a session: | ||
| - **Positioning**: Automatically positions relative to main flyout width | ||
| - **Styling**: Supports `backgroundStyle` prop for default/shaded backgrounds | ||
| - **Constraints**: Forces `type="overlay"` and `ownFocus={false}` | ||
| - **Width Integration**: Uses main flyout width for positioning | ||
|
|
||
| ### `src/components/flyout/manager/flyout_managed.tsx` | ||
| The managed flyout wrapper that integrates with the flyout manager system, handling registration and lifecycle management. Includes size validation for managed flyouts according to business rules. | ||
|
|
||
| ### `src/components/flyout/manager/flyout_validation.ts` | ||
| Validation utilities for flyout size business rules: | ||
| - **Named Size Validation**: Managed flyouts must use named sizes (s, m, l) | ||
| - **Size Combination Rules**: Parent and child can't both be 'm', parent can't be 'l' with child | ||
| - **Error Handling**: Comprehensive error messages for invalid configurations | ||
|
|
||
| ### `src/components/flyout/manager/index.ts` | ||
| Exports all manager-related components and utilities for easy importing. | ||
|
|
||
| ## Specialized Flyout Components | ||
|
|
||
| ### `src/components/flyout/flyout_resizable.tsx` | ||
| A resizable flyout variant that adds drag-to-resize functionality: | ||
| - **Drag Resize**: Mouse/touch drag to resize flyout width | ||
| - **Keyboard Resize**: Arrow key navigation for accessibility | ||
| - **Constraints**: Configurable min/max width with window bounds checking | ||
| - **Callbacks**: `onResize` callback for width change notifications | ||
| - **Visual Indicator**: Resize handle with border indicator | ||
| - **Note**: Not yet integrated into main flyout routing logic | ||
|
|
||
| ### `src/components/flyout/flyout_menu.tsx` | ||
| A specialized flyout component for menu-style content: | ||
| - **Layout**: Flex-based header with back button, popover, title, and close button | ||
| - **Context Integration**: Uses `EuiFlyoutMenuContext` for close handling | ||
| - **Accessibility**: Proper ARIA labels and screen reader support | ||
| - **Styling**: Custom menu-specific styling via `flyout_menu.styles.ts` | ||
|
|
||
| ### `src/components/flyout/flyout_menu_context.ts` | ||
| React context for flyout menu components, providing `onClose` callback to child components. | ||
|
|
||
| ## Styling and Theming | ||
|
|
||
| ### `src/components/flyout/flyout.styles.ts` | ||
| Core flyout styling with emotion CSS-in-JS: | ||
| - Responsive design patterns | ||
| - Theme variable integration | ||
| - Animation and transition styles | ||
| - Size and positioning utilities | ||
|
|
||
| ### `src/components/flyout/flyout_menu.styles.ts` | ||
| Menu-specific styling for the flyout menu component. | ||
|
|
||
| ### `src/components/flyout/manager/flyout.styles.ts` | ||
| Managed flyout styling, including background styles for child flyouts. | ||
|
|
||
| ## Testing and Documentation | ||
|
|
||
| ### `src/components/flyout/flyout.spec.tsx` | ||
| Unit tests for the main flyout component functionality. | ||
|
|
||
| ### `src/components/flyout/flyout.test.tsx` | ||
| Additional test coverage for flyout behavior and edge cases. | ||
|
|
||
| ### `src/components/flyout/flyout_menu.stories.tsx` | ||
| Storybook stories demonstrating flyout menu usage and variations. | ||
|
|
||
| ### `src/components/flyout/manager/flyout_manager.stories.tsx` | ||
| Storybook stories for the flyout manager system and session management. | ||
|
|
||
| ### `src/components/flyout/manager/flyout_child.stories.tsx` | ||
| Storybook stories showcasing child flyout behavior and positioning. | ||
|
|
||
| ## Integration | ||
|
|
||
| ### `src/components/flyout/index.ts` | ||
| Main export file that exposes all public flyout APIs: | ||
| - Core components: `EuiFlyout`, `EuiFlyoutComponent` | ||
| - Body/Header/Footer components | ||
| - Resizable and menu variants | ||
| - Animation utilities | ||
|
|
||
| ### `src/components/provider/provider.tsx` | ||
| The EUI provider that includes `EuiFlyoutManager` in its component tree, ensuring flyout management is available throughout the application. | ||
|
|
||
| ## Key Features | ||
|
|
||
| - **Session Management**: Multi-level flyout sessions with main/child relationships | ||
| - **Accessibility**: Full keyboard navigation, screen reader support, focus management with sophisticated ESC key handling | ||
| - **Responsive Design**: Adaptive behavior based on screen size and breakpoints with intelligent layout switching for managed flyouts (side-by-side vs stacked) when combined flyout widths exceed 90% of viewport | ||
| - **Theme Integration**: Seamless integration with EUI's theming system | ||
| - **Type Safety**: Comprehensive TypeScript support with proper prop typing and validation | ||
| - **Performance**: Optimized rendering with proper cleanup and memory management | ||
| - **Size Validation**: Business rule enforcement for flyout size combinations and managed flyout constraints | ||
|
|
||
| ## TODOs | ||
|
|
||
| ### Performance Issues | ||
|
|
||
| - **Excessive Re-renders**: The flyout manager reducer creates new arrays on every action, causing unnecessary re-renders for all flyout components | ||
| - **Unmemoized Style Calculations**: The `cssStyles` array in `flyout.component.tsx` is recalculated on every render without memoization | ||
| - **Memory Leaks**: `document.activeElement` is stored in a ref but never cleaned up, potentially causing memory leaks | ||
| - **Inefficient DOM Queries**: Focus trap selectors query the DOM on every render without caching | ||
|
|
||
| ### Accessibility Issues | ||
|
|
||
| - **Focus Trap Edge Cases**: The focus trap logic with shards could fail if DOM elements are removed or changed during flyout lifecycle | ||
| - **Missing Error Recovery**: No fallback behavior when focus management fails | ||
| - **Inconsistent Keyboard Navigation**: Different flyout types may have different keyboard behavior patterns | ||
|
|
||
| ### Architectural Concerns | ||
|
|
||
| - **Tight Coupling**: The flyout system is tightly coupled to the provider system, making it difficult to use standalone | ||
| - **State Management Complexity**: The session management system has complex state transitions that could lead to inconsistent UI states | ||
| - **Missing Error Boundaries**: No error handling for flyout rendering failures or state corruption | ||
| - **Unclear Session Logic**: The complex session routing logic in `flyout.tsx` (lines 40-50) is difficult to understand and maintain | ||
| - **Incomplete Integration**: Resizable flyout functionality exists but is not integrated into main routing logic | ||
| - **Missing Cleanup**: Focus references and event listeners are not properly cleaned up | ||
|
|
||
| ### Recommended Improvements | ||
|
|
||
| 1. **Memoize Style Calculations**: Use `useMemo` for the `cssStyles` array to prevent unnecessary recalculations | ||
| 2. **Add Error Boundaries**: Wrap flyout components in error boundaries to handle rendering failures gracefully | ||
| 3. **Improve Type Safety**: Replace `any` types with proper type guards and add comprehensive prop validation | ||
| 4. **Optimize State Updates**: Use immutable update patterns that minimize re-renders in the manager | ||
| 5. **Add Cleanup Logic**: Properly clean up focus references and event listeners in useEffect cleanup functions | ||
| 6. **Simplify Session Logic**: Break down the complex session routing logic into smaller, testable functions | ||
| 7. **Integrate Resizable Flyouts**: Complete the integration of resizable flyout functionality into the main routing logic | ||
| 8. **Add Comprehensive Testing**: Add unit tests for complex state transitions and edge cases | ||
| 9. **Improve Documentation**: Add inline documentation for complex logic and state management patterns | ||
| 10. **Performance Monitoring**: Add performance monitoring for flyout rendering and state updates |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| /* | ||
| * 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 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 or the Server | ||
| * Side Public License, v 1. | ||
| */ | ||
|
|
||
| import { EuiBreakpointSize } from '../../services'; | ||
|
|
||
| /** Allowed flyout render types. */ | ||
| export const FLYOUT_TYPES = ['push', 'overlay'] as const; | ||
| /** Type representing a supported flyout render type. */ | ||
| export type _EuiFlyoutType = (typeof FLYOUT_TYPES)[number]; | ||
|
|
||
| /** Allowed flyout attachment sides. */ | ||
| export const FLYOUT_SIDES = ['left', 'right'] as const; | ||
| /** Type representing a supported flyout side. */ | ||
| export type _EuiFlyoutSide = (typeof FLYOUT_SIDES)[number]; | ||
|
|
||
| /** Allowed named flyout sizes used by the manager. */ | ||
| export const FLYOUT_SIZES = ['s', 'm', 'l'] as const; | ||
| /** Type representing a supported named flyout size. */ | ||
| export type EuiFlyoutSize = (typeof FLYOUT_SIZES)[number]; | ||
|
|
||
| /** Allowed padding sizes for flyout content. */ | ||
| export const FLYOUT_PADDING_SIZES = ['none', 's', 'm', 'l'] as const; | ||
| /** Type representing a supported flyout padding size. */ | ||
| export type _EuiFlyoutPaddingSize = (typeof FLYOUT_PADDING_SIZES)[number]; | ||
|
|
||
| /** Default minimum breakpoint at which push-type flyouts begin to push content. */ | ||
| export const DEFAULT_PUSH_MIN_BREAKPOINT: EuiBreakpointSize = 'l'; | ||
| /** Default flyout type when none is provided. */ | ||
| export const DEFAULT_TYPE: _EuiFlyoutType = 'overlay'; | ||
| /** Default side where flyouts anchor when none is provided. */ | ||
| export const DEFAULT_SIDE: _EuiFlyoutSide = 'right'; | ||
| /** Default named flyout size. */ | ||
| export const DEFAULT_SIZE: EuiFlyoutSize = 'm'; | ||
| /** Default padding size inside flyouts. */ | ||
| export const DEFAULT_PADDING_SIZE: _EuiFlyoutPaddingSize = 'l'; | ||
|
|
||
| /** | ||
| * Custom type checker for named flyout sizes since the prop | ||
| * `size` can also be CSSProperties['width'] (string | number) | ||
| */ | ||
| export function isEuiFlyoutSizeNamed(value: unknown): value is EuiFlyoutSize { | ||
| return FLYOUT_SIZES.includes(value as EuiFlyoutSize); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
I don't mind adding these but I'd put them in a separate "section", right now they seem under "Yarn". Maybe even move to "IDE-specific files".