Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/eui/src/components/flyout/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const EuiManagedFlyout = ({
onClose: onCloseProp,
onActive: onActiveProp,
level,
size = 'm',
size: sizeProp,
css: customCss,
flyoutMenuProps: _flyoutMenuProps,
...props
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand All @@ -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,
Expand Down Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion packages/eui/src/components/flyout/manager/layout_mode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
50 changes: 33 additions & 17 deletions packages/eui/src/components/flyout/manager/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});
});
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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"',
});
});
});
Expand All @@ -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);
});
Expand Down
18 changes: 10 additions & 8 deletions packages/eui/src/components/flyout/manager/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"',
};
}

Expand Down