diff --git a/packages/eui/src/components/flyout/README.md b/packages/eui/src/components/flyout/README.md index 6dd9b9748bc..f4f54d47271 100644 --- 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 index 17ad3e87e85..bea58e358e7 100644 --- 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( + {}} + // 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( = { 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 = { 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 = ({

This is the child flyout content.

Size restrictions apply:

    -
  • When main panel is 's', child can be 's', or 'm'
  • -
  • When main panel is 'm', child is limited to 's'
  • +
  • Both flyouts cannot be size "m"
  • +
  • Both flyouts cannot be size "fill"
  • +
  • + Size "l" can only be used if the other flyout + is "fill" +

diff --git a/packages/eui/src/components/flyout/manager/layout_mode.ts b/packages/eui/src/components/flyout/manager/layout_mode.ts index 2184223ade3..48e57a85c56 100644 --- 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 index 857d0918172..55cb531c56c 100644 --- 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 index 094395b4208..caf784da2cf 100644 --- 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,8 +81,10 @@ 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"', @@ -90,18 +92,18 @@ export function validateSizeCombination( } // 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"', }; }