From 65f29db7e1b7d24ff14a7a99f3c8d98dc7344d40 Mon Sep 17 00:00:00 2001 From: Kalle <38327916+Sendouc@users.noreply.github.com> Date: Sun, 20 Sep 2026 18:02:58 +0300 Subject: [PATCH] Revert "Dialog scroll lock" This reverts commit 5906cdeb77c35e92cfaba46401fcea5b9b418fda. --- .../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, 2 insertions(+), 137 deletions(-) delete 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 10b0d737c..1d073b907 100644 --- a/app/components/elements/Dialog.browser.test.tsx +++ b/app/components/elements/Dialog.browser.test.tsx @@ -120,54 +120,6 @@ 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 3bbdd53c3..5f6df54a7 100644 --- a/app/components/elements/Dialog.tsx +++ b/app/components/elements/Dialog.tsx @@ -67,19 +67,9 @@ 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; - } - }} + ref={ref} id={id} className={clsx(className, { [styles.blurredBackdrop]: blurredBackdrop, @@ -110,79 +100,6 @@ 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 deleted file mode 100644 index e50102c9a..000000000 --- a/changelog/2026-09-20-dialog-scroll-lock.md +++ /dev/null @@ -1,4 +0,0 @@ ---- -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 58b764cdc..9f25d1886 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. */ -async function waitOutLossConfirmMinGap(page: Page) { +export 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); }