mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
fix(shell): make AuthShell the single owner of scroll bottom-space
SettingsShell's content column carried pb-24 md:pb-0 that double-counted the bottom-nav clearance AuthShell's <main> already reserves, stacking ~96 px of dead scroll below the last card on mobile. AdminShell still carried the pre-#154 min-h-[calc(100dvh-12rem)] column reserve on <main>, over-reserving on short sections at every breakpoint. Delete the redundant SettingsShell gutter, move AdminShell's floor off <main> onto its grid, and factor the budget-derived grid floor into one shared SUB_SHELL_GRID_FLOOR constant so the two shells can never drift. Extend the guard to /admin/* and measure against each element's content-box bottom so a redundant sub-shell pb-* now fails instead of inflating the allowed budget.
This commit is contained in:
1 parent
79a78162e2
commit
df78b29cfd
4 files changed
+89
-30
No files matched your search
@@ -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<HTMLElement>("*")) {
|
||||
@@ -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 {
|
||||
|
||||
@@ -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. */}
|
||||
<div className="grid gap-6 md:grid-cols-[220px_1fr] md:grid-rows-[auto_1fr]">
|
||||
<div
|
||||
className={cn(
|
||||
"grid gap-6 md:grid-cols-[220px_1fr] md:grid-rows-[auto_1fr]",
|
||||
SUB_SHELL_GRID_FLOOR,
|
||||
)}
|
||||
>
|
||||
{/* Heading — desktop only (mobile renders it above the strip). */}
|
||||
<div className="hidden md:col-start-2 md:row-start-1 md:block">
|
||||
{headingBlock}
|
||||
@@ -366,14 +372,14 @@ export function AdminShell({ active, children }: AdminShellProps) {
|
||||
</ul>
|
||||
</aside>
|
||||
|
||||
{/* Main column (cards) — row 2 / col 2. Same
|
||||
`min-h-[calc(100dvh-12rem)]` reserve as `<SettingsShell>`
|
||||
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. */}
|
||||
<main className="min-h-[calc(100dvh-12rem)] min-w-0 md:col-start-2 md:row-start-2">
|
||||
{children}
|
||||
</main>
|
||||
{/* 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 `<SettingsShell>`) 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. */}
|
||||
<main className="min-w-0 md:col-start-2 md:row-start-2">{children}</main>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -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
|
||||
* `<main>` 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 `<main>`/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 `<main>`;
|
||||
* 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)]";
|
||||
@@ -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. */}
|
||||
<div className="grid gap-6 md:min-h-[calc(100dvh-10.5rem)] md:grid-cols-[220px_1fr] md:grid-rows-[auto_1fr]">
|
||||
<div
|
||||
className={cn(
|
||||
"grid gap-6 md:grid-cols-[220px_1fr] md:grid-rows-[auto_1fr]",
|
||||
SUB_SHELL_GRID_FLOOR,
|
||||
)}
|
||||
>
|
||||
{/* Heading — desktop only here (mobile renders it above the strip). */}
|
||||
{headingBlock ? (
|
||||
<div className="hidden md:col-start-2 md:row-start-1 md:block">
|
||||
@@ -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 `<md` so the last form field on
|
||||
`/settings/account` (Geschlecht / Gender) and
|
||||
`/settings/ai` (Aktiver Provider) is not eaten by the
|
||||
floating mobile bottom-nav. Desktop reverts to the parent's
|
||||
own padding because the bottom-nav doesn't render on `md+`.
|
||||
The column carries NO bottom gutter of its own. AuthShell's
|
||||
`<main>` already reserves `pb-[calc(4rem+safe-area)]` on `<md` to
|
||||
clear the floating bottom-nav, and its wrapper reserves `pb-20` for
|
||||
the Coach FAB. The old `pb-24 md:pb-0` (v1.4.33 F14) double-counted
|
||||
that same 64 px nav clearance and stacked ~96 px of dead scroll
|
||||
below the last card on mobile — removed so the shell owns all
|
||||
bottom-space (over-scroll class, #154 lineage).
|
||||
|
||||
v1.16.4 — a `<div>`, not `<main>`: the surrounding AuthShell
|
||||
already provides the page's single `<main>` landmark and a
|
||||
nested second one is an a11y violation. */}
|
||||
<div className="min-w-0 pb-24 md:col-start-2 md:row-start-2 md:pb-0">
|
||||
{children}
|
||||
</div>
|
||||
<div className="min-w-0 md:col-start-2 md:row-start-2">{children}</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user