Skip to content

Commit d7c4b6e

Browse files
tsullivantsullivan
authored
Merge branch 'feat/flyout-system' into flyout-system/fix-session-logic
2 parents 2b2ea52 + cbb5862 commit d7c4b6e

12 files changed

Lines changed: 425 additions & 196 deletions

File tree

‎packages/eui/src/components/flyout/flyout.component.tsx‎

Lines changed: 80 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import React, {
1212
useEffect,
13+
useLayoutEffect,
1314
useRef,
1415
useMemo,
1516
useCallback,
@@ -40,6 +41,9 @@ import {
4041
useFlyoutLayoutMode,
4142
useFlyoutId,
4243
useFlyoutWidth,
44+
useIsFlyoutActive,
45+
useFlyoutManager,
46+
useHasPushPadding,
4347
} from './manager';
4448

4549
import { CommonProps, PropsOfElement } from '../common';
@@ -268,6 +272,19 @@ export const EuiFlyoutComponent = forwardRef(
268272
const internalParentFlyoutRef = useRef<HTMLDivElement>(null);
269273
const isPushed = useIsPushed({ type, pushMinBreakpoint });
270274

275+
const currentSession = useCurrentSession();
276+
const isInManagedContext = useIsInManagedFlyout();
277+
const flyoutId = useFlyoutId(id);
278+
const layoutMode = useFlyoutLayoutMode();
279+
const isActiveManagedFlyout = useIsFlyoutActive(flyoutId);
280+
const flyoutManager = useFlyoutManager();
281+
282+
// Use a ref to access the latest flyoutManager without triggering effect re-runs
283+
const flyoutManagerRef = useRef(flyoutManager);
284+
useEffect(() => {
285+
flyoutManagerRef.current = flyoutManager;
286+
}, [flyoutManager]);
287+
271288
const {
272289
onMouseDown: onMouseDownResizableButton,
273290
onKeyDown: onKeyDownResizableButton,
@@ -295,31 +312,66 @@ export const EuiFlyoutComponent = forwardRef(
295312
]);
296313
const { width } = useResizeObserver(isPushed ? resizeRef : null, 'width');
297314

298-
useEffect(() => {
299-
/**
300-
* Accomodate for the `isPushed` state by adding padding to the body equal to the width of the element
301-
*/
302-
if (isPushed) {
303-
const paddingSide =
304-
side === 'left' ? 'paddingInlineStart' : 'paddingInlineEnd';
305-
const cssVarName = `--euiPushFlyoutOffset${
306-
side === 'left' ? 'InlineStart' : 'InlineEnd'
307-
}`;
315+
/**
316+
* Effect for adding push padding to body. Using useLayoutEffect to ensure
317+
* padding changes happen synchronously before child components render -
318+
* this is needed to prevent RemoveScrollBar from measuring the body in an
319+
* inconsistent state during flyout transitions.
320+
*/
321+
useLayoutEffect(() => {
322+
if (!isPushed) {
323+
return; // Only push-type flyouts manage body padding
324+
}
308325

309-
document.body.style[paddingSide] = `${width}px`;
326+
const shouldApplyPadding = !isInManagedContext || isActiveManagedFlyout;
327+
328+
const paddingSide =
329+
side === 'left' ? 'paddingInlineStart' : 'paddingInlineEnd';
330+
const cssVarName = `--euiPushFlyoutOffset${
331+
side === 'left' ? 'InlineStart' : 'InlineEnd'
332+
}`;
333+
const managerSide = side === 'left' ? 'left' : 'right';
310334

311-
// EUI doesn't use this css variable, but it is useful for consumers
335+
if (shouldApplyPadding) {
336+
document.body.style[paddingSide] = `${width}px`;
312337
setGlobalCSSVariables({
313338
[cssVarName]: `${width}px`,
314339
});
315-
return () => {
316-
document.body.style[paddingSide] = '';
317-
setGlobalCSSVariables({
318-
[cssVarName]: null,
319-
});
320-
};
340+
// Update manager state if in managed context
341+
if (isInManagedContext && flyoutManagerRef.current) {
342+
flyoutManagerRef.current.setPushPadding(managerSide, width);
343+
}
344+
} else {
345+
// Explicitly remove padding when this push flyout becomes inactive
346+
document.body.style[paddingSide] = '';
347+
setGlobalCSSVariables({
348+
[cssVarName]: null,
349+
});
350+
// Clear manager state if in managed context
351+
if (isInManagedContext && flyoutManagerRef.current) {
352+
flyoutManagerRef.current.setPushPadding(managerSide, 0);
353+
}
321354
}
322-
}, [isPushed, setGlobalCSSVariables, side, width]);
355+
356+
// Cleanup on unmount
357+
return () => {
358+
document.body.style[paddingSide] = '';
359+
setGlobalCSSVariables({
360+
[cssVarName]: null,
361+
});
362+
// Clear manager state on unmount if in managed context
363+
if (isInManagedContext && flyoutManagerRef.current) {
364+
flyoutManagerRef.current.setPushPadding(managerSide, 0);
365+
}
366+
};
367+
}, [
368+
isPushed,
369+
isInManagedContext,
370+
isActiveManagedFlyout,
371+
setGlobalCSSVariables,
372+
side,
373+
width,
374+
]);
323375

324376
/**
325377
* This class doesn't actually do anything by EUI, but is nice to add for consumers (JIC)
@@ -332,13 +384,6 @@ export const EuiFlyoutComponent = forwardRef(
332384
};
333385
}, []);
334386

335-
const currentSession = useCurrentSession();
336-
const isInManagedContext = useIsInManagedFlyout();
337-
338-
// Get flyout manager context for dynamic width calculation
339-
const flyoutId = useFlyoutId(id);
340-
const layoutMode = useFlyoutLayoutMode();
341-
342387
// Memoize flyout identification and relationships to prevent race conditions
343388
const flyoutIdentity = useMemo(() => {
344389
if (!flyoutId || !currentSession) {
@@ -604,6 +649,14 @@ export const EuiFlyoutComponent = forwardRef(
604649

605650
const maskCombinedRefs = useCombinedRefs([maskProps?.maskRef, maskRef]);
606651

652+
/**
653+
* For overlay flyouts in managed contexts, coordinate scroll locking with push flyout state.
654+
*/
655+
const hasPushPaddingInManager = useHasPushPadding();
656+
const shouldDeferScrollLock =
657+
!isPushed && isInManagedContext && hasPushPaddingInManager;
658+
const shouldUseScrollLock = hasOverlayMask && !shouldDeferScrollLock;
659+
607660
return (
608661
<EuiFlyoutOverlay
609662
hasOverlayMask={hasOverlayMask}
@@ -617,7 +670,7 @@ export const EuiFlyoutComponent = forwardRef(
617670
<EuiWindowEvent event="keydown" handler={onKeyDown} />
618671
<EuiFocusTrap
619672
disabled={isPushed}
620-
scrollLock={hasOverlayMask}
673+
scrollLock={shouldUseScrollLock}
621674
clickOutsideDisables={!ownFocus}
622675
onClickOutside={onClickOutside}
623676
{...focusTrapProps}

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,51 @@ describe('EuiFlyout', () => {
437437
});
438438
});
439439

440+
describe('push padding manager state coordination', () => {
441+
it('applies body padding for push flyouts', () => {
442+
const { container } = render(
443+
<EuiFlyout
444+
onClose={() => {}}
445+
type="push"
446+
pushMinBreakpoint="xs"
447+
data-test-subj="push-flyout"
448+
/>
449+
);
450+
451+
const flyout = container.querySelector('[data-test-subj="push-flyout"]');
452+
expect(flyout).toBeInTheDocument();
453+
454+
// Body should have padding applied
455+
const bodyPaddingEnd = document.body.style.paddingInlineEnd;
456+
expect(bodyPaddingEnd).toBeTruthy();
457+
});
458+
459+
it('removes body padding on unmount', () => {
460+
const { unmount } = render(
461+
<EuiFlyout
462+
onClose={() => {}}
463+
type="push"
464+
pushMinBreakpoint="xs"
465+
data-test-subj="push-flyout"
466+
/>
467+
);
468+
469+
// Verify padding was applied
470+
expect(document.body.style.paddingInlineEnd).toBeTruthy();
471+
472+
unmount();
473+
474+
// Verify padding was cleared
475+
expect(document.body.style.paddingInlineEnd).toBe('');
476+
});
477+
478+
afterEach(() => {
479+
// Clean up body styles after each test
480+
document.body.style.paddingInlineStart = '';
481+
document.body.style.paddingInlineEnd = '';
482+
});
483+
});
484+
440485
describe('flyout routing logic', () => {
441486
it('routes to child flyout automatically when nested inside a parent flyout', () => {
442487
const { getByTestSubject } = render(

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ export const ACTION_SET_ACTIVITY_STAGE = `${PREFIX}/setActivityStage` as const;
3535
export const ACTION_GO_BACK = `${PREFIX}/goBack` as const;
3636
/** Dispatched to navigate to a specific flyout (remove all sessions after it). */
3737
export const ACTION_GO_TO_FLYOUT = `${PREFIX}/goToFlyout` as const;
38+
/** Dispatched to set push padding offset for a side. */
39+
export const ACTION_SET_PUSH_PADDING = `${PREFIX}/setPushPadding` as const;
3840

3941
/**
4042
* Add a flyout to manager state. The manager will create or update
@@ -91,6 +93,13 @@ export interface GoToFlyoutAction extends BaseAction {
9193
flyoutId: string;
9294
}
9395

96+
/** Set push padding offset for a specific side. */
97+
export interface SetPushPaddingAction extends BaseAction {
98+
type: typeof ACTION_SET_PUSH_PADDING;
99+
side: 'left' | 'right';
100+
width: number;
101+
}
102+
94103
/** Union of all flyout manager actions. */
95104
export type Action =
96105
| AddFlyoutAction
@@ -100,7 +109,8 @@ export type Action =
100109
| SetLayoutModeAction
101110
| SetActivityStageAction
102111
| GoBackAction
103-
| GoToFlyoutAction;
112+
| GoToFlyoutAction
113+
| SetPushPaddingAction;
104114

105115
/**
106116
* Register a flyout with the manager.
@@ -173,3 +183,13 @@ export const goToFlyout = (flyoutId: string): GoToFlyoutAction => ({
173183
type: ACTION_GO_TO_FLYOUT,
174184
flyoutId,
175185
});
186+
187+
/** Set push padding offset for a specific side. */
188+
export const setPushPadding = (
189+
side: 'left' | 'right',
190+
width: number
191+
): SetPushPaddingAction => ({
192+
type: ACTION_SET_PUSH_PADDING,
193+
side,
194+
width,
195+
});

0 commit comments

Comments
 (0)