Skip to content

Commit 3cc1e53

Browse files
tsullivantsullivan
authored
[Flyout System] improve size validation flexibility and bidirectionality (#9094)
1 parent ff3c788 commit 3cc1e53

7 files changed

Lines changed: 86 additions & 36 deletions

File tree

‎packages/eui/src/components/flyout/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ The managed flyout wrapper that integrates with the flyout manager system, handl
5454

5555
### `src/components/flyout/manager/flyout_validation.ts`
5656
Validation utilities for managed flyout props:
57-
- **Named Size Validation**: Managed flyouts must use named sizes (s, m, l). If not provided, defaults to 'm'.
58-
- **Size Combination Rules**: Parent and child can't both be 'm', parent can't be 'l' with child
57+
- **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'.
58+
- **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'
5959
- **Title**: Must be provided either through `flyoutMenuProps` or `aria-label`
6060
- **Error Handling**: Comprehensive error messages for invalid configurations
6161

‎packages/eui/src/components/flyout/manager/flyout_managed.test.tsx‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ describe('EuiManagedFlyout', () => {
282282
});
283283

284284
describe('size handling', () => {
285-
it('defaults size to "m" when no size is provided', () => {
285+
it('defaults main flyout size to "m" when no size is provided', () => {
286286
// Import the real validation function to test the actual behavior
287287
const { validateManagedFlyoutSize } = jest.requireActual('./validation');
288288

@@ -308,6 +308,31 @@ describe('EuiManagedFlyout', () => {
308308
require('./validation').validateManagedFlyoutSize = originalMock;
309309
});
310310

311+
it('defaults child flyout size to "s" when no size is provided', () => {
312+
// Import the real validation function to test the actual behavior
313+
const { validateManagedFlyoutSize } = jest.requireActual('./validation');
314+
315+
// Temporarily restore the real validation function for this test
316+
const originalMock = require('./validation').validateManagedFlyoutSize;
317+
require('./validation').validateManagedFlyoutSize =
318+
validateManagedFlyoutSize;
319+
320+
const { getByTestSubject } = renderInProvider(
321+
<EuiManagedFlyout
322+
id="default-child-size-test"
323+
level={LEVEL_CHILD}
324+
onClose={() => {}}
325+
// Explicitly not providing size prop
326+
/>
327+
);
328+
329+
// The flyout should render successfully, indicating the default size worked
330+
expect(getByTestSubject('managed-flyout')).toBeInTheDocument();
331+
332+
// Restore the mock
333+
require('./validation').validateManagedFlyoutSize = originalMock;
334+
});
335+
311336
it('uses provided size when size is explicitly set', () => {
312337
const { getByTestSubject } = renderInProvider(
313338
<EuiManagedFlyout

‎packages/eui/src/components/flyout/manager/flyout_managed.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export const EuiManagedFlyout = ({
7474
onClose: onCloseProp,
7575
onActive: onActiveProp,
7676
level,
77-
size = 'm',
77+
size: sizeProp,
7878
css: customCss,
7979
flyoutMenuProps: _flyoutMenuProps,
8080
...props
@@ -89,6 +89,9 @@ export const EuiManagedFlyout = ({
8989
const layoutMode = useFlyoutLayoutMode();
9090
const styles = useEuiMemoizedStyles(euiManagedFlyoutStyles);
9191

92+
// Set default size based on level: main defaults to 'm', child defaults to 's'
93+
const size = sizeProp ?? (level === LEVEL_CHILD ? 's' : 'm');
94+
9295
// Validate size
9396
const sizeTypeError = validateManagedFlyoutSize(size, flyoutId, level);
9497
if (sizeTypeError) {

‎packages/eui/src/components/flyout/manager/flyout_manager.stories.tsx‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ const meta: Meta<FlyoutChildStoryArgs> = {
6868
component: EuiFlyoutChild,
6969
argTypes: {
7070
childSize: {
71-
options: ['s', 'm', 'fill'],
71+
options: ['s', 'm', 'l', 'fill'],
7272
control: { type: 'radio' },
7373
description:
74-
'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`.',
74+
'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".',
7575
},
7676
childBackgroundStyle: {
7777
options: ['default', 'shaded'],
@@ -83,10 +83,10 @@ const meta: Meta<FlyoutChildStoryArgs> = {
8383
description: 'The maximum width of the child flyout.',
8484
},
8585
mainSize: {
86-
options: ['s', 'm', 'fill'],
86+
options: ['s', 'm', 'l', 'fill', '400px'],
8787
control: { type: 'radio' },
8888
description:
89-
'The size of the main (parent) flyout. If `m`, the child must be `s`. If `s`, the child can be `s`, or `m`.',
89+
'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".',
9090
},
9191
mainFlyoutType: {
9292
options: FLYOUT_TYPES,
@@ -269,8 +269,12 @@ const StatefulFlyout: React.FC<FlyoutChildStoryArgs> = ({
269269
<p>This is the child flyout content.</p>
270270
<p>Size restrictions apply:</p>
271271
<ul>
272-
<li>When main panel is 's', child can be 's', or 'm'</li>
273-
<li>When main panel is 'm', child is limited to 's'</li>
272+
<li>Both flyouts cannot be size &quot;m&quot;</li>
273+
<li>Both flyouts cannot be size &quot;fill&quot;</li>
274+
<li>
275+
Size &quot;l&quot; can only be used if the other flyout
276+
is &quot;fill&quot;
277+
</li>
274278
</ul>
275279
<EuiSpacer />
276280
<p>

‎packages/eui/src/components/flyout/manager/layout_mode.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export const useApplyFlyoutLayoutMode = () => {
126126
let newLayoutMode: EuiFlyoutLayoutMode;
127127

128128
// Handle fill size flyouts: keep layout as side-by-side when fill flyout is present
129-
// This allows fill flyouts to dynamically calculate their width based on sibling
129+
// This allows fill flyouts to dynamically calculate their width based on the other in the pair
130130
if (parentFlyout?.size === 'fill' || childFlyout?.size === 'fill') {
131131
// For fill flyouts, we want to maintain side-by-side layout to enable dynamic width calculation
132132
// Only stack if the viewport is too small (below the small breakpoint)

‎packages/eui/src/components/flyout/manager/validation.test.ts‎

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,25 @@ describe('Flyout Size Validation', () => {
3333
});
3434

3535
describe('validateManagedFlyoutSize', () => {
36-
it('should return null for valid named sizes', () => {
37-
expect(validateManagedFlyoutSize('s', 'test-id', 'main')).toBeNull();
36+
it('should return null for valid named sizes in child flyouts', () => {
37+
expect(validateManagedFlyoutSize('s', 'test-id', 'child')).toBeNull();
3838
expect(validateManagedFlyoutSize('m', 'test-id', 'child')).toBeNull();
39-
expect(validateManagedFlyoutSize('l', 'test-id', 'main')).toBeNull();
39+
expect(validateManagedFlyoutSize('l', 'test-id', 'child')).toBeNull();
40+
});
41+
42+
it('should return null for main flyouts regardless of size', () => {
43+
expect(validateManagedFlyoutSize('100px', 'test-id', 'main')).toBeNull();
44+
expect(validateManagedFlyoutSize('s', 'test-id', 'main')).toBeNull();
4045
});
4146

42-
it('should return error for non-named sizes', () => {
43-
const error = validateManagedFlyoutSize('100px', 'test-id', 'main');
47+
it('should return error for non-named sizes in child flyouts', () => {
48+
const error = validateManagedFlyoutSize('100px', 'test-id', 'child');
4449
expect(error).toEqual({
4550
type: 'INVALID_SIZE_TYPE',
4651
message:
47-
'Managed flyouts must use named sizes (s, m, l, fill). Received: 100px',
52+
'Child flyouts must use named sizes (s, m, l, fill). Received: 100px',
4853
flyoutId: 'test-id',
49-
level: 'main',
54+
level: 'child',
5055
size: '100px',
5156
});
5257
});
@@ -75,13 +80,13 @@ describe('Flyout Size Validation', () => {
7580
describe('validateSizeCombination', () => {
7681
it('should return null for valid combinations', () => {
7782
expect(validateSizeCombination('s', 'm')).toBeNull();
78-
expect(validateSizeCombination('s', 'l')).toBeNull();
7983
expect(validateSizeCombination('m', 's')).toBeNull();
80-
expect(validateSizeCombination('m', 'l')).toBeNull();
8184
expect(validateSizeCombination('s', 'fill')).toBeNull();
8285
expect(validateSizeCombination('m', 'fill')).toBeNull();
8386
expect(validateSizeCombination('fill', 's')).toBeNull();
8487
expect(validateSizeCombination('fill', 'm')).toBeNull();
88+
expect(validateSizeCombination('l', 'fill')).toBeNull();
89+
expect(validateSizeCombination('fill', 'l')).toBeNull();
8590
});
8691

8792
it('should return error when parent and child are both m', () => {
@@ -100,12 +105,23 @@ describe('Flyout Size Validation', () => {
100105
});
101106
});
102107

103-
it('should return error when parent is l and there is a child', () => {
104-
const error = validateSizeCombination('l', 's');
105-
expect(error).toEqual({
108+
it('should return error when a flyout is l without the other being fill', () => {
109+
const errorParentL = validateSizeCombination('l', 's');
110+
expect(errorParentL).toEqual({
106111
type: 'INVALID_SIZE_COMBINATION',
107-
message:
108-
'Parent flyouts cannot be size "l" when there is a child flyout',
112+
message: 'Flyouts cannot be size "l" unless the other flyout is "fill"',
113+
});
114+
115+
const errorChildL = validateSizeCombination('s', 'l');
116+
expect(errorChildL).toEqual({
117+
type: 'INVALID_SIZE_COMBINATION',
118+
message: 'Flyouts cannot be size "l" unless the other flyout is "fill"',
119+
});
120+
121+
const errorParentLChildM = validateSizeCombination('l', 'm');
122+
expect(errorParentLChildM).toEqual({
123+
type: 'INVALID_SIZE_COMBINATION',
124+
message: 'Flyouts cannot be size "l" unless the other flyout is "fill"',
109125
});
110126
});
111127
});
@@ -125,15 +141,15 @@ describe('Flyout Size Validation', () => {
125141
const error: FlyoutValidationError = {
126142
type: 'INVALID_SIZE_TYPE',
127143
message:
128-
'Managed flyouts must use named sizes (s, m, l, fill). Received: 100px',
144+
'Child flyouts must use named sizes (s, m, l, fill). Received: 100px',
129145
flyoutId: 'test-id',
130-
level: 'main',
146+
level: 'child',
131147
size: '100px',
132148
};
133149

134150
const message = createValidationErrorMessage(error);
135151
expect(message).toBe(
136-
'EuiFlyout validation error: Managed flyouts must use named sizes (s, m, l, fill). Received: 100px'
152+
'EuiFlyout validation error: Child flyouts must use named sizes (s, m, l, fill). Received: 100px'
137153
);
138154
expect(consoleSpy).toHaveBeenCalledWith(error);
139155
});

‎packages/eui/src/components/flyout/manager/validation.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import { EuiFlyoutSize, FLYOUT_SIZES } from '../const';
1010
import { EuiFlyoutComponentProps } from '../flyout.component';
1111
import { EuiFlyoutMenuProps } from '../flyout_menu';
12-
import { LEVEL_MAIN } from './const';
12+
import { LEVEL_CHILD, LEVEL_MAIN } from './const';
1313
import { EuiFlyoutLevel } from './types';
1414

1515
type FlyoutValidationErrorType =
@@ -42,11 +42,11 @@ export function validateManagedFlyoutSize(
4242
flyoutId: string,
4343
level: EuiFlyoutLevel
4444
): FlyoutValidationError | null {
45-
if (!isNamedSize(size)) {
45+
if (level === LEVEL_CHILD && !isNamedSize(size)) {
4646
const namedSizes = FLYOUT_SIZES.join(', ');
4747
return {
4848
type: 'INVALID_SIZE_TYPE',
49-
message: `Managed flyouts must use named sizes (${namedSizes}). Received: ${size}`,
49+
message: `Child flyouts must use named sizes (${namedSizes}). Received: ${size}`,
5050
flyoutId,
5151
level,
5252
size,
@@ -81,27 +81,29 @@ export function validateSizeCombination(
8181
parentSize: EuiFlyoutSize,
8282
childSize: EuiFlyoutSize
8383
): FlyoutValidationError | null {
84+
const sizes = [parentSize, childSize];
85+
8486
// Parent and child can't both be 'm'
85-
if (parentSize === 'm' && childSize === 'm') {
87+
if (sizes.every((s) => s === 'm')) {
8688
return {
8789
type: 'INVALID_SIZE_COMBINATION',
8890
message: 'Parent and child flyouts cannot both be size "m"',
8991
};
9092
}
9193

9294
// Parent and child can't both be 'fill'
93-
if (parentSize === 'fill' && childSize === 'fill') {
95+
if (sizes.every((s) => s === 'fill')) {
9496
return {
9597
type: 'INVALID_SIZE_COMBINATION',
9698
message: 'Parent and child flyouts cannot both be size "fill"',
9799
};
98100
}
99101

100-
// Parent can't be 'l' if there is a child
101-
if (parentSize === 'l') {
102+
// Flyout can't be 'l' if the other in the pair is not "fill"
103+
if (sizes.includes('l') && !sizes.includes('fill')) {
102104
return {
103105
type: 'INVALID_SIZE_COMBINATION',
104-
message: 'Parent flyouts cannot be size "l" when there is a child flyout',
106+
message: 'Flyouts cannot be size "l" unless the other flyout is "fill"',
105107
};
106108
}
107109

0 commit comments

Comments
 (0)