Fix what the branch review found, starting with a gate that was red
The whole-branch review turned up a blocker the CI gate could not see,
which is the point of it being a separate gate.
fixtures/browser-drive.mjs is not in .amadan/ci.d/10-gate — Chromium
cannot start inside the agent sandbox, so it is run by hand — and it
is the only thing in this repo that checks the keyboard path. It was
red on this branch and had not been run. Two separate faults:
- Check 7 anchored itself on 'form[action="/signout"] button'. That
button is now inside the shell's account dropdown, a CLOSED
<details>: the node is in the DOM, so querySelector finds it and
the guard passes, but it is not rendered, so .focus() does
nothing. The loop then counted Tab presses from wherever focus
already was — the editor, from check 6 — and reported 8. A true
measurement of nothing, which is the exact shape AGENTS.md warns
about twice. It now anchors on [rst-shell-brand], which is
rendered at every width, and asserts that the anchor took before
counting anything.
- Check 8's bound of 3 was derived when the path was sign-out ->
bar -> editor. The exports moved into the bar's trailing end and
the account control moved into the shell, so the real path is 5:
account, the toolbar entire as one roving stop, PDF, Markdown,
editor. Re-derived rather than nudged, with the derivation in the
comment; if the toolbar ever stops being one stop it becomes 18.
23/23 checks pass now, and were seen to fail first.
The rest were the review's should-fixes, all of them comments that
claimed more than the code did:
- [docs-shell-title] said "truncation over wrapping" and did the
opposite. Flex line-breaking uses an item's hypothetical main
size, so flex: 0 1 auto on a nowrap span drops the title to a
second row of the bar and the account to a third, and only
ellipsises once the title alone exceeds the bar. flex: 1 1 0
never forces a break. Verified at 560px with a 97-character
title: one row, ellipsis.
- [docs-bar]'s note on the auto margin was backwards in both
halves. Auto margins take free space per line, so on a wrapped
line the exports stay at the trailing end; space-between would
put a lone item at flex-start. And space-between could not
separate the two links from each other in any case — they are one
flex item.
- [rst-shell-brand] is a 21px target. [docs-back], which it
replaces, had already found this and stated the 24px floor
outright, with a measured note on why deriving it from tokens
fails. Deleting [docs-back] deleted that; the rule is back, in
head.html, where the app's one shared <head> definition already
lives.
- The narrow-screen character counts divided by two different
numbers in the same paragraph (12.8px/char and 8px/char) while
saying "measured". One divisor now, stated.
- signin.html's reason for hiding the bar named the account menu,
which tokens.css already hides when its menu is empty. What that
page actually has to hide is the default brand and the
narrow-width hamburger.
And the references the move left pointing at a header that no longer
exists: AGENTS.md, render.go's signout and export catalog comments,
handlers.go's SignOutLabel and BackLabel, export/pdf.go,
toolbar-roving.mjs, and list_test.go's claim that an undefined shell
block loses the route (it keeps the href and loses only the label, so
the label comparison is the assertion doing the work).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GvAMFZXaJSaXRqGVn11WXj
AGENTS.md+2 −2fixtures/browser-drive.mjs+50 −12fixtures/toolbar-roving.mjs+1 −1internal/docs/handlers.go+8 −6internal/docs/list_test.go+11 −4internal/docs/render.go+16 −9internal/docs/views/head.html+25 −1internal/docs/views/show.html+51 −19internal/docs/views/signin.html+10 −6internal/export/pdf.go+4 −2
CI passed — run details
Intent
merge and deploy?
<task-notification> <task-id>a3af3ffd64deaeb3f</task-id> <tool-use-id>toolu_01ExXDofJng15fX3dQWjoNEM</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-docs/7925695a-b185-431d-9d20-42b4dfbeb800/tasks/a3af3ffd64deaeb3f.output</output-file> <status>completed</status> <summary>Agent "Fable whole-branch review" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>Review complete. Sanity check: `go build ./...` and `go test ./internal/docs/` both pass on the branch (122s); I re-read the ask (review only, no edits) and modified nothing. What follows is the report to relay. ## Whole-branch review: `worktree-tighten-chrome` (20515c2) vs `origin/main` ### BLOCKER **1. The repo's own keyboard-path gate is red on this branch, and was not run.** `fixtures/browser-drive.mjs:534` starts checks 7 and 8 by focusing `form[action="/signout"] button`. This branch moves that button inside the shell's closed `<details rst-dropdown rst-shell-account>` (`topbar.html`, `show.html:63`). Content of a closed `<details>` is not rendered, so `.focus()` is a no-op in Chromium and Firefox; the drive then starts from wherever focus already was — the editor, per line 503. Worse, even if the start were fixed to the account summary, the DOM path is now summary → toolbar (1 stop) → PDF → Markdown → editor = **4 presses**, and check 8 (`browser-drive.mjs:555`) asserts `reachedAfter <= 3`. So the only mechanism this repo has for the `role="toolbar"` contract (AGENTS.md's own "invisible in a unit test" case) fails on this branch under every starting point. CI stays green because the drive is not in `10-gate` — precisely the "passed" vs "could not see" trap AGENTS.md records. The commit message claims Chromium measurements but never mentions the drive. Fix: retarget the start (e.g. `[rst-shell-account] > summary`), raise the bound with a re-derived comment (line 512-514's "two presses suffice" is now false), run `scripts/browser-drive.sh` with the sandbox off, and say so in the discussion. ### SHOULD-FIX **2. `show.html:106-118` — `[docs-shell-title]`'s "truncation over wrapping" is not what the CSS does.** `[rst-shell-bar]` is `flex-wrap: wrap` (tokens.css:1346). Flex line-breaking uses each item's hypothetical main size — for `flex-basis: auto` on nowrap text, its full width — and `min-inline-size: 0` / shrink only act after the line is chosen. So a title that does not fit beside the brand drops to its own row, the account/menu control to the row after, and ellipsis appears only once the title alone exceeds the bar. Estimate at 320px (14px/650 brand "Documents" ≈ 68px, Menu summary ≈ 66px, three 16px gaps, 32px bar padding): any title over roughly 19 characters at 12.5px gives a two-row bar, over ~30 a three-row one. Estimated from tokens, not measured — measure it. Fix is `flex: 1 1 0` (basis 0 never forces a wrap; the item grows into free space and shrinks with ellipsis). Secondary: when it does truncate, the full title is readable nowhere on the page (not focusable, no tooltip, no other rendering) — an SC 1.4.10 loss-of-information question the old wrapping `<h1>` did not have. **3. `show.html:523-529, 559-565` — the auto-margin reasoning is inverted.** Flexbox hands each line's positive free space to auto margins before `justify-content` (css-flexbox-1 §9.5 step 16, per line). On its own wrapped line, `[docs-doc-actions]` with `margin-inline-start: auto` sits at the trailing end (flush *right*), not "flush left under the toolbar"; and `space-between` with a single item on a line is `flex-start` — flush left — and cannot "strand them apart", because both links are inside one flex item. The rendered result is fine; the recorded reason is backwards in a codebase that holds comments to be measured. **4. `show.html:54` — the brand link is a 21px-tall target, and the branch deleted the rule that had fixed exactly that.** The old `[docs-back]` set `min-block-size: 24px` with a measured comment on why derived sizes fail. Its replacement `[rst-shell-brand]` gets no padding (tokens.css:1347); body is 14px × 1.5 → 21px. It passes SC 2.5.8 only via the spacing exception (≥16px gap horizontally; 8px+1px+8px vertically to the toolbar), not via size, and AGENTS.md says "24×24 targets". Removed without mention. Add a min-block-size for the brand on the signed-in pages, or propose it upstream per spec §10. **5. `show.html:162-176` — arithmetic verified, character counts not.** 151/96/352 check out: 16px root, `--rst-sp-4` = 16px, 1in = 96px, and the `[rst-page]` override wins by source order at equal (0,1,0) specificity. But "352px ≈ 44 characters" uses 8px/char while "96px ≈ seven characters" uses 13.7px/char; 12pt Georgia averages 8–9px/char, so 96px is 11–12 characters. "Seven" is scaled from the old "five at 64px" (itself inconsistent with "40 at 320px" pre-branch) and the paragraph still says "measured". The 256→288 toolbar claim is correct against the old and new padding; I could not re-measure "both overflow" (no browser in the sandbox). **6. `render.go:39-43`, `signin.html:30-45` — the "must hide the account in CSS" reasoning is partly wrong.** tokens.css:1354 `[rst-shell-account]:has(> [rst-dropdown-menu]:empty) { display: none }` already hides the account menu on any page that defines no account block, which signin does not. What signin actually has to hide is the default "Home" brand and, under 800px, the Menu disclosure (which would open an empty tail). The rule and its specificity note are correct; the stated reason is not. **7. Stale references the move left behind:** AGENTS.md:30 ("linked from both list.html and show.html's page headers"), render.go:174 (docs.signout.label "page headers"), render.go's `docs.export.*` comment ("show.html's page header"), handlers.go:68 ("list.html's header also carries"), handlers.go:273 ("[docs-back]"), internal/export/pdf.go:277, fixtures/toolbar-roving.mjs:15. ### NITS **8. `list_test.go:214-218` and the commit message:** "the route is exactly as easy to lose ... falls back to the layout's default" — the default *is* `<a rst-shell-brand href="/">` (topbar.html), so an undefined block keeps the route and loses only the label. The test's first assertion therefore passes against the default too; only the label comparison is load-bearing. Fine as a test, wrong as a comment. **9. No `<h1>`:** not a WCAG 2.2 AA failure — no SC requires one, the span is not visually a heading so 1.3.1 does not bite, `<title>` carries it, the page is behind `RequireSession` so SEO is moot, and the old page already mixed the header h1 with the document's own h1s from `Store.HTML`. One sentence in the comment, no change. **10. `list.html:55`:** brand links to `/` from `/` — a self-link, standard pattern, and useful from `/?q=`. No action. ### Verified and fine CSS scoping "by delivery" is genuinely true (`render.go` clones base before `ParseFS` of each page; partials carry no `[rst-page]` rule). Nothing else on show depended on `[rst-page]`'s padding or max-width: `[docs-save-status]` and `[docs-jumpto]` are `position: fixed`, `[docs-bar]`'s sticky is unaffected. The roving widget (root `[docs-toolbar]`, `editor-entry.js:238`) owns exactly the 14 formatting controls; the exports are outside it. Account dropdown is a native `<details>` and sign-out is a form POST — scriptless. Skip link is a sibling of the hidden bar (`topbar.html`) and still targets `#main`. No literal English entered a template; the shell's own strings are rastrillo keys present in v0.22.0 `locales/en.toml`. Attribute grammar and `docs-*` prefixes respected. **VERDICT: CHANGES REQUIRED** result: Whole-branch review of tighten-chrome (20515c2) — CHANGES REQUIRED: the branch breaks fixtures/browser-drive.mjs's keyboard-path checks (sign-out start target now inside a closed details; editor is 4 tab presses from the bar against a ≤3 bound) and was not driven; plus six should-fix items (title truncation claim false under flex-wrap, inverted auto-margin comment, 21px brand target, inconsistent character counts, wrong account-hiding rationale, stale header references).</result> <usage><subagent_tokens>151641</subagent_tokens><tool_uses>33</tool_uses><duration_ms>631011</duration_ms></usage> </task-notification>
Attributed to this commit: 244 g CO₂e (27071750 tokens). This is an attribution, not a measurement — usage counted from this commit's own marker forward to the next one on worktree-tighten-chrome, never backward from it. A turn that produced more than one commit books all of its tokens to the last one, and a concurrent session on this branch can add to the same figure. Some of it was priced using the shared default rate, because no model-specific factor row exists yet for at least one model involved. The Carbon view has the repo total, which is never derived from this attribution.
diff --git a/AGENTS.md b/AGENTS.md| index bf1bf8b..b2580de 100644 |
| --- a/AGENTS.md |
| +++ b/AGENTS.md |
| @@ -26,8 +26,8 @@ Every document route — `GET /`, `POST /documents`, `GET /d/{sqid}` — sits |
| behind rastrillo/auth's `RequireSession` and is unreachable without a real |
| session (`internal/docs/auth.go`). Sign-in is a magic link: `POST /signin` |
| emails (or, in development, logs) a single-use link; following it |
| -(`GET /auth/verify`) sets a real session cookie; `POST /signout` (linked |
| -from both `list.html` and `show.html`'s page headers) really revokes it, |
| +(`GET /auth/verify`) sets a real session cookie; `POST /signout` (in the shell |
| +bar's account menu on both signed-in screens) really revokes it, |
| not just clearing the cookie client-side. No passkey second factor is |
| wired, on purpose — spec §8.1's ruling: `passkey.Config.Origin` derives |
| its relying party from this instance's own host, so a person in several |
diff --git a/fixtures/browser-drive.mjs b/fixtures/browser-drive.mjs| index 4ea64d2..f9d1cf3 100644 |
| --- a/fixtures/browser-drive.mjs |
| +++ b/fixtures/browser-drive.mjs |
| @@ -525,13 +525,34 @@ async function caretChecks() { |
| // see it".) |
| // |
| // Bounded rather than exact: the bar is one tab stop today |
| - // (toolbar.mjs's roving tabindex) so two presses suffice, but |
| - // asserting "two" would break the week someone adds a control |
| - // outside the bar for a reason that has nothing to do with this. |
| - // Twelve is generous and still fails loudly the day focus cannot |
| - // get out of the toolbar at all. |
| - await evaluate( |
| - `(()=>{const b=document.querySelector('form[action="/signout"] button');if(b)b.focus();return !!b})()`, |
| + // (toolbar.mjs's roving tabindex), but asserting an exact number |
| + // would break the week someone adds a control outside the bar for |
| + // a reason that has nothing to do with this. Twelve is generous |
| + // and still fails loudly the day focus cannot get out of the |
| + // toolbar at all. |
| + // |
| + // Starts from the shell bar's brand link, and that is not a free |
| + // choice. It used to start from the sign-out button, which was in |
| + // this page's own header; sign-out is now inside the shell's |
| + // account dropdown, a CLOSED <details>, whose content is in the |
| + // DOM but is not rendered — so querySelector still finds the |
| + // button, .focus() is a silent no-op, and the loop below counted |
| + // Tab presses from wherever focus happened to be already (the |
| + // editor, from check 6 above). It reported 8, which was a true |
| + // measurement of nothing. That is the failure shape AGENTS.md |
| + // names twice, so: the anchor must be something always rendered. |
| + // [rst-shell-brand] is — it is the first focusable thing in the |
| + // bar at every width — and the assertion below fails if it ever |
| + // stops being focusable, rather than quietly measuring from |
| + // somewhere else. |
| + const anchored = await evaluate( |
| + `(()=>{const b=document.querySelector('[rst-shell-brand]');if(!b)return false;b.focus();` + |
| + `return document.activeElement===b})()`, |
| + ) |
| + check( |
| + 'the tab-order check starts where it thinks it does', |
| + anchored === true, |
| + 'could not focus [rst-shell-brand] — the count below would measure from somewhere else', |
| ) |
| await sleep(200) |
| let reachedAfter = -1 |
| @@ -545,19 +566,36 @@ async function caretChecks() { |
| } |
| } |
| check( |
| - 'the document is reachable by Tab from the page header', |
| + 'the document is reachable by Tab from the shell bar', |
| reachedAfter > 0, |
| reachedAfter > 0 ? '' : 'twelve presses of Tab never reached the editor', |
| ) |
| // 8. And the bar is ONE stop on the way, not one per control. |
| // role="toolbar" is a promise that the arrow keys move within it; |
| - // a bar that spends eleven tab presses has made that promise and |
| - // broken it. |
| + // a bar that spends fourteen tab presses has made that promise |
| + // and broken it. |
| + // |
| + // Five, derived rather than chosen, at this drive's 1280px (above |
| + // the shell's own 800px breakpoint, so the bar is laid out wide: |
| + // the hamburger is hidden and the tail is display: contents). |
| + // From [rst-shell-brand], in DOM order: the account summary (1), |
| + // the formatting bar, entire, as one roving-tabindex stop (2), |
| + // the PDF export (3), the Markdown export (4), the editor (5). |
| + // |
| + // It was three when the path was sign-out -> bar -> editor and |
| + // this page drew its own header. The two exports moved into the |
| + // bar's trailing end and the account control moved into the shell, |
| + // which is +2 for reasons that have nothing to do with the roving |
| + // tabindex — exactly the drift the original comment predicted and |
| + // left slack for. The slack is spent now, so the number is |
| + // re-derived above rather than nudged: if the toolbar ever stops |
| + // being one stop this becomes 18 and fails, which is the whole |
| + // point of the check. |
| check( |
| 'the formatting bar is a single tab stop', |
| - reachedAfter > 0 && reachedAfter <= 3, |
| - `the editor was ${reachedAfter} tab presses from the header`, |
| + reachedAfter > 0 && reachedAfter <= 5, |
| + `the editor was ${reachedAfter} tab presses from the shell bar`, |
| ) |
| // 9. ...and the arrow keys are what move inside it. |
diff --git a/fixtures/toolbar-roving.mjs b/fixtures/toolbar-roving.mjs| index 3f0bf97..7bd5857 100644 |
| --- a/fixtures/toolbar-roving.mjs |
| +++ b/fixtures/toolbar-roving.mjs |
| @@ -12,7 +12,7 @@ |
| // pattern, which `role="toolbar"` in views/show.html is a promise to |
| // implement. |
| // |
| -// Without it the bar is eleven tab stops between the page header and |
| +// Without it the bar is fourteen tab stops between the shell bar and |
| // the first word of the document, on every page load, for someone who |
| // only wanted to type. It is also what a word processor does, so it is |
| // what people's hands already expect. (Raised by the caret-placement |
diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go| index 4a2a995..f626cad 100644 |
| --- a/internal/docs/handlers.go |
| +++ b/internal/docs/handlers.go |
| @@ -65,8 +65,8 @@ type handlers struct { |
| // everyone else (fix round 1: the original version of this screen |
| // had no path to POST /documents at all outside a hand-typed curl). |
| // |
| -// SignOutLabel backs the sign-out control list.html's header also |
| -// carries — added in the fix round that deleted the development-only |
| +// SignOutLabel backs the sign-out control in the shell bar's account |
| +// menu, which this screen fills with an "account" block — added in the fix round that deleted the development-only |
| // stand-in identity: POST /signout (auth.go) always existed and |
| // worked, but nothing on any page linked to it, which matters once |
| // this is reachable at a real URL rather than only ever behind a |
| @@ -269,10 +269,12 @@ type showPage struct { |
| HTML template.HTML |
| SignOutLabel string // see listPage.SignOutLabel's own doc comment |
| - // BackLabel is the document page's one route back to the list |
| - // (views/show.html's [docs-back]). Until this landed, GET |
| - // /d/{sqid} offered no in-app navigation at all — its header held |
| - // sign-out and nothing else — so a person who arrived by a |
| + // BackLabel is the document page's one route back to the list — |
| + // the label on the shell bar's brand link, which views/show.html |
| + // fills with a "brand" block (it was [docs-back], a breadcrumb in |
| + // this page's own header, until the chrome moved to the topbar |
| + // shell). Until it landed, GET /d/{sqid} offered no in-app |
| + // navigation at all — its header held sign-out and nothing else — so a person who arrived by a |
| // bookmark or a mailed link, rather than by clicking through from |
| // GET /, had no way back to their own documents except the |
| // browser's back button, which that arrival gives them nothing to |
diff --git a/internal/docs/list_test.go b/internal/docs/list_test.go| index ea06a77..61e25ea 100644 |
| --- a/internal/docs/list_test.go |
| +++ b/internal/docs/list_test.go |
| @@ -210,10 +210,17 @@ func TestUnknownDocument404s(t *testing.T) { |
| // topbar shell's brand slot, which show.html fills with the same href |
| // and the same label (render.go's shellLayout comment has the why). |
| // This test moved with it rather than being deleted, because what it |
| -// holds is the route, not the element that happened to carry it — and |
| -// the route is exactly as easy to lose in the new home as in the old |
| -// one, since a shell block a page never defines silently falls back to |
| -// the layout's default. |
| +// holds is the route, not the element that happened to carry it. |
| +// |
| +// Note what that means for each half below, because they are not |
| +// equally load-bearing any more. The layout's own default brand is |
| +// already <a rst-shell-brand href="/">, so a page that stopped |
| +// defining "brand" would keep the href and lose only the label: the |
| +// first assertion would still pass and the second is what would |
| +// catch it. The first is not useless — it catches the shell being |
| +// swapped for one with no brand at all, which is exactly the kind of |
| +// whole-app change this branch itself was — but the label comparison |
| +// is the one doing the work. |
| // |
| // Asserted on the rendered page rather than the template's bytes: the |
| // href has to survive the handler populating it, not merely be |
diff --git a/internal/docs/render.go b/internal/docs/render.go| index b8db7e5..e412baf 100644 |
| --- a/internal/docs/render.go |
| +++ b/internal/docs/render.go |
| @@ -38,10 +38,15 @@ import ( |
| // |
| // Two things to know before changing this again. It is a whole-app |
| // choice — newRenderer parses exactly one layout, so there is no |
| -// per-screen opt-out — and the shell renders [rst-shell-account] |
| -// itself, outside any block an app can override, so a screen that |
| -// must not offer an account menu hides it in its own stylesheet. |
| -// signin.html is the one that does, and its comment says why. |
| +// per-screen opt-out. And a page's only lever on the bar is the |
| +// blocks the layout exposes (brand, nav, account, locale, foot): |
| +// the bar element itself, the account disclosure and the narrow-width |
| +// hamburger are markup the shell emits outside all of them. Leaving |
| +// "account" undefined does remove the account menu — tokens.css hides |
| +// a disclosure whose menu is empty — but nothing removes the default |
| +// brand, so a screen that must offer no bar at all hides it in its |
| +// own stylesheet. signin.html is the one that does, and its comment |
| +// says why, including why the obvious selector for it silently fails. |
| const ( |
| dayTheme = "day" |
| shellLayout = "topbar" |
| @@ -170,9 +175,11 @@ var Catalog = rastrillo.Catalog{ |
| "docs.signin.error.expired": "That link has expired or was already used. Enter your email below for a new one.", |
| "docs.signin.error.unknown": "We couldn't sign you in. Enter your email below for a new link.", |
| - // docs.signout.label is the sign-out control's text (list.html's |
| - // and show.html's page headers) — both its button label and its |
| - // accessible name. |
| + // docs.signout.label is the sign-out control's text — both its |
| + // button label and its accessible name. It lives in the shell |
| + // bar's account menu, which list.html and show.html each fill with |
| + // an "account" block; it was in their two page headers until the |
| + // chrome moved to the topbar shell (see shellLayout above). |
| "docs.signout.label": "Sign out", |
| // docs.editor.status.* is the save-status indicator fix round 1's |
| @@ -247,8 +254,8 @@ var Catalog = rastrillo.Catalog{ |
| "docs.editor.toolbar.align.center": "Align centre", |
| "docs.editor.toolbar.align.right": "Align right", |
| - // docs.export.* — the two download links in show.html's page |
| - // header. The short forms are what the links say; the long forms |
| + // docs.export.* — the two download links at the trailing end of |
| + // show.html's formatting bar ([docs-doc-actions]). The short forms are what the links say; the long forms |
| // are their accessible names, because "PDF" on its own does not |
| // say what will happen when it is followed. |
| "docs.export.pdf.short": "PDF", |
diff --git a/internal/docs/views/head.html b/internal/docs/views/head.html| index feaffe5..125e95d 100644 |
| --- a/internal/docs/views/head.html |
| +++ b/internal/docs/views/head.html |
| @@ -22,4 +22,28 @@ |
| not out of the rastrillo.Assets tree {{asset}} fingerprints. |
| assets_test.go asserts this markup and that constant agree, so |
| the pair cannot drift into a silent 404. */}} |
| -{{define "head"}}<link rel="icon" type="image/svg+xml" href="/static/icon.svg">{{end}} |
| +{{/* The one app-wide style rule, and it is here for the same reason |
| + the icon is: it belongs to every page and must be declared once. |
| + |
| + [rst-shell-brand] is now a real target on the signed-in screens — |
| + it is the route back to the list, which used to be [docs-back] in |
| + show.html's own header. tokens.css gives the brand colour, weight |
| + and no padding at all, so it measures 21px tall against 14px body |
| + text at line-height 1.5. AGENTS.md commits to WCAG 2.2 AA with no |
| + exemptions, and SC 2.5.8's 24x24 floor is one of the three it |
| + names. 21px clears that only through the spacing exception, which |
| + is a different argument that stops holding the moment anything |
| + lands beside the brand. |
| + |
| + [docs-back] had already learned this and stated the floor outright |
| + rather than hoping padding added up to it — measured, its first |
| + version came to 22px because --rst-fs-sm is 12.5px and not the |
| + 14px it was assumed to be. Deleting [docs-back] deleted that |
| + finding with it; this is the same rule at the link's new address. |
| + inline-flex is what makes min-block-size apply to a link at all. |
| + |
| + Written against the framework's own element rather than proposed |
| + upstream yet: spec §10 says a component we keep editing belongs |
| + upstream, and if the sibling apps want the same floor this is the |
| + rule to send. It is inert on sign-in, which hides the bar. */}} |
| +{{define "head"}}<link rel="icon" type="image/svg+xml" href="/static/icon.svg"><style>[rst-shell-brand]{display:inline-flex;align-items:center;min-block-size:24px}</style>{{end}} |
diff --git a/internal/docs/views/show.html b/internal/docs/views/show.html| index acfcec0..5cb89c0 100644 |
| --- a/internal/docs/views/show.html |
| +++ b/internal/docs/views/show.html |
| @@ -103,12 +103,30 @@ |
| heading in the only place that counts, <title>, and a visible h1 |
| that is not the document's first line was the thing making the |
| old header read as a screen heading rather than a document's |
| - name. Truncation over wrapping — flex: 0 1 auto with a zero |
| - min-inline-size, which is what lets a flex item shrink below its |
| - content at all — so a long title gives way to the account |
| - control rather than pushing it off the bar. */ |
| + name. |
| + |
| + flex: 1 1 0, and the zero basis is the whole point rather than a |
| + tidy default. [rst-shell-bar] wraps (tokens.css), and a flex |
| + container decides its line breaks from each item's HYPOTHETICAL |
| + main size — for `flex-basis: auto` on a nowrap span, its full |
| + untruncated width — before shrinking or min-inline-size get a |
| + say. So the obvious spelling, flex: 0 1 auto with |
| + min-inline-size: 0, does not truncate a long title at all: it |
| + drops the title onto a second row of the bar and the account |
| + control onto a third, and only ellipsises once the title alone |
| + is wider than the whole bar. A zero basis never forces a break, |
| + so the item grows into whatever space is free and shrinks with |
| + an ellipsis when there is none — which is the behaviour this |
| + paragraph used to claim and did not have. |
| + |
| + What that costs, knowingly: a truncated title is then readable |
| + nowhere on this screen — the span is not focusable and the |
| + document's own first line is not its title. It is still in the |
| + tab title, and in full on the list. A wrapping bar would keep it |
| + legible at the price of the chrome changing height as you open |
| + documents, which is the thing this whole branch is undoing. */ |
| [docs-shell-title] { |
| - flex: 0 1 auto; |
| + flex: 1 1 0; |
| min-inline-size: 0; |
| overflow: hidden; |
| white-space: nowrap; |
| @@ -166,14 +184,20 @@ |
| will own properly later — and on a desktop it is worth every |
| pixel. On a 375px phone the two together (375 - 32 - 192) leave a |
| 151px column, and at 320px, which this app supports without |
| - exemption (WCAG 2.2 AA SC 1.4.10), 96px: about seven characters |
| + exemption (WCAG 2.2 AA SC 1.4.10), 96px: about twelve characters |
| of 12pt body text per line. A page margin that squeezes the page |
| - is no longer describing the page. (The character counts here and |
| - below were measured against 12pt Georgia, the fallback for the |
| - serif this canvas used to default to. The default is Docs Sans |
| - now, whose characters run a little wider at the same size, so the |
| - counts are if anything generous — the breakpoint the arithmetic |
| - argues for does not move.) |
| + is no longer describing the page. |
| + |
| + (Every character count in this comment divides by 8px, the |
| + average advance of 12pt Georgia — the fallback for the serif this |
| + canvas used to default to. Stated because the counts here were |
| + for a while derived from two different divisors at once: "five |
| + characters" for 64px is 12.8px each, "40 characters" for 320px is |
| + 8px each, and both sat in the same paragraph claiming to be |
| + measured. The default is Docs Sans now, whose characters run a |
| + little wider at the same size, so these are if anything |
| + generous — the breakpoint the arithmetic argues for does not |
| + move.) |
| (The count and the numbers in this paragraph have now been wrong |
| in both directions, which is worth leaving on the record. They |
| @@ -520,13 +544,21 @@ |
| toolbar so the exports stay put with it instead of scrolling out |
| from beside it. |
| - It wraps, and the order matters when it does: at 320px the |
| - formatting groups take the first line or two and the exports drop |
| - to their own, which is why [docs-doc-actions] uses an auto margin |
| - rather than `justify-content: space-between` — an auto margin |
| - collapses on a wrapped line and leaves the exports flush left |
| - under the toolbar, where space-between would strand them apart |
| - from each other at opposite screen edges. */ |
| + It wraps: at 320px the formatting groups take the first lines and |
| + the exports drop to one of their own. |
| + |
| + [docs-doc-actions] gets there by an auto margin rather than |
| + `justify-content: space-between`, and the difference is only |
| + visible once the bar has wrapped. On one line the two are |
| + identical — two items, all the free space between them. On a |
| + wrapped line they are opposites: flexbox distributes free space |
| + to auto margins per line, so the auto margin keeps the exports at |
| + the trailing end, where they were before the wrap, while |
| + space-between gives a lone item flex-start and jumps them to the |
| + leading edge. Staying put is the point. (An earlier version of |
| + this comment had this exactly backwards, and also claimed |
| + space-between would separate the two links from each other, which |
| + it cannot — both are inside this one flex item.) */ |
| [docs-bar] { |
| position: sticky; |
| inset-block-start: 0; |
diff --git a/internal/docs/views/signin.html b/internal/docs/views/signin.html| index 6a23b21..725cb39 100644 |
| --- a/internal/docs/views/signin.html |
| +++ b/internal/docs/views/signin.html |
| @@ -35,12 +35,16 @@ |
| rather than by overriding a block, which is the only lever it |
| has. |
| - It hides them because there is nothing true to put in them. |
| - Every control that bar carries is for somebody with a session: |
| - an account menu for a person who has no account yet, and a brand |
| - link to "/" that RequireSession would bounce straight back to |
| - this page. An empty bar would be worse than none — a strip of |
| - chrome that looks like it should do something. |
| + It hides them because there is nothing true to put in them. The |
| + account menu is not actually the problem: tokens.css already |
| + carries `[rst-shell-account]:has(> [rst-dropdown-menu]:empty) |
| + { display: none }`, so a page that defines no "account" block — |
| + which this one does not — loses it without being asked. What is |
| + left over is the part this page really does have to deal with: |
| + the shell's DEFAULT brand, a link to "/" that RequireSession |
| + bounces straight back here, and below 800px the hamburger, which |
| + would open an empty tail. An empty bar is worse than none — a |
| + strip of chrome that looks like it should do something. |
| Hiding the bar rather than the two controls inside it, so the |
| rule cannot go stale the day the shell grows a third one. |
diff --git a/internal/export/pdf.go b/internal/export/pdf.go| index 79dc1eb..83f7772 100644 |
| --- a/internal/export/pdf.go |
| +++ b/internal/export/pdf.go |
| @@ -274,8 +274,10 @@ func PDF(doc Document) ([]byte, Report, error) { |
| // The title is set as a level-1 heading at the top of the first |
| // page. It is not part of the CRDT and so not part of the box tree |
| - // the screen renders — the screen puts it in the page header, |
| - // which is chrome around the paper rather than on it. A PDF has no |
| + // the screen renders — the screen puts it in the shell bar beside |
| + // the brand link, which is chrome around the paper rather than on |
| + // it (it was a page header until the chrome moved to the topbar |
| + // shell; either way it is not on the paper). A PDF has no |
| // chrome: a file that does not say which document it is, is worse |
| // than one that occasionally repeats a heading its author also |
| // typed. Markdown does the same thing for the same reason. |