[Flyout System] Improve validation and error logging (#9091)

tsullivan · web-flow · commit 1e403fa3449b · 2025-10-10T14:38:02.000-07:00
diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.tsx
@@ -33,7 +33,7 @@ import {
   useIsFlyoutActive,
   useParentFlyoutSize,
 } from './hooks';
-import { useIsFlyoutRegistered } from './selectors';
+import { useCurrentMainFlyout, useIsFlyoutRegistered } from './selectors';
 import type { EuiFlyoutLevel } from './types';
 import {
   createValidationErrorMessage,
@@ -86,6 +86,7 @@ export const EuiManagedFlyout = ({
   const { addFlyout, closeFlyout, setFlyoutWidth, goBack, getHistoryItems } =
     useFlyoutManager();
   const parentSize = useParentFlyoutSize(flyoutId);
+  const parentFlyout = useCurrentMainFlyout();
   const layoutMode = useFlyoutLayoutMode();
   const styles = useEuiMemoizedStyles(euiManagedFlyoutStyles);
 
@@ -105,6 +106,7 @@ export const EuiManagedFlyout = ({
     const combinationError = validateSizeCombination(parentSize, size);
     if (combinationError) {
       combinationError.flyoutId = flyoutId;
+      combinationError.parentFlyoutId = parentFlyout?.flyoutId;
       combinationError.level = level;
       throw new Error(createValidationErrorMessage(combinationError));
     }
diff --git a/packages/eui/src/components/flyout/manager/validation.test.ts b/packages/eui/src/components/flyout/manager/validation.test.ts
@@ -10,7 +10,6 @@ import {
   isNamedSize,
   validateManagedFlyoutSize,
   validateSizeCombination,
-  validateFlyoutSize,
   createValidationErrorMessage,
   FlyoutValidationError,
   validateFlyoutTitle,
@@ -90,7 +89,6 @@ describe('Flyout Size Validation', () => {
       expect(error).toEqual({
         type: 'INVALID_SIZE_COMBINATION',
         message: 'Parent and child flyouts cannot both be size "m"',
-        size: 'm',
       });
     });
 
@@ -99,7 +97,6 @@ describe('Flyout Size Validation', () => {
       expect(error).toEqual({
         type: 'INVALID_SIZE_COMBINATION',
         message: 'Parent and child flyouts cannot both be size "fill"',
-        size: 'fill',
       });
     });
 
@@ -109,105 +106,78 @@ describe('Flyout Size Validation', () => {
         type: 'INVALID_SIZE_COMBINATION',
         message:
           'Parent flyouts cannot be size "l" when there is a child flyout',
-        size: 'l',
       });
     });
   });
 
-  describe('validateFlyoutSize', () => {
-    it('should validate managed flyout size type', () => {
-      const error = validateFlyoutSize('100px', 'test-id', 'main');
-      expect(error?.type).toBe('INVALID_SIZE_TYPE');
-    });
-
-    it('should validate size combinations for child flyouts', () => {
-      // Parent and child both 'm' should fail
-      const error1 = validateFlyoutSize('m', 'child-id', 'child', 'm');
-      expect(error1?.type).toBe('INVALID_SIZE_COMBINATION');
-      expect(error1?.message).toContain(
-        'Parent and child flyouts cannot both be size "m"'
-      );
-
-      // Parent and child both 'fill' should fail
-      const error2 = validateFlyoutSize('fill', 'child-id', 'child', 'fill');
-      expect(error2?.type).toBe('INVALID_SIZE_COMBINATION');
-      expect(error2?.message).toContain(
-        'Parent and child flyouts cannot both be size "fill"'
-      );
+  describe('createValidationErrorMessage', () => {
+    let consoleSpy: jest.SpyInstance;
 
-      // Parent 'l' with child should fail
-      const error3 = validateFlyoutSize('s', 'child-id', 'child', 'l');
-      expect(error3?.type).toBe('INVALID_SIZE_COMBINATION');
-      expect(error3?.message).toContain(
-        'Parent flyouts cannot be size "l" when there is a child flyout'
-      );
+    beforeEach(() => {
+      consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
     });
 
-    it('should return null for valid child flyout combinations', () => {
-      expect(validateFlyoutSize('s', 'child-id', 'child', 'm')).toBeNull();
-      expect(validateFlyoutSize('l', 'child-id', 'child', 'm')).toBeNull();
-      expect(validateFlyoutSize('s', 'child-id', 'child', 's')).toBeNull();
-      expect(validateFlyoutSize('fill', 'child-id', 'child', 'm')).toBeNull();
-      expect(validateFlyoutSize('s', 'child-id', 'child', 'fill')).toBeNull();
+    afterEach(() => {
+      consoleSpy.mockRestore();
     });
-  });
 
-  describe('createValidationErrorMessage', () => {
     it('should create error message for invalid size type', () => {
       const error: FlyoutValidationError = {
         type: 'INVALID_SIZE_TYPE',
         message:
-          'Managed flyouts must use named sizes (s, m, l). Received: 100px',
+          'Managed flyouts must use named sizes (s, m, l, fill). Received: 100px',
         flyoutId: 'test-id',
         level: 'main',
         size: '100px',
       };
 
       const message = createValidationErrorMessage(error);
       expect(message).toBe(
-        'EuiFlyout validation error: Managed flyouts must use named sizes (s, m, l). Received: 100px'
+        'EuiFlyout validation error: Managed flyouts must use named sizes (s, m, l, fill). Received: 100px'
       );
+      expect(consoleSpy).toHaveBeenCalledWith(error);
     });
 
     it('should create error message for invalid size combination', () => {
       const error: FlyoutValidationError = {
         type: 'INVALID_SIZE_COMBINATION',
         message: 'Parent and child flyouts cannot both be size "m"',
-        size: 'm',
       };
 
       const message = createValidationErrorMessage(error);
       expect(message).toBe(
         'EuiFlyout validation error: Parent and child flyouts cannot both be size "m"'
       );
+      expect(consoleSpy).toHaveBeenCalledWith(error);
     });
 
     it('should create error message for invalid fill size combination', () => {
       const error: FlyoutValidationError = {
         type: 'INVALID_SIZE_COMBINATION',
         message: 'Parent and child flyouts cannot both be size "fill"',
-        size: 'fill',
       };
 
       const message = createValidationErrorMessage(error);
       expect(message).toBe(
         'EuiFlyout validation error: Parent and child flyouts cannot both be size "fill"'
       );
+      expect(consoleSpy).toHaveBeenCalledWith(error);
     });
 
     it('should create error message for invalid flyout menu title', () => {
       const error: FlyoutValidationError = {
         type: 'INVALID_FLYOUT_MENU_TITLE',
         message:
-          "Managed flyouts require either a 'flyoutMenuProps' a 'title' property, or an 'aria-label' to provide the title.",
+          "Managed flyouts require either a 'flyoutMenuProps.title' or an 'aria-label' to provide the flyout menu title.",
         flyoutId: 'test-id',
         level: 'main',
       };
 
       const message = createValidationErrorMessage(error);
       expect(message).toBe(
-        "EuiFlyout validation error: Managed flyouts require either a 'flyoutMenuProps' a 'title' property, or an 'aria-label' to provide the title."
+        "EuiFlyout validation error: Managed flyouts require either a 'flyoutMenuProps.title' or an 'aria-label' to provide the flyout menu title."
       );
+      expect(consoleSpy).toHaveBeenCalledWith(error);
     });
 
     it('should handle unknown error types', () => {
@@ -222,6 +192,7 @@ describe('Flyout Size Validation', () => {
       expect(message).toBe(
         'EuiFlyout validation error: Unknown validation error'
       );
+      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_CHILD, LEVEL_MAIN } from './const';
+import { LEVEL_MAIN } from './const';
 import { EuiFlyoutLevel } from './types';
 
 type FlyoutValidationErrorType =
@@ -21,8 +21,10 @@ export interface FlyoutValidationError {
   type: FlyoutValidationErrorType;
   message: string;
   flyoutId?: string;
+  parentFlyoutId?: string;
   level?: EuiFlyoutLevel;
-  size?: string; // Allow any string for error reporting
+  size?: string | number;
+  parentSize?: string | number;
 }
 
 /**
@@ -47,7 +49,7 @@ export function validateManagedFlyoutSize(
       message: `Managed flyouts must use named sizes (${namedSizes}). Received: ${size}`,
       flyoutId,
       level,
-      size: `${size}`,
+      size,
     };
   }
   return null;
@@ -84,7 +86,6 @@ export function validateSizeCombination(
     return {
       type: 'INVALID_SIZE_COMBINATION',
       message: 'Parent and child flyouts cannot both be size "m"',
-      size: childSize,
     };
   }
 
@@ -93,7 +94,6 @@ export function validateSizeCombination(
     return {
       type: 'INVALID_SIZE_COMBINATION',
       message: 'Parent and child flyouts cannot both be size "fill"',
-      size: childSize,
     };
   }
 
@@ -102,57 +102,27 @@ export function validateSizeCombination(
     return {
       type: 'INVALID_SIZE_COMBINATION',
       message: 'Parent flyouts cannot be size "l" when there is a child flyout',
-      size: parentSize,
     };
   }
 
   return null;
 }
 
-/**
- * Comprehensive validation for flyout size rules
- */
-export function validateFlyoutSize(
-  size: EuiFlyoutComponentProps['size'],
-  flyoutId: string,
-  level: EuiFlyoutLevel,
-  parentSize?: EuiFlyoutSize
-): FlyoutValidationError | null {
-  // First validate that managed flyouts use named sizes
-  const sizeTypeError = validateManagedFlyoutSize(size, flyoutId, level);
-  if (sizeTypeError) {
-    return sizeTypeError;
-  }
-
-  // If this is a child flyout and we have parent size, validate combination
-  if (level === LEVEL_CHILD && parentSize && isNamedSize(size)) {
-    const combinationError = validateSizeCombination(parentSize, size);
-    if (combinationError) {
-      combinationError.flyoutId = flyoutId;
-      combinationError.level = level;
-      return combinationError;
-    }
-  }
-
-  return null;
-}
-
 /**
  * Creates a user-friendly error message for validation errors
  */
 export function createValidationErrorMessage(
   error: FlyoutValidationError
 ): string {
-  const prefix = `EuiFlyout validation error: `;
+  console.error(error);
+  const prefix = `EuiFlyout validation error`;
 
   switch (error.type) {
     case 'INVALID_SIZE_TYPE':
-      return `${prefix}${error.message}`;
     case 'INVALID_SIZE_COMBINATION':
-      return `${prefix}${error.message}`;
     case 'INVALID_FLYOUT_MENU_TITLE':
-      return `${prefix}${error.message}`;
+      return `${prefix}: ${error.message}`;
     default:
-      return `${prefix}Unknown validation error`;
+      return `${prefix}: Unknown validation error`;
   }
 }
