From 8128ec2f42e37cf078feb446c6c0f10705fbe92b Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Wed, 9 Sep 2026 15:52:46 +0200 Subject: [PATCH] Keep the reader in place when the drawer closes, and stop the CSS promising a header it deleted --- AGENTS.md | 27 +++++++++++++-------------- CHANGELOG.md | 5 ++++- e2e-tests/visual.spec.ts | 28 +++++++++++++++++++++++++--- public/css/styles.css | 3 +-- src/ui/data-table.test.ts | 1 + 5 files changed, 44 insertions(+), 20 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6919f70..62a2de1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,12 +36,15 @@ branch, create a PR and merge it when the CI/CD turns green. ## Project priorities (do not erode) 1. **Simplicity** — prefer the solution that is easiest to understand, smallest, and most readable. - **A page is a document**: it scrolls, and the chrome scrolls with it. Bounding the viewport to - hold something still — a `100dvh` box, `position: sticky`/`fixed` chrome, `overflow: hidden` on - `body` — buys an app-like look with CSS the next reader has to reverse-engineer, and every such - box is one more thing to undo before the content under it can be reached. Sticky headers and - full-height panes do not earn that. The mobile off-canvas nav is the one exception, because an - overlay has no other spelling. + **A page is a document**: it scrolls, and the chrome scrolls with it. Nothing may bound the + viewport to hold content still — no `height: 100dvh` frame, no `overflow: hidden` on `body`, no + `position: sticky` header. Each such box buys an app-like look with CSS the next reader has to + reverse-engineer, and is one more thing to undo before the content under it can be reached. + Overlays are not this: the skip link, the mobile off-canvas nav and its scrim sit *above* the + document rather than holding it still, and have no other spelling — the document keeps scrolling + behind the open nav, accepted rather than overlooked. Only the document scroller gets keyboard + paging unconditionally and back/forward scroll restoration, and a page a box clips fails silently: + nothing in a test or a console says content is unreachable below the fold. 2. **Few dependencies** — runtime deps stay minimal (today `ejs`, `lucide-static`, `@larvit/log`, `postgres`). Prefer the Node standard library; justify any new dependency; do not add frameworks. The **host is stateless — it owns no schema and stores nothing of its own**; a plugin may own a @@ -284,14 +287,10 @@ Revisit only if the stated reason stops holding. it means disclosure rather than popup: the nav tree. `shell.ejs` hand-rolls the same block for the profile menu (its trigger composes escaped user values and its one item is a CSRF POST form) — keep the two in step. -- **Nothing bounds the viewport, so there is one scroller: the document.** `.app` is - `min-height: 100dvh`, never a `100dvh` box with `overflow: hidden`. It was the latter once, and a - page taller than the window lost everything past it — silently, in every engine, because nothing in - a test or a console says content is unreachable below the fold. Only the document scroller gets - **keyboard paging** unconditionally and **scroll restoration** on back/forward; a bounded region - gets keys only once focus is inside it, which without script needs a focusable descendant. - A plugin that wants a full-height pane owns that in its own stylesheet — the shell offers no opt-out - and no `.table thead` stickiness, per priority 1. +- **One scroller, the document** (priority 1). `.app` is `min-height: 100dvh`. `.nav`'s + `overflow-y: auto` and `.side-footer`'s `flex: 0 0 auto` are not leftovers of a bounded frame: + they are what makes the off-canvas panel usable with a long tree. `#nav-toggle` is `position: fixed` + for the same reason — a label click focuses it, and a browser scrolls a focused element into view. - **`ICON_NAMES` (`src/ui/icons.ts`) is a host-owned registry, not a frozen plugin contract**, so it is deliberately not re-exported from `@plainpages/plugin-api`. The palette may narrow when the last reference to an id goes, and a plugin needing one gets it re-registered in the same change. Accepted cost: an diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e4f76a..c709ee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,8 +30,11 @@ shell offers no opt-out, per the simplicity priority in `AGENTS.md`. 3. A page holding a region that filled the content column (`flex: 1 1 auto; min-height: 0` with its own `overflow`) no longer gets a bounded column to fill, so that region grows and the page scrolls. Either let it, or give the region its own height in the plugin's stylesheet. -4. A `data-table` keeps working; its header no longer stays put while the rows scroll. +4. A `data-table` no longer scrolls its rows in a bounded region: the page scrolls, and the header + scrolls with it. +The sidebar stretches the whole document, so on a long page its footer — theme, language, profile and +**Sign out** — sits at the end of that page rather than the bottom of the screen. ## 0.3.0 diff --git a/e2e-tests/visual.spec.ts b/e2e-tests/visual.spec.ts index c30c61d..4e5c93d 100644 --- a/e2e-tests/visual.spec.ts +++ b/e2e-tests/visual.spec.ts @@ -28,8 +28,8 @@ test.beforeEach(async ({ context }) => { await context.addCookies([{ name: SESSION_COOKIE, url: BASE_URL, value: devSession() }]); }); -// A key press, not scrollIntoView — a script can scroll an overflow-hidden box, a reader cannot; -// and not the wheel, which Firefox's synthetic event never delivers to the document. +// A key press, not scrollIntoView (a script can scroll a box no reader can) and not the wheel +// (Firefox's synthetic event never reaches the document). for (const [name, path, tail] of [ ["the starter dashboard", "/dashboard", ".form-actions .btn"], ["the public landing", "/", ".landing-actions .btn"], @@ -42,12 +42,34 @@ for (const [name, path, tail] of [ const overflows = await page.evaluate(() => document.documentElement.scrollHeight > window.innerHeight); expect(overflows, "the page must overflow, or it proves nothing").toBe(true); await page.keyboard.press("End"); - // Whole, not merely touched: a control half under the fold is not reachable either. await expect(page.locator(tail).last()).toBeInViewport({ ratio: 1 }); }); } } +// The drawer is an overlay, so the document keeps scrolling behind it. Closing it must then leave +// the reader where they were: the toggle is focusable and its label is what a tap hits, and a +// browser scrolls a focused element into view — from the top of the document that meant jumping +// there. `#nav-toggle` is `position: fixed` so there is nothing to scroll to. +test("closing the mobile drawer leaves the reader where the scrim found them", async ({ page }) => { + await page.setViewportSize({ width: 390, height: 200 }); + await page.goto("/dashboard"); + + await page.locator(".hamburger").click(); + await expect(page.locator("#nav-toggle")).toBeChecked(); + // Scripted, because a key press with focus on the toggle does not scroll in every engine — and + // what is under test is closing the drawer, not how the reader got down the page. + await page.evaluate(() => window.scrollTo(0, 120)); + const at = await page.evaluate(() => window.scrollY); + expect(at, "the page must have somewhere to scroll behind the scrim").toBeGreaterThan(0); + + // The exposed strip beside the 264px panel: the scrim spans the viewport, so its centre is under + // the drawer and a centre click lands on the panel instead. + await page.locator(".scrim").click({ position: { x: 340, y: 100 } }); + await expect(page.locator("#nav-toggle")).not.toBeChecked(); + expect(await page.evaluate(() => window.scrollY), "closing the drawer must not move the page").toBe(at); +}); + test("captures the live pages for review", async ({ page }) => { await page.goto("/dashboard"); await expect(page.locator(".sidebar")).toBeVisible(); diff --git a/public/css/styles.css b/public/css/styles.css index b571293..341a100 100644 --- a/public/css/styles.css +++ b/public/css/styles.css @@ -556,7 +556,6 @@ span.nav-self { cursor: default; } /* static / non-clickable */ .pill-clear:hover { text-decoration: underline; } /* ---------- 9. TABLE --------------------------------------- */ -/* A bounded region is opt-in: give this a height and the header below stays put inside it. */ .table-wrap { overflow-x: auto; } table.table { width: 100%; border-collapse: separate; border-spacing: 0; @@ -694,7 +693,7 @@ th[aria-sort="descending"] .sort-ico { transform: rotate(180deg); } } /* the nav-toggle checkbox itself is visually hidden but focusable */ -#nav-toggle { position: absolute; opacity: 0; pointer-events: none; } +#nav-toggle { position: fixed; top: 0; left: 0; opacity: 0; pointer-events: none; } /* admin forms: create/edit user, account actions */ .form-page { padding: 16px; display: flex; flex-direction: column; gap: 14px; max-width: 560px; } diff --git a/src/ui/data-table.test.ts b/src/ui/data-table.test.ts index 18f43aa..839f530 100644 --- a/src/ui/data-table.test.ts +++ b/src/ui/data-table.test.ts @@ -42,6 +42,7 @@ test("data-table renders sortable headers, row-select, typed cells, badges and k const html = flat(await render(config)); assert.match(html, /
People in the directory<\/caption>/); + // Row-select: header select-all + per-row checkbox with a descriptive label. assert.match(html, /
<\/th>/); assert.match(html, /<\/td>/);