diff --git a/e2e/settings-overscroll.spec.ts b/e2e/settings-overscroll.spec.ts index e9dd554d4..1b11d2cd6 100644 --- a/e2e/settings-overscroll.spec.ts +++ b/e2e/settings-overscroll.spec.ts @@ -13,18 +13,28 @@ import { STORAGE_STATE_PATH } from "./setup/global-setup"; * the shell's own budget and reopens the dark-band-below-the-last-card * bug. * - * The assertion: `main.scrollHeight` ≤ bottom edge of the lowest content - * element + the shell-owned padding + tolerance. Runs across the - * settings routes (short hub, long sortable-list subpages, long form - * section) at desktop AND phone-shaped viewports, because the two shell + * The assertion: `main.scrollHeight` ≤ bottom edge of the lowest real + * content element + the shell-owned padding + tolerance. "Real content" + * is measured against each element's CONTENT-box bottom, not its padding- + * box: a sub-shell column that re-declares its own bottom gutter (`pb-*`) + * no longer folds that gutter into `contentBottom`, so a redundant reserve + * FAILS the assertion instead of being absorbed by it. Runs across both + * two-column sub-shells — Settings AND Admin (they share the grid-floor + * source) — at desktop AND phone-shaped viewports, because the two shell * paddings differ per breakpoint. */ const ROUTES = [ + // Settings sub-shell — short hub, long sortable-list subpages, long form. "/settings/layout", "/settings/layout/dashboard", "/settings/layout/insights", "/settings/account", "/settings/notifications", + // Admin sub-shell — short overview + a longer list page. Guards that the + // pre-#154 column reserve stays retired on every admin breakpoint. + "/admin", + "/admin/login-overview", + "/admin/system-status", ] as const; const VIEWPORTS = [ @@ -32,7 +42,7 @@ const VIEWPORTS = [ { name: "phone", width: 390, height: 844, expectedPad: 144 }, // pb-20 + main pb-16 ] as const; -test.describe("settings vertical over-scroll guard", () => { +test.describe("settings + admin vertical over-scroll guard", () => { test.use({ storageState: STORAGE_STATE_PATH }); test.beforeEach(({}, testInfo) => { @@ -56,11 +66,17 @@ test.describe("settings vertical over-scroll guard", () => { if (!main) return null; const wrapper = main.firstElementChild as HTMLElement | null; if (!wrapper) return null; - // Lowest content edge: max bottom over the wrapper's visible, - // non-fixed descendants, in the scroll container's coordinate - // space. Elements inside nested scroll containers are clipped - // by their own overflow and never add page scroll height, so - // skip anything whose scrollable ancestor is not `main`. + // Lowest content edge: max CONTENT-box bottom over the wrapper's + // visible, non-fixed descendants, in the scroll container's + // coordinate space. Using the content box (padding-box bottom + // minus the element's own bottom padding + border) means a + // layout column's own `pb-*` gutter is NOT counted — its last + // real child (a card) is still measured through the column's + // content box, so a redundant sub-shell bottom gutter shows up + // as pure over-scroll instead of inflating the allowed budget. + // Elements inside nested scroll containers are clipped by their + // own overflow and never add page scroll height, so skip + // anything whose scrollable ancestor is not `main`. let maxBottom = 0; const mainTop = main.getBoundingClientRect().top; for (const el of wrapper.querySelectorAll("*")) { @@ -84,7 +100,10 @@ test.describe("settings vertical over-scroll guard", () => { p = p.parentElement; } if (inner) continue; - const bottom = rect.bottom + main.scrollTop - mainTop; + const padBottom = parseFloat(cs.paddingBottom) || 0; + const borderBottom = parseFloat(cs.borderBottomWidth) || 0; + const bottom = + rect.bottom - padBottom - borderBottom + main.scrollTop - mainTop; if (bottom > maxBottom) maxBottom = bottom; } return { diff --git a/src/components/admin/admin-shell.tsx b/src/components/admin/admin-shell.tsx index 8f3935573..0029c85d3 100644 --- a/src/components/admin/admin-shell.tsx +++ b/src/components/admin/admin-shell.tsx @@ -37,6 +37,7 @@ import { } from "lucide-react"; import { cn } from "@/lib/utils"; +import { SUB_SHELL_GRID_FLOOR } from "@/components/layout/shell-metrics"; import { useAuth } from "@/hooks/use-auth"; import { useTranslations } from "@/lib/i18n/context"; import { isAdminSectionSlug, type AdminSectionSlug } from "./section-slugs"; @@ -304,7 +305,12 @@ export function AdminShell({ active, children }: AdminShellProps) { col 1, cards in row 2 / col 2. The nav's first item starts the same grid row as the first card, so they align by construction across locales and however the heading wraps — no fixed-height spacer. */} -
+
{/* Heading — desktop only (mobile renders it above the strip). */}
{headingBlock} @@ -366,14 +372,14 @@ export function AdminShell({ active, children }: AdminShellProps) { - {/* Main column (cards) — row 2 / col 2. Same - `min-h-[calc(100dvh-12rem)]` reserve as `` - so navigating between admin sub-pages (e.g. system-status - → login-overview) does not jump the page height while the - new section is still fetching. */} -
- {children} -
+ {/* Main column (cards) — row 2 / col 2, the grid's `1fr` row, so it + stretches to fill the shared `SUB_SHELL_GRID_FLOOR` on the grid + (matches ``) without over-shooting. The loading-jump + floor lives on the GRID, not here: a column-level + `min-h-[calc(100dvh-12rem)]` reserve (the pre-#154 shape this used + to carry) over-reserves on short sections and on mobile, scrolling + the page into a dark band below the last card. */} +
{children}
); diff --git a/src/components/layout/shell-metrics.ts b/src/components/layout/shell-metrics.ts new file mode 100644 index 000000000..6b9ea1252 --- /dev/null +++ b/src/components/layout/shell-metrics.ts @@ -0,0 +1,29 @@ +/** + * Shared scroll-floor for the two-column sub-shells (Settings, Admin). + * + * Scroll height is shell-owned (over-scroll class, #154 lineage): only + * `AuthShell` owns the scroll container's height and bottom padding — the + * `
` clears the mobile bottom-nav and its wrapper clears the Coach FAB. + * A sub-shell must NEVER re-declare a bottom gutter (`pb-*`) or a viewport + * reserve (`min-h-[calc(100dvh-…)]`) on its own content column: a nested + * reserve stacks on the shell budget and scrolls past the last card. + * + * The one sanctioned reserve is this loading-jump floor, applied to the + * sub-shell GRID (not to a column): it keeps the page height stable while a + * freshly-navigated section is still fetching, without over-shooting the + * viewport on a short section. The `
`/cards column is the grid's `1fr` + * row, so it stretches to fill exactly and never over-scrolls. + * + * Both sub-shells derive the floor from THIS single constant so the two can + * never drift — Admin previously carried a stale, all-breakpoint + * `min-h-[calc(100dvh-12rem)]` column reserve that this de-duplication retires. + * + * Budget: top bar 4rem + the AuthShell wrapper's `pt-6` (1.5rem) + `pb-20` + * (5rem) = 10.5rem. Retune here — and only here — if those shell paddings + * change. (Latent follow-up: this constant assumes zero banners above `
`; + * a locale/offline/demo banner or an iOS PWA safe-area inset makes it + * over-reserve. A fully self-accounting `min-h-full` chain would need the + * AuthShell content wrapper to pass a definite height through, which is out of + * scope for the shell-only over-scroll fix.) + */ +export const SUB_SHELL_GRID_FLOOR = "md:min-h-[calc(100dvh-10.5rem)]"; diff --git a/src/components/settings/settings-shell.tsx b/src/components/settings/settings-shell.tsx index 4254f5001..b2f92ded7 100644 --- a/src/components/settings/settings-shell.tsx +++ b/src/components/settings/settings-shell.tsx @@ -41,6 +41,7 @@ import { } from "lucide-react"; import { cn } from "@/lib/utils"; +import { SUB_SHELL_GRID_FLOOR } from "@/components/layout/shell-metrics"; import { useAuth } from "@/hooks/use-auth"; import { useMounted } from "@/hooks/use-mounted"; import { useTranslations } from "@/lib/i18n/context"; @@ -506,7 +507,12 @@ export function SettingsShell({ holds across locales and however the title or subtitle wraps. No fixed-height spacer to guess. The `gap-6` between rows reproduces the `space-y-6` the heading-to-first-card gap used to carry. */} -
+
{/* Heading — desktop only here (mobile renders it above the strip). */} {headingBlock ? (
@@ -570,19 +576,18 @@ export function SettingsShell({ still prevents the short-loading-state → long-list height jump the old reserve guarded, just measured against the real budget. - v1.4.33 F14 — `pb-24 md:pb-0` reserves a 96 px bottom gutter - on `` already reserves `pb-[calc(4rem+safe-area)]` on ``, not `
`: the surrounding AuthShell already provides the page's single `
` landmark and a nested second one is an a11y violation. */} -
- {children} -
+
{children}
);