diff --git a/semcore/core/__tests__/usePreventScroll.test.tsx b/semcore/core/__tests__/usePreventScroll.test.tsx index ee86e48592..e37dda4e7b 100644 --- a/semcore/core/__tests__/usePreventScroll.test.tsx +++ b/semcore/core/__tests__/usePreventScroll.test.tsx @@ -1,5 +1,5 @@ import { cleanup, renderHook } from '@semcore/testing-utils/testing-library'; -import { expect, test, describe, beforeEach, afterEach } from '@semcore/testing-utils/vitest'; +import { expect, test, describe, beforeEach, afterEach, vi } from '@semcore/testing-utils/vitest'; import usePreventScroll from '../src/utils/use/usePreventScroll'; @@ -63,6 +63,66 @@ describe('usePreventScroll', () => { expect(document.body.style.overflow).toBe(''); }); + test.sequential.each(['hidden', 'clip'])( + 'Verify skips locking when body overflow is already %s', + (overflow) => { + const spy = vi + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ paddingRight: '0px', overflow } as CSSStyleDeclaration); + + const { unmount } = renderHook(() => usePreventScroll(true)); + + expect(document.body.style.overflow).toBe(''); + expect(document.body.style.paddingRight).toBe(''); + expect(document.body.style.boxSizing).toBe(''); + + unmount(); + spy.mockRestore(); + }, + ); + + test.sequential.each(['hidden', 'clip'])( + 'Verify preserves pre-existing inline overflow:%s through skip and unmount', + (overflow) => { + document.body.style.overflow = overflow; + + const spy = vi + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ paddingRight: '0px', overflow } as CSSStyleDeclaration); + + const { unmount } = renderHook(() => usePreventScroll(true)); + + // Nothing should have been changed on mount + expect(document.body.style.overflow).toBe(overflow); + expect(document.body.style.paddingRight).toBe(''); + expect(document.body.style.boxSizing).toBe(''); + + unmount(); + spy.mockRestore(); + + // Pre-existing inline style must remain intact after unmount + expect(document.body.style.overflow).toBe(overflow); + }, + ); + + test.sequential.each(['scroll', 'auto', 'visible'])( + 'Verify does not skip locking when body overflow is %s', + (overflow) => { + const spy = vi + .spyOn(window, 'getComputedStyle') + .mockReturnValue({ paddingRight: '0px', overflow } as CSSStyleDeclaration); + + const { unmount } = renderHook(() => usePreventScroll(true)); + + // Normal locking should still apply for non-skipped overflow values + expect(document.body.style.overflow).toBe('hidden'); + expect(document.body.style.boxSizing).toBe('border-box'); + + unmount(); + spy.mockRestore(); + }, + ); + test.sequential('Verify reacts to visible changing from true to false', () => { const { rerender, unmount } = renderHook(({ visible }) => usePreventScroll(visible), { initialProps: { visible: true }, diff --git a/semcore/core/src/utils/use/usePreventScroll.tsx b/semcore/core/src/utils/use/usePreventScroll.tsx index 3f978c3f62..a96553bf3d 100644 --- a/semcore/core/src/utils/use/usePreventScroll.tsx +++ b/semcore/core/src/utils/use/usePreventScroll.tsx @@ -36,6 +36,8 @@ const lockedBodyStyles = { overflow: '', boxSizing: '', }; + +const overflowValuesToSkip = new Set(['clip', 'hidden']); export default function usePreventScroll(visible = true, disabled = false) { const scrollbarWidth = React.useMemo(getScrollbarWidth, [getScrollbarWidth]); const id = useUID('scroll-preventer-'); @@ -46,12 +48,14 @@ export default function usePreventScroll(visible = true, disabled = false) { scrollPreventers.add(id); if (scrollPreventers.size > 1) return; - const { paddingRight } = window.getComputedStyle(document.body); + const { paddingRight, overflow } = window.getComputedStyle(document.body); lockedBodyStyles.paddingRight = document.body.style.paddingRight; lockedBodyStyles.overflow = document.body.style.overflow; lockedBodyStyles.boxSizing = document.body.style.boxSizing; + if (overflowValuesToSkip.has(overflow)) return; + const intPaddingRight = getIntValueFromCss(paddingRight); let intPaddingRightFromStyle = getIntValueFromCss(document.body.style.paddingRight); // Detected own style for window inside window diff --git a/semcore/modal/__tests__/modal.browser-test.tsx b/semcore/modal/__tests__/modal.browser-test.tsx index 78b41933e9..d456f8b268 100644 --- a/semcore/modal/__tests__/modal.browser-test.tsx +++ b/semcore/modal/__tests__/modal.browser-test.tsx @@ -664,4 +664,73 @@ test.describe(`@modal ${TAG.FUNCTIONAL}`, () => { }); }); }); + + test.describe('Modal body scroll lock', () => { + const getBodyInlineStyles = (page: Page) => + page.evaluate(() => ({ + overflow: document.body.style.overflow, + paddingRight: document.body.style.paddingRight, + boxSizing: document.body.style.boxSizing, + })); + + test('Verify modal does not cause layout shift when body overflow is hidden', { + tag: [TAG.PRIORITY_MEDIUM, TAG.MOUSE, '@modal'], + }, async ({ page, browserName }) => { + test.skip(browserName !== 'chromium', 'Scroll-lock logic is engine-independent; covered by unit tests'); + await loadPage(page, 'stories/components/modal/advanced/examples/modal_causes_layout_shift.tsx', 'en'); + + await test.step('Switch body overflow to hidden', async () => { + await locators.button(page, 'Toggle body overflow').click(); + await expect(page.getByText('Current overflow value: hidden')).toBeVisible(); + }); + + await test.step('Open modal and verify body styles are untouched', async () => { + await locators.button(page, 'Open modal').click(); + await expect(locators.dialog(page)).toBeVisible(); + + const styles = await getBodyInlineStyles(page); + expect(styles.boxSizing).toBe(''); + expect(styles.paddingRight).toBe(''); + expect(styles.overflow).toBe('hidden'); + }); + + await test.step('Close modal and verify body overflow is preserved', async () => { + await page.keyboard.press('Escape'); + await expect(locators.dialog(page)).toBeHidden(); + expect((await getBodyInlineStyles(page)).overflow).toBe('hidden'); + }); + }); + + test('Verify stacked modals keep body locked until both close', { + tag: [TAG.PRIORITY_MEDIUM, TAG.MOUSE, '@modal'], + }, async ({ page, browserName }) => { + test.skip(browserName !== 'chromium', 'Scroll-lock logic is engine-independent; covered by unit tests'); + await loadPage(page, 'stories/components/modal/advanced/examples/modal_causes_layout_shift.tsx', 'en'); + + await test.step('Open first modal — body gets locked', async () => { + await locators.button(page, 'Open modal').click(); + await expect(locators.modal(page, 0)).toBeVisible(); + expect((await getBodyInlineStyles(page)).overflow).toBe('hidden'); + }); + + await test.step('Open nested modal over the first one', async () => { + await locators.button(page, 'Open modal over modal').click(); + await expect(locators.dialog(page, 'Nested modal')).toBeVisible(); + expect((await getBodyInlineStyles(page)).overflow).toBe('hidden'); + }); + + await test.step('Close nested modal — body stays locked by the first modal', async () => { + await locators.button(page, 'Close nested modal').click(); + await expect(locators.dialog(page, 'Nested modal')).toBeHidden(); + await expect(locators.modal(page, 0)).toBeVisible(); + expect((await getBodyInlineStyles(page)).overflow).toBe('hidden'); + }); + + await test.step('Close first modal — body is unlocked and restored', async () => { + await page.keyboard.press('Escape'); + await expect(locators.modal(page)).toBeHidden(); + expect((await getBodyInlineStyles(page)).overflow).toBe('visible'); + }); + }); + }); }); diff --git a/stories/components/modal/advanced/examples/modal_causes_layout_shift.tsx b/stories/components/modal/advanced/examples/modal_causes_layout_shift.tsx new file mode 100644 index 0000000000..78bddcafb1 --- /dev/null +++ b/stories/components/modal/advanced/examples/modal_causes_layout_shift.tsx @@ -0,0 +1,72 @@ +import { Flex } from '@semcore/ui/base-components'; +import Button from '@semcore/ui/button'; +import Modal from '@semcore/ui/modal'; +import { Text } from '@semcore/ui/typography'; +import React from 'react'; + +const overflowValues = ['visible', 'hidden', 'clip'] as const; + +const Demo = () => { + const [overflow, setOverflow] = React.useState<(typeof overflowValues)[number]>('visible'); + const [visible, setVisible] = React.useState(false); + const [nestedVisible, setNestedVisible] = React.useState(false); + + const handleOpen = React.useCallback(() => setVisible(true), []); + const handleClose = React.useCallback(() => setVisible(false), []); + const handleOpenNested = React.useCallback(() => setNestedVisible(true), []); + const handleCloseNested = React.useCallback(() => setNestedVisible(false), []); + + React.useEffect(() => { + document.body.style.overflow = overflow; + return () => { + document.body.style.removeProperty('overflow'); + }; + }, [overflow]); + + return ( + + + Current overflow value: {overflow} + + + + + + Do you want to save your changes? + + Your changes will be lost if you don't save them. + + + + + + + + Nested modal + + This modal is opened over another modal. + + + + + ); +}; + +export default Demo; diff --git a/stories/components/modal/advanced/modal.stories.tsx b/stories/components/modal/advanced/modal.stories.tsx index f0eb53daea..4c1379a2d2 100644 --- a/stories/components/modal/advanced/modal.stories.tsx +++ b/stories/components/modal/advanced/modal.stories.tsx @@ -1,6 +1,7 @@ import type { Meta, StoryObj } from '@storybook/react-vite'; import CloseOnlyEscOrCloseButtonExample from './examples/close_only_esc_or_close_button.tsx'; +import ModalCausesLayoutShiftExample from './examples/modal_causes_layout_shift.tsx'; import ModaliFrameExample from './examples/modal_iframe'; import OpenModalDynamicallyExample from './examples/modal_open_dynamically'; import ModalPreventFocusExample from './examples/modal_prevent_focus'; @@ -46,3 +47,7 @@ export const ModalIcon: StoryObj = { export const OutsideClick: StoryObj = { render: OutsideClickExample, }; + +export const ModalCausesLayoutShift: StoryObj = { + render: ModalCausesLayoutShiftExample, +};