From 5906cdeb77c35e92cfaba46401fcea5b9b418fda Mon Sep 17 00:00:00 2001 From: Kalle <38327916+Sendouc@users.noreply.github.com> Date: Sun, 20 Sep 2026 12:07:43 +0300 Subject: [PATCH] Dialog scroll lock --- .../elements/Dialog.browser.test.tsx | 48 +++++++++++ app/components/elements/Dialog.tsx | 85 ++++++++++++++++++- changelog/2026-09-20-dialog-scroll-lock.md | 4 + e2e/pages/sendouq/sendouq-match-page.ts | 2 +- 4 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 changelog/2026-09-20-dialog-scroll-lock.md diff --git a/app/components/elements/Dialog.browser.test.tsx b/app/components/elements/Dialog.browser.test.tsx index 1d073b907..10b0d737c 100644 --- a/app/components/elements/Dialog.browser.test.tsx +++ b/app/components/elements/Dialog.browser.test.tsx @@ -120,6 +120,54 @@ describe("SendouDialog", () => { expect(openDialog().open).toBe(true); }); + test("locks page scrolling while open without changing the page width", async () => { + const tall = document.createElement("div"); + tall.style.height = "300vh"; + document.body.appendChild(tall); + cleanupFns.push(() => tall.remove()); + const root = document.documentElement; + const widthBefore = root.clientWidth; + + const screen = await render( + withRouter( + Open} + showCloseButton + > + Content + , + ), + ); + + await screen.getByRole("button", { name: "Open" }).click(); + await expect.element(screen.getByText("Content")).toBeVisible(); + await vi.waitFor(() => expect(root.style.overflow).toBe("hidden")); + expect(root.clientWidth).toBe(widthBefore); + + await screen.getByRole("button", { name: "Close" }).click(); + await vi.waitFor(() => expect(root.style.overflow).toBe("")); + expect(root.style.scrollbarGutter).toBe(""); + expect(root.clientWidth).toBe(widthBefore); + }); + + test("releases the scroll lock when an open dialog unmounts", async () => { + const screen = await render( + withRouter( + {}}> + Content + , + ), + ); + await expect.element(screen.getByText("Content")).toBeVisible(); + await vi.waitFor(() => + expect(document.documentElement.style.overflow).toBe("hidden"), + ); + + await screen.unmount(); + expect(document.documentElement.style.overflow).toBe(""); + }); + test("focuses the dialog itself instead of the close button on open", async () => { await render( withRouter( diff --git a/app/components/elements/Dialog.tsx b/app/components/elements/Dialog.tsx index 5f6df54a7..3bbdd53c3 100644 --- a/app/components/elements/Dialog.tsx +++ b/app/components/elements/Dialog.tsx @@ -67,9 +67,19 @@ function DialogElement({ children, ref, }: DialogElementProps) { + const dialogRef = React.useRef(null); + useScrollLockWhileOpen(dialogRef); + return ( { + dialogRef.current = dialog; + if (typeof ref === "function") { + ref(dialog); + } else if (ref) { + ref.current = dialog; + } + }} id={id} className={clsx(className, { [styles.blurredBackdrop]: blurredBackdrop, @@ -100,6 +110,79 @@ function closeOnBackdropClick(event: React.MouseEvent) { } } +/** + * Locks page scrolling for as long as the dialog is open. Open state is read + * from the DOM (`toggle` events plus the initial `open`) rather than React + * state, so a dialog opened before hydration or closed natively is covered. + */ +function useScrollLockWhileOpen( + dialogRef: React.RefObject, +) { + React.useEffect(() => { + const dialog = dialogRef.current; + if (!dialog) return; + + let release: (() => void) | undefined; + const syncLock = (isOpen: boolean) => { + if (isOpen) { + release ??= lockPageScroll(); + } else { + release?.(); + release = undefined; + } + }; + const onToggle = (event: Event) => { + syncLock((event as ToggleEvent).newState === "open"); + }; + + dialog.addEventListener("toggle", onToggle); + syncLock(dialog.open); + return () => { + dialog.removeEventListener("toggle", onToggle); + syncLock(false); + }; + }, [dialogRef]); +} + +let pageScrollLocks = 0; +let restorePageScroll: (() => void) | undefined; + +/** + * Hides the root scrollbar without the page reflowing into its space: when a + * classic scrollbar was taking up width, `scrollbar-gutter: stable` keeps that + * width reserved. Overlay scrollbars take no width so nothing is reserved. + * Reference counted so nested dialogs release the lock only once all close. + */ +function lockPageScroll() { + if (pageScrollLocks++ === 0) { + const root = document.documentElement; + const scrollbarTakesWidth = window.innerWidth > root.clientWidth; + const previous = { + overflow: root.style.overflow, + scrollbarGutter: root.style.scrollbarGutter, + }; + + root.style.overflow = "hidden"; + if (scrollbarTakesWidth) { + root.style.scrollbarGutter = "stable"; + } + restorePageScroll = () => { + root.style.overflow = previous.overflow; + root.style.scrollbarGutter = previous.scrollbarGutter; + }; + } + + let released = false; + return () => { + if (released) return; + released = true; + if (--pageScrollLocks === 0) { + restorePageScroll?.(); + restorePageScroll = undefined; + } + }; +} + /** Invoker commands open and close the dialog natively; this guards the JS fallback for browsers without them. */ function supportsInvokerCommands() { return "commandForElement" in HTMLButtonElement.prototype; diff --git a/changelog/2026-09-20-dialog-scroll-lock.md b/changelog/2026-09-20-dialog-scroll-lock.md new file mode 100644 index 000000000..e50102c9a --- /dev/null +++ b/changelog/2026-09-20-dialog-scroll-lock.md @@ -0,0 +1,4 @@ +--- +type: bug +--- +The page behind an open dialog no longer scrolls, and the layout stays put when the scrollbar is hidden diff --git a/e2e/pages/sendouq/sendouq-match-page.ts b/e2e/pages/sendouq/sendouq-match-page.ts index 9f25d1886..58b764cdc 100644 --- a/e2e/pages/sendouq/sendouq-match-page.ts +++ b/e2e/pages/sendouq/sendouq-match-page.ts @@ -251,7 +251,7 @@ export class SendouQMatchPage { } /** Waits out the gap the loss confirm keeps between arming and accepting the second tap. */ -export async function waitOutLossConfirmMinGap(page: Page) { +async function waitOutLossConfirmMinGap(page: Page) { // biome-ignore lint/nursery/noPlaywrightWaitForTimeout: the min gap after arming has no observable end await page.waitForTimeout(LOSS_CONFIRM_MIN_GAP_MS); }