Skip to content

Commit 348500d

Browse files
committed
Fix managed flyout body padding bug by conditionally applying push padding only for active flyouts and disabling scroll lock for overlay flyouts when push padding exists.
1 parent 1a7492f commit 348500d

File tree

1 file changed

+93
-25
lines changed

1 file changed

+93
-25
lines changed

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

Lines changed: 93 additions & 25 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,7 @@ import {
4041
useFlyoutLayoutMode,
4142
useFlyoutId,
4243
useFlyoutWidth,
44+
useIsFlyoutActive,
4345
} from './manager';
4446

4547
import { CommonProps, PropsOfElement } from '../common';
@@ -49,6 +51,7 @@ import type { EuiButtonIconPropsForButton } from '../button';
4951
import { EuiI18n } from '../i18n';
5052
import { useResizeObserver } from '../observer/resize_observer';
5153
import { EuiScreenReaderOnly } from '../accessibility';
54+
import { useMutationObserver } from '../observer/mutation_observer';
5255

5356
import { EuiFlyoutCloseButton } from './_flyout_close_button';
5457
import { euiFlyoutStyles, composeFlyoutInlineStyles } from './flyout.styles';
@@ -267,6 +270,13 @@ export const EuiFlyoutComponent = forwardRef(
267270
const internalParentFlyoutRef = useRef<HTMLDivElement>(null);
268271
const isPushed = useIsPushed({ type, pushMinBreakpoint });
269272

273+
// Get managed flyout context early so it's available for effects
274+
const currentSession = useCurrentSession();
275+
const isInManagedContext = useIsInManagedFlyout();
276+
const flyoutId = useFlyoutId(id);
277+
const layoutMode = useFlyoutLayoutMode();
278+
const isActiveManagedFlyout = useIsFlyoutActive(flyoutId);
279+
270280
const {
271281
onMouseDown: onMouseDownResizableButton,
272282
onKeyDown: onKeyDownResizableButton,
@@ -294,31 +304,56 @@ export const EuiFlyoutComponent = forwardRef(
294304
]);
295305
const { width } = useResizeObserver(isPushed ? resizeRef : null, 'width');
296306

297-
useEffect(() => {
307+
/**
308+
* Use useLayoutEffect (not useEffect) to ensure padding changes happen synchronously
309+
* before child components render. This prevents RemoveScrollBar from measuring the body
310+
* in an inconsistent state during flyout transitions.
311+
*/
312+
useLayoutEffect(() => {
298313
/**
299-
* Accomodate for the `isPushed` state by adding padding to the body equal to the width of the element
314+
* Accomodate for the `isPushed` state by adding padding to the body equal to the width of the element.
315+
* For managed flyouts, only apply padding if this flyout is active.
300316
*/
301-
if (isPushed) {
302-
const paddingSide =
303-
side === 'left' ? 'paddingInlineStart' : 'paddingInlineEnd';
304-
const cssVarName = `--euiPushFlyoutOffset${
305-
side === 'left' ? 'InlineStart' : 'InlineEnd'
306-
}`;
317+
if (!isPushed) {
318+
return; // Only push-type flyouts manage body padding
319+
}
307320

308-
document.body.style[paddingSide] = `${width}px`;
321+
const shouldApplyPadding = !isInManagedContext || isActiveManagedFlyout;
322+
323+
const paddingSide =
324+
side === 'left' ? 'paddingInlineStart' : 'paddingInlineEnd';
325+
const cssVarName = `--euiPushFlyoutOffset${
326+
side === 'left' ? 'InlineStart' : 'InlineEnd'
327+
}`;
309328

310-
// EUI doesn't use this css variable, but it is useful for consumers
329+
if (shouldApplyPadding) {
330+
document.body.style[paddingSide] = `${width}px`;
311331
setGlobalCSSVariables({
312332
[cssVarName]: `${width}px`,
313333
});
314-
return () => {
315-
document.body.style[paddingSide] = '';
316-
setGlobalCSSVariables({
317-
[cssVarName]: null,
318-
});
319-
};
334+
} else {
335+
// Explicitly remove padding when this push flyout becomes inactive
336+
document.body.style[paddingSide] = '';
337+
setGlobalCSSVariables({
338+
[cssVarName]: null,
339+
});
320340
}
321-
}, [isPushed, setGlobalCSSVariables, side, width]);
341+
342+
// Cleanup on unmount
343+
return () => {
344+
document.body.style[paddingSide] = '';
345+
setGlobalCSSVariables({
346+
[cssVarName]: null,
347+
});
348+
};
349+
}, [
350+
isPushed,
351+
isInManagedContext,
352+
isActiveManagedFlyout,
353+
setGlobalCSSVariables,
354+
side,
355+
width,
356+
]);
322357

323358
/**
324359
* This class doesn't actually do anything by EUI, but is nice to add for consumers (JIC)
@@ -331,13 +366,6 @@ export const EuiFlyoutComponent = forwardRef(
331366
};
332367
}, []);
333368

334-
const currentSession = useCurrentSession();
335-
const isInManagedContext = useIsInManagedFlyout();
336-
337-
// Get flyout manager context for dynamic width calculation
338-
const flyoutId = useFlyoutId(id);
339-
const layoutMode = useFlyoutLayoutMode();
340-
341369
// Memoize flyout identification and relationships to prevent race conditions
342370
const flyoutIdentity = useMemo(() => {
343371
if (!flyoutId || !currentSession) {
@@ -603,6 +631,46 @@ export const EuiFlyoutComponent = forwardRef(
603631

604632
const maskCombinedRefs = useCombinedRefs([maskProps?.maskRef, maskRef]);
605633

634+
/**
635+
* Track whether body padding from a push flyout exists to coordinate scroll locking.
636+
* Use state to allow updates when padding changes.
637+
*/
638+
const [hasPushFlyoutPadding, setHasPushFlyoutPadding] = useState(() => {
639+
if (isPushed || !isInManagedContext) return false;
640+
const leftPadding = document.body.style.paddingInlineStart;
641+
const rightPadding = document.body.style.paddingInlineEnd;
642+
return !!(leftPadding || rightPadding);
643+
});
644+
645+
/**
646+
* Monitor body padding and update state when it changes.
647+
* This allows overlay flyouts to enable scroll lock once push padding is removed.
648+
*/
649+
const checkPadding = useCallback(() => {
650+
const leftPadding = document.body.style.paddingInlineStart;
651+
const rightPadding = document.body.style.paddingInlineEnd;
652+
const hasPadding = !!(leftPadding || rightPadding);
653+
setHasPushFlyoutPadding(hasPadding);
654+
}, []);
655+
656+
// Monitor body style changes for overlay flyouts in managed contexts
657+
useMutationObserver(
658+
isPushed || !isInManagedContext ? null : document.body,
659+
checkPadding,
660+
{ attributeFilter: ['style'] }
661+
);
662+
663+
// Check padding state immediately after render
664+
useLayoutEffect(() => {
665+
if (!isPushed && isInManagedContext) {
666+
checkPadding();
667+
} else {
668+
setHasPushFlyoutPadding(false);
669+
}
670+
}, [isPushed, isInManagedContext, checkPadding]);
671+
672+
const shouldUseScrollLock = hasOverlayMask && !hasPushFlyoutPadding;
673+
606674
return (
607675
<EuiFlyoutOverlay
608676
hasOverlayMask={hasOverlayMask}
@@ -616,7 +684,7 @@ export const EuiFlyoutComponent = forwardRef(
616684
<EuiWindowEvent event="keydown" handler={onKeyDown} />
617685
<EuiFocusTrap
618686
disabled={isPushed}
619-
scrollLock={hasOverlayMask}
687+
scrollLock={shouldUseScrollLock}
620688
clickOutsideDisables={!ownFocus}
621689
onClickOutside={onClickOutside}
622690
{...focusTrapProps}

0 commit comments

Comments
 (0)