Merge pull request #676 from MBombeck/fix/settings-overscroll

Give the scroll container its own containing block

CHANGELOG.md

@@ -19,6 +19,17 @@
   again from the app ends up as a single entry rather than a duplicate or an
   error.
 
+### Fixed
+
+- A page no longer grows a second scrollbar for the moment between appearing and
+  becoming interactive. Every screen with a toggle briefly rendered a hidden form
+  control that sat outside the scrolling area, so the whole document became
+  scrollable to roughly twice the height of the screen until the page finished
+  loading. On a slow connection or a busy device the window was long enough to
+  flick past the end of a settings page and land on nothing. The scrolling area
+  now contains its own children, which closes the whole class rather than the one
+  page it was noticed on.
+
 ### Migration
 
 - This release removes duplicate ECG recordings. An existing database can hold

e2e/settings-overscroll.spec.ts

@@ -135,15 +135,40 @@ test.describe("settings + admin vertical over-scroll guard", () => {
         expect(dims, "main-content / wrapper present").not.toBeNull();
         if (!dims) return;
 
-        // One-scroll-floor (UI-STANDARDS §9): `<main>` is the ONLY vertical
-        // scroll surface — the document itself must never become scrollable
-        // next to it (a second painted scrollbar + a dead dark band under
-        // the shell). The historic offender class: an absolutely-positioned
-        // sr-only element (the dnd drag-hint paragraph) whose containing
-        // block resolves to the initial containing block because no ancestor
-        // between it and the root is positioned — its static position below
-        // a long list then extends the DOCUMENT's scrollable overflow while
-        // `<main>` clips everything in normal flow.
+        // One-scroll-floor (UI-STANDARDS §9), structural half. `<main>` may
+        // only be the ONLY vertical scroll surface if it also OWNS the
+        // containing block of its absolutely-positioned descendants: an
+        // abspos box with no positioned ancestor resolves against the
+        // initial containing block, which sits outside the scroll container,
+        // so `overflow-y-auto` never clips it and its static position
+        // extends the DOCUMENT's scrollable overflow instead.
+        //
+        // This is asserted separately from the height read below because the
+        // two catch the same defect at different reliabilities. The height
+        // read only sees an escapee that is mounted at measure time, and the
+        // escapees are mostly transient: a Radix `<Switch>` outside a
+        // `<form>` renders a hidden `position:absolute` bubble input on the
+        // first pass and drops it once its button ref resolves, and the root
+        // `loading.tsx` chart skeleton carries an `sr-only` announcement
+        // that lives only until the real content streams in. Both made the
+        // document scrollable for the whole pre-hydration window, and both
+        // are gone by the time a fast machine gets around to measuring, so
+        // the height read caught them on a loaded CI runner and nowhere
+        // else. The computed style of `<main>` is timing-free.
+        const mainPosition = await page.evaluate(
+          () =>
+            getComputedStyle(document.getElementById("main-content")!).position,
+        );
+        expect(
+          mainPosition,
+          "<main id=main-content> must establish a containing block, or an " +
+            "absolutely-positioned descendant escapes its overflow clip and " +
+            "becomes a second vertical scroll surface",
+        ).not.toBe("static");
+
+        // Same rule, measured: the document itself must never become
+        // scrollable next to `<main>` (a second painted scrollbar + a dead
+        // dark band under the shell).
         const doc = await page.evaluate(() => ({
           scrollHeight: document.documentElement.scrollHeight,
           clientHeight: document.documentElement.clientHeight,

src/components/layout/auth-shell.tsx

@@ -319,7 +319,25 @@ export function AuthShell({
             // wrapper then recentres and the whole column, including the
             // admin/settings sidebar, shifts a few px sideways on every
             // toggle. Reserving the gutter holds the layout still.
-            className="flex-1 [scrollbar-gutter:stable] overflow-y-auto pb-[calc(4rem+env(safe-area-inset-bottom,0px))] md:pb-0"
+            //
+            // `relative` makes this element the containing block for every
+            // absolutely-positioned descendant that has no nearer positioned
+            // ancestor. Without it those boxes resolve against the INITIAL
+            // containing block, which lives outside the scroll container, so
+            // `overflow-y-auto` never clips them: their static position sits
+            // wherever the flow put them and extends the DOCUMENT's
+            // scrollable overflow instead, painting a second vertical scroll
+            // surface beside `<main>` (UI-STANDARDS §9 one-scroll-floor).
+            // The class is not hypothetical and not confined to app code. A
+            // Radix `<Switch>` outside a `<form>` renders a hidden
+            // `position:absolute` bubble input on the first pass and drops it
+            // only once the button ref resolves, so every route carrying a
+            // switch below the fold (/settings/modules is the long one) grew
+            // a full-height document scrollbar for the entire pre-hydration
+            // window. Anchoring the scroll container closes the escape route
+            // for all of them at once instead of asking each new
+            // absolutely-positioned child to remember its own wrapper.
+            className="relative flex-1 [scrollbar-gutter:stable] overflow-y-auto pb-[calc(4rem+env(safe-area-inset-bottom,0px))] md:pb-0"
           >
             {/*
               v1.4.33 IW9 — container normalised on `max-w-screen-xl`