diff --git a/context/code/clients.md b/context/code/clients.md index fcd549b7..aed8eee8 100644 --- a/context/code/clients.md +++ b/context/code/clients.md @@ -2071,6 +2071,88 @@ the rest of the styling is **argued and not tested**, on the same terms as the rest of this section, and a change to it is reviewed by reading and by looking at the page. +### A reserve is a floor, and below the width that holds it the bar must reflow (improvise-tg27) + +The bug was reported as "a narrow window clips grid columns without usable +horizontal scroll", and the suspicion in the ticket was a flex `min-width:auto` +trap swallowing `#grid`'s overflow. It was not that. `#grid` carries +`overflow: auto`, which already zeroes a flex item's automatic minimum, so the +grid could always shrink. What could not shrink was the **app bar**. + +Measured at `22ab58f` on the grouped harness fixture: the `document` was +**1011px wide at every window narrower than 1011px** — a hard floor, not a +gradual overflow. The bar's reserved children (`#build-version` 14ch, +`#doc-name` 34ch, `#view-name` 16ch, `#mode` 21ch) are all `flex: none`, and +`#status` is `flex: 0 0 auto`; only `#hint` can shrink, and it bottoms out at +zero. Below that floor the *page* grew a horizontal scrollbar, and what it +scrolled to was the bar's own right-hand end — `#status`, `#mode` and `#hint` +sitting outside the window — while `main` stayed correctly 520px wide and the +grid slid off to the left as you scrolled. So the affordance a user reached for +led somewhere useless, which is the whole of what "not obviously scrollable" +meant. + +**Three things follow, and the first is the one to carry.** The reserve +(improvise-n6t8) is a *floor*, and a floor that exceeds the window is not a +reserve, it is an overflow. `header` therefore takes `flex-wrap: wrap`. That is +chosen over a `@media` breakpoint because **wrap is conditional on the thing +that actually matters**: an item wraps exactly when the line cannot hold it, so +the desktop bar is untouched by construction rather than by a number someone +picked. Measured on fresh loads of the harness fixture at 1024, 1100 and 1280 +the bar is still one line at 38px and `#grid` is unchanged. + +**The threshold is not a constant, and must not be asserted as one.** `#status` +composes the document's own name, so a longer name reflows a wider window: the +same 1024px window is one line under the name `shot` and two under +`narrow-tg27`. The spec therefore asserts no reflow at **1280** — where +improvise-n6t8's stability spec runs, which is the bar this change must not +disturb — and says nothing about where the first wrap falls. + +Second, **`#hint` is still squeezed, and the wrap does not fix it** — which is +worth writing down because the first draft of this section claimed it did. The +hint is the one line naming the keys that reach a column the window cannot +hold, and being the single elastic child (`flex: 1 1 0`) makes it the first +thing any shortfall takes. After the wrap its width is **non-monotonic in the +window**: measured settled, 26px at 360, **1px at 520**, 81 at 600, 317 at 700, +385 at 768, 67 at 900, 6 at 1024 (fixture `shot`). It is whatever the line break happens to +leave, because `#view-name` + `#mode` + `#status` can fill a wrapped line +exactly. So no assertion is made about it, and the phone-width case is a real +gap this ticket does not close. Making it legible means giving it a +`min-width` so it *forces* its own line, which is a direct change to +improvise-n6t8's "basis ZERO so it contributes nothing to overflow" and belongs +in a ticket that argues it. + +The measurement that caught the false claim is worth keeping too: an early +version of the spec read the hint immediately after the grid painted and got +186px, because `#status` had not yet composed the transport's fact and was +narrower. `theGrouped` now waits for `#status` before measuring — a bar that is +still settling is a transient, and a test that measures one is flaky in the +direction that reads as a pass. + +Third, **a fixed width is a fraction of a narrow window.** `#categories` at +15rem is 240px of a 360px phone, leaving the grid 120px. `max-width: 40vw` on +the three panels keeps the grid the larger share. It buys nothing in projected +*columns* — `COL_PX` is 110 and the extent is `floor(width/110) - frameCost`, +so one column is what fits either way — and a great deal in legibility. + +**What is not the bug, and must not be "fixed".** At 360px the grid shows one +data column and the other five are **never projected**: the browser's `Viewport` +is a real window (gotcha 44/47), so no amount of scrolling can reveal a column +the host was never asked for. The ticket's own "or another explicit navigation" +is the answer, and it already worked — `ArrowRight` walks all six, because +`resolve.rs` composes `SetSelected` plus the offsets `viewport_effects` returns. +Over-announcing the extent so `#grid` has something to scroll was considered and +declined: a fixed over-fetch reveals a fixed number of extra columns and so +solves nothing general, while announcing the whole grid defeats the windowed +viewport that exists for large documents. It would also be a change to the +`ResizeObserver` arithmetic, which nothing tests. + +`harness/tests/narrow.spec.js` is the instrument, and its assertion is +`documentElement.scrollWidth <= clientWidth` — a question a browser answers +exactly. Note what was **not** done: `body { overflow-x: hidden }` would have +made every window pass that assertion by making `scrollWidth` equal +`clientWidth` unconditionally, which is a check answering its neighbour's +question. The bar has to genuinely fit. + --- ## `web/worker.js` — the worker half diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index 63b813fb..b109d916 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -3812,3 +3812,117 @@ a walk that silently visited nothing would otherwise pass forever, which is the vacuous-assertion shape design-principles §7 names. The same test is what makes the refusal arm safe to leave unreachable instead of inventing a second frame for it. + +## A narrow window does not widen the document (improvise-tg27) + +`harness/tests/narrow.spec.js`. Reproduced at `22ab58f` before anything was +changed, and the ticket's own diagnosis was wrong in a way worth recording: it +suspected a `min-width: auto` flex trap swallowing `#grid`'s overflow, and +`#grid` has `overflow: auto`, which already zeroes that minimum. The reasoning +is in `context/code/clients.md`. + +The measurement that found it: at 360, 420, 520, 600, 700, 768, 820, 900, 950 +and 1000 px, `documentElement.scrollWidth` read **1011 at every one of them** — +a floor rather than a slope, which is the signature of a sum of `flex: none` +reserves rather than of a clipped child. `main` was correctly window-width +throughout, so nothing about the grid's own box was wrong. + +### "a narrow window does not scroll the document horizontally" + +One test over three widths — 360, 520 and 768 — because the property is one +property and a per-width test name would have been three names for it. Three +assertions per width, and the first is a precondition rather than the subject: +`dataCols < 6` states that `grouped.improv` really is clipped there. Without it +every assertion below would hold on a window wide enough to show the whole +grid, which is the "passes on a document with no overflow" shape. + +`escaping` is reported as a list of ids rather than a count, so a failure names +which children of `
` sat outside the window — at 520 that was +`view-name`, `mode`, `hint` and `status`. Each assertion's message carries the +width, since a loop inside one test otherwise reports only that something +failed. + +**Fault injected:** `web/style.css` restored to `22ab58f` and the site +reassembled. Fails at all three narrow widths with *"the document scrolls +horizontally"*, `1011` against the window width. + +### "the grid keeps the larger share of a phone-width window" + +`#categories` is `width: 15rem` and `flex: none` — 240px of a 360px phone, +leaving the grid 120. The assertion is an **ordering**, grid wider than panel, +and not a ratio: the exact split is a stylesheet number and pinning it would +fail on every future tweak while catching nothing. + +**Fault injected:** `web/style.css` restored to `22ab58f` and the site +reassembled. Fails with *"the panel takes more of the window than the grid"*, +120 against 240. 360 and not 520: unfixed, 520 already passes this ordering +(280 against 240), so a cap that bites on a phone needs a phone to see it. + +### What this ticket does NOT fix, and a claim that was wrong for an hour + +`#hint` — the one line naming the keys that reach a clipped column — is still +squeezed. An earlier version of this spec asserted it became legible, and that +assertion **passed**, which is the part worth recording: it read the hint +immediately after the grid painted, when `#status` had not yet composed the +transport's fact and was narrower, and measured 186px. Settled, the same width +is **1px**. + +Measured settled after the fix, `#hint` is 26px at 360, 1 at 520, 81 at 600, +317 at 700, 385 at 768, 67 at 900 and 6 at 1024 — non-monotonic, because it is +whatever the wrap happens to leave once `#view-name`, `#mode` and `#status` +have taken their reserves. No assertion is made about it. `theGrouped` now +waits for `#status` before measuring anything, because a bar that is still +settling is a transient and a spec that measures one fails in the direction +that reads as a pass. + +### "at 1280px the bar holds its reserve on one line" + +The other direction, and the reason `flex-wrap` was chosen over a `@media` +breakpoint. improvise-n6t8's stability spec runs at 1280, so this is what says +the bar that spec measures is untouched: one line, 38px, nothing wrapped. + +It passes **before and after**, which is the point: a no-change assertion, not +a bug reproduction. `docScrollWidth <= clientWidth` holds here on the unfixed +tree too, since 1011 is the floor. + +**A first version also asserted it at 1024 and failed**, which is the finding +worth keeping. `#status` composes the document's *name*, so the width at which +the reserve stops fitting depends on the document: at 1024 the fixture `shot` +is one line and `narrow-tg27` is two. The reflow threshold is therefore not a +constant and is deliberately not asserted anywhere — only that 1280 is above +it for a document with a harness-length name. + +### "the measurement sees an overflow that is really there" + +The check-the-checker. Every assertion above is a negative — *nothing* +overflows — and a negative is vacuous unless the measurement can see a positive. +So one is injected at the same width through the same path: `#doc-name`, which +held the largest of the original reserves, is given `flex: none; width: 1400px; +max-width: none` — the exact shape the fixed reserves had. + +Both halves are asserted, because they can fail apart: `docScrollWidth` must +exceed the window (the page-level check works) *and* `escaping` must name +`doc-name` (the per-child check works). A version asserting only the first +would pass against a `escaping` computation that always returned `[]`. + +**Fault injected:** the injection removed. The test then fails on its own +`toBeGreaterThan`, which is what says the assertion is not self-satisfying. + +### "the keyboard reaches a column the window cannot hold" + +Why the clipping is acceptable at all, and the assertion that separates *this* +bug from the one the ticket might be read as describing. At 520px one data +column is projected and the other five are never sent, so scrolling `#grid` +cannot reveal them — the browser's `Viewport` is a real window (gotcha 44/47). +The ticket's "or another explicit navigation" is the keyboard, and it already +worked before this change. + +The walk checks **every intermediate step**, alternating `Web`/`Shop`, not only +the final column: a client that jumped straight to the end would satisfy a +last-column assertion alone. It then reads all three header levels, because +`Shop` at the innermost level does not distinguish `Mar/Shop` from `Jan/Shop`. + +This test passes before and after. It is here because a future "fix" that made +`#grid` scroll by over-announcing the extent would have to keep it passing, and +because without it the narrow-width specs read as if clipping itself were the +defect. diff --git a/harness/tests/narrow.spec.js b/harness/tests/narrow.spec.js new file mode 100644 index 00000000..c8079760 --- /dev/null +++ b/harness/tests/narrow.spec.js @@ -0,0 +1,167 @@ +// A narrow window must not widen the DOCUMENT (improvise-tg27). The app bar's +// reserve is a floor, and below the width that holds it the page grew a +// horizontal scrollbar that led to the bar's own tail rather than to grid +// columns. See context/code/clients.md and context/test-evidence/clients.md. + +import { test, expect } from "@playwright/test"; + +/// `harness/fixtures/grouped.improv`: two categories on each axis, so the grid +/// is six data columns behind two row-header levels. Wide enough that a +/// phone-width window genuinely clips it -- the precondition every assertion +/// here rests on, and asserted rather than assumed. +async function theGrouped(page, width, height = 700) { + await page.setViewportSize({ width, height }); + await page.goto("/app/index.html?doc=narrow-tg27&seed=grouped.improv"); + await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 }); + await expect(page.locator("#view-name")).toHaveText("Grouped"); + // The bar is not SETTLED when the grid first paints: `#status` composes the + // transport's fact and the document's, and it is the widest thing on a + // wrapped line. Measuring before it arrives measures a transient. + await expect(page.locator("#status")).toContainText("in-page worker"); +} + +/// What the page's own scrollers say. `documentElement` and not `#grid`: the +/// grid is ALLOWED to scroll -- it is the one scroller this app has -- and the +/// document is not. +const pageMetrics = (page) => + page.evaluate(() => { + const d = document.documentElement; + const box = (n) => { + const r = n.getBoundingClientRect(); + return { left: Math.round(r.left), right: Math.round(r.right), width: Math.round(r.width) }; + }; + const header = document.querySelector("header"); + return { + win: innerWidth, + docScrollWidth: d.scrollWidth, + docClientWidth: d.clientWidth, + headerHeight: Math.round(header.getBoundingClientRect().height), + escaping: [...header.children] + .filter((n) => box(n).right > innerWidth + 1) + .map((n) => n.id || n.tagName), + grid: box(document.getElementById("grid")), + categories: box(document.getElementById("categories")), + dataCols: + (document.querySelector("#grid tbody tr:not(:has(th.group))")?.children.length ?? 0) - 2, + }; + }); + +/// 360 is a phone in portrait, 520 the width the bug was reported at, 768 a +/// tablet. Each is below the ~1011px the bar's reserve needed, which is what +/// pinned the document at 1011px wide at all three. +const NARROW = [360, 520, 768]; + +test.describe("a narrow window does not widen the document (improvise-tg27)", () => { + test("a narrow window does not scroll the document horizontally", async ({ page }) => { + for (const width of NARROW) { + await theGrouped(page, width); + const m = await pageMetrics(page); + + // The precondition. A window wide enough for the whole grid would + // satisfy everything below while proving nothing, so the spec states + // that the fixture really is clipped here -- six data columns exist. + expect(m.dataCols, `the fixture is not clipped at ${width}px`).toBeLessThan(6); + + // The bug, stated once: the document was 1011px wide at every one of + // these widths, so the page -- not the grid -- carried the scrollbar. + expect(m.docScrollWidth, `the document scrolls horizontally at ${width}px`).toBeLessThanOrEqual( + m.docClientWidth, + ); + + // And what that scrollbar led to: the bar's own right-hand end, outside + // the window. Reported as ids so a failure says which children. + expect(m.escaping, `bar children outside the window at ${width}px`).toEqual([]); + } + }); + + test("the grid keeps the larger share of a phone-width window", async ({ page }) => { + await theGrouped(page, 360); + const m = await pageMetrics(page); + + // `#categories` is 15rem and `flex: none` -- 240px of a 360px phone, + // leaving the grid 120. The assertion is an ORDERING and not a ratio: the + // exact split is a stylesheet number and pinning it catches nothing. + expect(m.grid.width, "the panel takes more of the window than the grid").toBeGreaterThan( + m.categories.width, + ); + }); + + // The reflow is for windows that cannot hold the reserve, and for no others. + // 1280 is where improvise-n6t8's stability spec runs, so this is what says + // the bar that spec measures is untouched. The threshold itself is NOT + // asserted and is not a constant: `#status` composes the document's name, so + // a longer name reflows a wider window. clients.md. + test("at 1280px the bar holds its reserve on one line", async ({ page }) => { + await theGrouped(page, 1280, 800); + const wide = await pageMetrics(page); + expect(wide.headerHeight, "the bar wrapped at a width that holds it").toBeLessThan(50); + expect(wide.docScrollWidth).toBeLessThanOrEqual(wide.docClientWidth); + }); + + // The check-the-checker. Every assertion above is a negative -- "nothing + // overflows" -- and a negative is vacuous unless the measurement can SEE an + // overflow. So one is injected, at the same width, through the same path. + test("the measurement sees an overflow that is really there", async ({ page }) => { + await theGrouped(page, 520); + expect((await pageMetrics(page)).docScrollWidth).toBeLessThanOrEqual(520); + + // A bar child that cannot shrink and cannot wrap: exactly the shape the + // fixed reserves had before `flex-wrap`. Injected on #doc-name because it + // held the largest of them (34ch). + await page.evaluate(() => { + const n = document.getElementById("doc-name"); + n.style.flex = "none"; + n.style.width = "1400px"; + n.style.maxWidth = "none"; + }); + + const hurt = await pageMetrics(page); + expect( + hurt.docScrollWidth, + "an injected 1400px reserve did not widen the document", + ).toBeGreaterThan(hurt.docClientWidth); + expect(hurt.escaping, "an injected overflow was not seen escaping the bar").toContain( + "doc-name", + ); + }); + + // Why clipping is acceptable at all: the browser's viewport is a REAL window + // (gotcha 44/47), so at 520px one data column is what fits and the rest are + // never projected -- no scrolling reveals them. The ticket's "or another + // explicit navigation" is the keyboard, and this asserts it reaches all six. + test("the keyboard reaches a column the window cannot hold", async ({ page }) => { + await theGrouped(page, 520); + await page.locator("#grid").focus(); + + const innermost = () => + page.evaluate(() => { + const rows = document.querySelectorAll("#grid thead tr"); + return [...rows[rows.length - 1].children].map((c) => c.textContent).filter(Boolean); + }); + + expect(await innermost()).toEqual(["Web"]); + + // Five presses walk Jan/Web to Mar/Shop, the last of the six. Every + // intermediate step is checked: a client that jumped to the end would + // satisfy a last-column assertion alone. + const walk = []; + for (let i = 0; i < 5; i++) { + await page.keyboard.press("ArrowRight"); + await expect.poll(async () => (await innermost())[0]).toBe(i % 2 === 0 ? "Shop" : "Web"); + walk.push((await innermost())[0]); + } + expect(walk).toEqual(["Shop", "Web", "Shop", "Web", "Shop"]); + + // The last column really is the last: the outer levels say Q2/Mar, which + // "Shop" alone cannot distinguish from Jan/Shop. + const levels = await page.evaluate(() => + [...document.querySelectorAll("#grid thead tr")].map((r) => + [...r.children] + .map((c) => c.textContent) + .filter(Boolean) + .join(""), + ), + ); + expect(levels).toEqual(["▼ Q2", "Mar", "Shop"]); + }); +}); diff --git a/web/style.css b/web/style.css index 662aeee9..88ed64cc 100644 --- a/web/style.css +++ b/web/style.css @@ -77,6 +77,9 @@ body { header { display: flex; + /* The reserve is a FLOOR, so below the width that holds it the bar must + reflow rather than widen the document (improvise-tg27). clients.md. */ + flex-wrap: wrap; gap: 0.75rem; align-items: center; flex: none; @@ -115,6 +118,8 @@ header h1 { #doc-name { flex: none; width: 34ch; + /* A reserve wider than the window is not a reserve, it is an overflow. */ + max-width: 100%; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; @@ -195,7 +200,10 @@ header h1 { backstop for an unusually long peer address — app.js puts the whole text on `title`, so an ellipsis loses nothing. */ flex: 0 0 auto; - max-width: 52ch; + /* 52ch is the backstop for a long peer address; `100%` is the one that + matters on a phone, where 52ch is wider than the window and the bar + would overflow its own line rather than wrap (improvise-tg27). */ + max-width: min(52ch, 100%); white-space: nowrap; overflow: hidden; text-overflow: ellipsis; @@ -384,14 +392,21 @@ header h1 { /* ── The frame: panels and grid ──────────────────────────────────────── */ +/* `min-width` as well as `min-height`: without it a flex child's automatic + minimum is its content, and `main` would be sized by the widest panel plus + the grid rather than by the window (improvise-tg27). */ main { flex: 1; display: flex; min-height: 0; + min-width: 0; } +/* The panels are fixed-width beside the grid, and a fixed width is a fraction + of a narrow window: capped, the grid keeps the larger share. clients.md. */ #categories { width: 15rem; + max-width: 40vw; flex: none; overflow: auto; padding: 0.5rem 0.25rem; @@ -402,6 +417,7 @@ main { #formulas, #views { width: 18rem; + max-width: 40vw; flex: none; overflow: auto; padding: 0.5rem 0.25rem;