feat(flyout): improve size validation flexibility and bidirectionality

tsullivan · tsullivan · commit 677d2c8d3a42 · 2025-10-13T10:27:57.000-07:00
- Allow main flyouts to use custom size values (e.g., '400px') while child flyouts must use named sizes
- Update 'l' size validation to be bidirectional: either flyout can be 'l' if the other is 'fill'
- Refactor validation logic to use array methods for cleaner, more concise code
- Update all tests to reflect new validation rules
- Update documentation and storybook with correct size combination rules
- Add 'l' and '400px' options to flyout manager storybook controls
diff --git a/packages/eui/src/components/flyout/README.md b/packages/eui/src/components/flyout/README.md
@@ -54,8 +54,8 @@ The managed flyout wrapper that integrates with the flyout manager system, handl
 
 ### `src/components/flyout/manager/flyout_validation.ts`
 Validation utilities for managed flyout props:
-- **Named Size Validation**: Managed flyouts must use named sizes (s, m, l). If not provided, defaults to 'm'.
-- **Size Combination Rules**: Parent and child can't both be 'm', parent can't be 'l' with child
+- **Named Size Validation**: Child flyouts must use named sizes (s, m, l, fill). Main flyouts can use named sizes or custom values (e.g., '400px'). If size is not provided, main flyouts default to 'm' and child flyouts default to 's'.
+- **Size Combination Rules**: Parent and child can't both be 'm', parent and child can't both be 'fill', and 'l' can only be used if the other flyout is 'fill'
 - **Title**: Must be provided either through `flyoutMenuProps` or `aria-label`
 - **Error Handling**: Comprehensive error messages for invalid configurations
 
diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx
@@ -282,7 +282,7 @@ describe('EuiManagedFlyout', () => {
   });
 
   describe('size handling', () => {
-    it('defaults size to "m" when no size is provided', () => {
+    it('defaults main flyout size to "m" when no size is provided', () => {
       // Import the real validation function to test the actual behavior
       const { validateManagedFlyoutSize } = jest.requireActual('./validation');
 
@@ -308,6 +308,31 @@ describe('EuiManagedFlyout', () => {
       require('./validation').validateManagedFlyoutSize = originalMock;
     });
 
+    it('defaults child flyout size to "s" when no size is provided', () => {
+      // Import the real validation function to test the actual behavior
+      const { validateManagedFlyoutSize } = jest.requireActual('./validation');
+
+      // Temporarily restore the real validation function for this test
+      const originalMock = require('./validation').validateManagedFlyoutSize;
+      require('./validation').validateManagedFlyoutSize =
+        validateManagedFlyoutSize;
+
+      const { getByTestSubject } = renderInProvider(
+        <EuiManagedFlyout
+          id="default-child-size-test"
+          level={LEVEL_CHILD}
+          onClose={() => {}}
+          // Explicitly not providing size prop
+        />
+      );
+
+      // The flyout should render successfully, indicating the default size worked
+      expect(getByTestSubject('managed-flyout')).toBeInTheDocument();
+
+      // Restore the mock
+      require('./validation').validateManagedFlyoutSize = originalMock;
+    });
+
     it('uses provided size when size is explicitly set', () => {
       const { getByTestSubject } = renderInProvider(
         <EuiManagedFlyout
diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.tsx
@@ -74,7 +74,7 @@ export const EuiManagedFlyout = ({
   onClose: onCloseProp,
   onActive: onActiveProp,
   level,
-  size = 'm',
+  size: sizeProp,
   css: customCss,
   flyoutMenuProps: _flyoutMenuProps,
   ...props
@@ -89,6 +89,9 @@ export const EuiManagedFlyout = ({
   const layoutMode = useFlyoutLayoutMode();
   const styles = useEuiMemoizedStyles(euiManagedFlyoutStyles);
 
+  // Set default size based on level: main defaults to 'm', child defaults to 's'
+  const size = sizeProp ?? (level === LEVEL_CHILD ? 's' : 'm');
+
   // Validate size
   const sizeTypeError = validateManagedFlyoutSize(size, flyoutId, level);
   if (sizeTypeError) {
diff --git a/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx b/packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx
@@ -68,10 +68,10 @@ const meta: Meta<FlyoutChildStoryArgs> = {
   component: EuiFlyoutChild,
   argTypes: {
     childSize: {
-      options: ['s', 'm', 'fill'],
+      options: ['s', 'm', 'l', 'fill'],
       control: { type: 'radio' },
       description:
-        'The size of the child flyout. If the main is `s`, the child can be `s`, or `m`. If the main is `m`, the child can only be `s`.',
+        'The size of the child flyout. Valid combinations: both cannot be "m", both cannot be "fill", and "l" can only be used if the other flyout is "fill".',
     },
     childBackgroundStyle: {
       options: ['default', 'shaded'],
@@ -83,10 +83,10 @@ const meta: Meta<FlyoutChildStoryArgs> = {
       description: 'The maximum width of the child flyout.',
     },
     mainSize: {
-      options: ['s', 'm', 'fill'],
+      options: ['s', 'm', 'l', 'fill', '400px'],
       control: { type: 'radio' },
       description:
-        'The size of the main (parent) flyout. If `m`, the child must be `s`. If `s`, the child can be `s`, or `m`.',
+        'The size of the main (parent) flyout. Can use named sizes (s, m, l, fill) or custom values like "400px". Valid combinations: both cannot be "m", both cannot be "fill", and "l" can only be used if the other flyout is "fill".',
     },
     mainFlyoutType: {
       options: FLYOUT_TYPES,
@@ -269,8 +269,12 @@ const StatefulFlyout: React.FC<FlyoutChildStoryArgs> = ({
                     <p>This is the child flyout content.</p>
                     <p>Size restrictions apply:</p>
                     <ul>
-                      <li>When main panel is 's', child can be 's', or 'm'</li>
-                      <li>When main panel is 'm', child is limited to 's'</li>
+                      <li>Both flyouts cannot be size &quot;m&quot;</li>
+                      <li>Both flyouts cannot be size &quot;fill&quot;</li>
+                      <li>
+                        Size &quot;l&quot; can only be used if the other flyout
+                        is &quot;fill&quot;
+                      </li>
                     </ul>
                     <EuiSpacer />
                     <p>
diff --git a/packages/eui/src/components/flyout/manager/layout_mode.ts b/packages/eui/src/components/flyout/manager/layout_mode.ts
@@ -126,7 +126,7 @@ export const useApplyFlyoutLayoutMode = () => {
     let newLayoutMode: EuiFlyoutLayoutMode;
 
     // Handle fill size flyouts: keep layout as side-by-side when fill flyout is present
-    // This allows fill flyouts to dynamically calculate their width based on sibling
+    // This allows fill flyouts to dynamically calculate their width based on the other in the pair
     if (parentFlyout?.size === 'fill' || childFlyout?.size === 'fill') {
       // For fill flyouts, we want to maintain side-by-side layout to enable dynamic width calculation
       // Only stack if the viewport is too small (below the small breakpoint)
diff --git a/packages/eui/src/components/flyout/manager/validation.test.ts b/packages/eui/src/components/flyout/manager/validation.test.ts
@@ -33,20 +33,25 @@ describe('Flyout Size Validation', () => {
   });
 
   describe('validateManagedFlyoutSize', () => {
-    it('should return null for valid named sizes', () => {
-      expect(validateManagedFlyoutSize('s', 'test-id', 'main')).toBeNull();
+    it('should return null for valid named sizes in child flyouts', () => {
+      expect(validateManagedFlyoutSize('s', 'test-id', 'child')).toBeNull();
       expect(validateManagedFlyoutSize('m', 'test-id', 'child')).toBeNull();
-      expect(validateManagedFlyoutSize('l', 'test-id', 'main')).toBeNull();
+      expect(validateManagedFlyoutSize('l', 'test-id', 'child')).toBeNull();
+    });
+
+    it('should return null for main flyouts regardless of size', () => {
+      expect(validateManagedFlyoutSize('100px', 'test-id', 'main')).toBeNull();
+      expect(validateManagedFlyoutSize('s', 'test-id', 'main')).toBeNull();
     });
 
-    it('should return error for non-named sizes', () => {
-      const error = validateManagedFlyoutSize('100px', 'test-id', 'main');
+    it('should return error for non-named sizes in child flyouts', () => {
+      const error = validateManagedFlyoutSize('100px', 'test-id', 'child');
       expect(error).toEqual({
         type: 'INVALID_SIZE_TYPE',
         message:
-          'Managed flyouts must use named sizes (s, m, l, fill). Received: 100px',
+          'Child flyouts must use named sizes (s, m, l, fill). Received: 100px',
         flyoutId: 'test-id',
-        level: 'main',
+        level: 'child',
         size: '100px',
       });
     });
@@ -75,13 +80,13 @@ describe('Flyout Size Validation', () => {
   describe('validateSizeCombination', () => {
     it('should return null for valid combinations', () => {
       expect(validateSizeCombination('s', 'm')).toBeNull();
-      expect(validateSizeCombination('s', 'l')).toBeNull();
       expect(validateSizeCombination('m', 's')).toBeNull();
-      expect(validateSizeCombination('m', 'l')).toBeNull();
       expect(validateSizeCombination('s', 'fill')).toBeNull();
       expect(validateSizeCombination('m', 'fill')).toBeNull();
       expect(validateSizeCombination('fill', 's')).toBeNull();
       expect(validateSizeCombination('fill', 'm')).toBeNull();
+      expect(validateSizeCombination('l', 'fill')).toBeNull();
+      expect(validateSizeCombination('fill', 'l')).toBeNull();
     });
 
     it('should return error when parent and child are both m', () => {
@@ -100,12 +105,23 @@ describe('Flyout Size Validation', () => {
       });
     });
 
-    it('should return error when parent is l and there is a child', () => {
-      const error = validateSizeCombination('l', 's');
-      expect(error).toEqual({
+    it('should return error when a flyout is l without the other being fill', () => {
+      const errorParentL = validateSizeCombination('l', 's');
+      expect(errorParentL).toEqual({
         type: 'INVALID_SIZE_COMBINATION',
-        message:
-          'Parent flyouts cannot be size "l" when there is a child flyout',
+        message: 'Flyouts cannot be size "l" unless the other flyout is "fill"',
+      });
+
+      const errorChildL = validateSizeCombination('s', 'l');
+      expect(errorChildL).toEqual({
+        type: 'INVALID_SIZE_COMBINATION',
+        message: 'Flyouts cannot be size "l" unless the other flyout is "fill"',
+      });
+
+      const errorParentLChildM = validateSizeCombination('l', 'm');
+      expect(errorParentLChildM).toEqual({
+        type: 'INVALID_SIZE_COMBINATION',
+        message: 'Flyouts cannot be size "l" unless the other flyout is "fill"',
       });
     });
   });
@@ -125,15 +141,15 @@ describe('Flyout Size Validation', () => {
       const error: FlyoutValidationError = {
         type: 'INVALID_SIZE_TYPE',
         message:
-          'Managed flyouts must use named sizes (s, m, l, fill). Received: 100px',
+          'Child flyouts must use named sizes (s, m, l, fill). Received: 100px',
         flyoutId: 'test-id',
-        level: 'main',
+        level: 'child',
         size: '100px',
       };
 
       const message = createValidationErrorMessage(error);
       expect(message).toBe(
-        'EuiFlyout validation error: Managed flyouts must use named sizes (s, m, l, fill). Received: 100px'
+        'EuiFlyout validation error: Child flyouts must use named sizes (s, m, l, fill). Received: 100px'
       );
       expect(consoleSpy).toHaveBeenCalledWith(error);
     });
diff --git a/packages/eui/src/components/flyout/manager/validation.ts b/packages/eui/src/components/flyout/manager/validation.ts
@@ -9,7 +9,7 @@
 import { EuiFlyoutSize, FLYOUT_SIZES } from '../const';
 import { EuiFlyoutComponentProps } from '../flyout.component';
 import { EuiFlyoutMenuProps } from '../flyout_menu';
-import { LEVEL_MAIN } from './const';
+import { LEVEL_CHILD, LEVEL_MAIN } from './const';
 import { EuiFlyoutLevel } from './types';
 
 type FlyoutValidationErrorType =
@@ -42,11 +42,11 @@ export function validateManagedFlyoutSize(
   flyoutId: string,
   level: EuiFlyoutLevel
 ): FlyoutValidationError | null {
-  if (!isNamedSize(size)) {
+  if (level === LEVEL_CHILD && !isNamedSize(size)) {
     const namedSizes = FLYOUT_SIZES.join(', ');
     return {
       type: 'INVALID_SIZE_TYPE',
-      message: `Managed flyouts must use named sizes (${namedSizes}). Received: ${size}`,
+      message: `Child flyouts must use named sizes (${namedSizes}). Received: ${size}`,
       flyoutId,
       level,
       size,
@@ -81,27 +81,29 @@ export function validateSizeCombination(
   parentSize: EuiFlyoutSize,
   childSize: EuiFlyoutSize
 ): FlyoutValidationError | null {
+  const sizes = [parentSize, childSize];
+
   // Parent and child can't both be 'm'
-  if (parentSize === 'm' && childSize === 'm') {
+  if (sizes.every((s) => s === 'm')) {
     return {
       type: 'INVALID_SIZE_COMBINATION',
       message: 'Parent and child flyouts cannot both be size "m"',
     };
   }
 
   // Parent and child can't both be 'fill'
-  if (parentSize === 'fill' && childSize === 'fill') {
+  if (sizes.every((s) => s === 'fill')) {
     return {
       type: 'INVALID_SIZE_COMBINATION',
       message: 'Parent and child flyouts cannot both be size "fill"',
     };
   }
 
-  // Parent can't be 'l' if there is a child
-  if (parentSize === 'l') {
+  // Flyout can't be 'l' if the other in the pair is not "fill"
+  if (sizes.includes('l') && !sizes.includes('fill')) {
     return {
       type: 'INVALID_SIZE_COMBINATION',
-      message: 'Parent flyouts cannot be size "l" when there is a child flyout',
+      message: 'Flyouts cannot be size "l" unless the other flyout is "fill"',
     };
   }
 
