amadan
Five design defects the Impeccable pass found on the three real screens
Found by running the app, signing in for real, and measuring the
rendered pages at 1440px and at the 320px reflow width spec §10.3
commits to — not by reading the templates. Each fix carries the
measurement that justified it.
1. A document had no way back to the list. GET /d/{sqid} rendered
sign-out and no link at all, so arriving by bookmark or mailed
link — the two ways in that do not pass through GET / — stranded a
person there. Adds [docs-back], a breadcrumb whose label reuses
docs.list.title rather than inventing a docs.show.back that could
drift from the screen it points at in eleven locales nobody here
reads. No new user-visible string, so no copy that has not been
through review.
2. A list row was 7% clickable. The row is 990x61; its only target
was the title's 268x16 text box, and the thumbnail — the part of a
document row a person aims at — did nothing. The 16px height also
cleared WCAG 2.2 AA's 24x24 minimum only via SC 2.5.8's Spacing
exception, a thin defence under "no exemptions". Ordinary
stretched-link overlay: one anchor, one tab stop, unchanged
accessible name, plus a pointer-only hover tint so the enlarged
target is visible rather than merely real. Costs the timestamp's
text selection.
3. Sign-in was an unlaid-out form. One email field stretched to 950px
inside a 992px box with its submit button flush against its
underside, zero gap. Now one 32rem measure shared by the box and
its callouts, field and button side by side.
4. The mobile header spent 175px of a 720px viewport before the first
document. A fixed 14rem field could not share a 288px column with
its button, so field, "New document" and "Sign out" each took a
row. 175px -> 133px. Took two attempts: a wrapping flex line
breaks before it shrinks, so the first version measured exactly as
tall as before it — nowrap is what forces the field to yield width
instead of the button yielding the line.
5. No favicon; /favicon.ico 404'd on every page. A hand-drawn sheet
mark on the day theme's accent, in the shell's documented "head"
block, defined once in the base template so all three pages
inherit it.
Two mistakes worth keeping, both caught by mechanisms rather than by
care, and both now held by one:
- The icon's first version named a CSS custom property in an XML
comment with its leading "--", which XML forbids. It served a
perfect 200 with the right Content-Type and byte count, and no
browser could parse it. Only rendering it showed anything wrong.
TestIconIsWellFormedXML decodes every token.
- It was first written to internal/docs/static/, which is the editor
build's output: the gate rebuilds into that directory, fails on any
dirt there, then git-cleans it — so the file reddened CI and was
then deleted by it. Hand-authored assets now live in brand/
(brandFS), and TestExactlyOneEditorBundleIsEmbedded asserts static/
holds nothing else, so this fails in go test rather than in CI.
Verified: .amadan/ci.d/10-gate exits 0. No horizontal overflow and no
console errors on any of the five surfaces at either width. The
bundled design detector reports one finding, verified as a false
positive: a blockquote's 3px border-inline-start inside [docs-text],
which is prose typography in rendered document content, not the
card/callout accent that rule is about.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QsrkL4m68poXhtf1tGaoek
10 files changed,
+571
−27
internal/docs/assets.go+60 −6internal/docs/assets_test.go+96 −0internal/docs/brand/icon.svg+44 −0internal/docs/handlers.go+19 −0internal/docs/list_test.go+128 −0internal/docs/render.go+17 −0internal/docs/views/head.html+25 −0internal/docs/views/list.html+79 −6internal/docs/views/show.html+45 −0internal/docs/views/signin.html+58 −15
CI passed — run details
Intent
Attributed to this commit: 342 g CO₂e (37222373 tokens). This is an attribution, not a measurement — usage counted from this commit's own marker forward to the next one on worktree-impeccable-init, 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/internal/docs/assets.go b/internal/docs/assets.go| index 057991b..6671e34 100644 |
| --- a/internal/docs/assets.go |
| +++ b/internal/docs/assets.go |
| @@ -17,8 +17,24 @@ import ( |
| "time" |
| ) |
| -// staticFS embeds this app's own committed static assets — today, |
| -// only the vendored editor bundle. internal/docs/static/, not a |
| +// brandFS embeds this app's own hand-authored assets — today, the tab |
| +// icon served at iconPath. |
| +// |
| +// A directory of its own, deliberately not static/ below. static/ is |
| +// the editor build's output: .amadan/ci.d/10-gate rebuilds the bundle |
| +// into it and then fails on any dirt at all in that directory, before |
| +// restoring it with git checkout and git clean. So a hand-written file |
| +// there does not merely sit oddly beside generated output — it turns |
| +// the gate red, and the gate's own cleanup then deletes it. That is |
| +// not hypothetical; icon.svg was written to static/ first and this is |
| +// what happened to it. |
| +// |
| +//go:embed brand |
| +var brandFS embed.FS |
| + |
| +// staticFS embeds the editor build's committed output — the vendored |
| +// editor bundle, and nothing else, for the reason brandFS's own doc |
| +// comment gives. internal/docs/static/, not a |
| // top-level static/: go:embed cannot reach outside (or above) the |
| // directory containing the .go file that embeds it — no ".." is ever |
| // allowed in an embed pattern — so this lives under internal/docs/ |
| @@ -65,6 +81,40 @@ func editorScriptPath() string { |
| return "/static/" + editorBundleName |
| } |
| +// iconPath is the absolute URL path this app's tab icon is served at, |
| +// and views/head.html's <link rel="icon"> href. Fixed rather than |
| +// content-hashed the way the editor bundle above is: a favicon is |
| +// requested by the browser for every page and changes about never, so |
| +// the hash would buy nothing, and a stable path is also what a client |
| +// that ignores the <link> and asks for a conventional location can be |
| +// pointed at. |
| +// |
| +// assets_test.go holds this against what views/head.html actually |
| +// writes, so the two can never drift into a 404 that only ever shows |
| +// up as a missing tab icon nobody files a bug about. |
| +const iconPath = "/static/icon.svg" |
| + |
| +// iconContentType is set explicitly for the same reason |
| +// editorBundleContentType below is: not left to the host's mime |
| +// registry. A browser will not render an icon served as |
| +// text/plain or application/octet-stream. |
| +const iconContentType = "image/svg+xml; charset=utf-8" |
| + |
| +// serveIcon writes the embedded tab icon. A day's cache, not the |
| +// editor bundle's immutable year: the path carries no content hash, |
| +// so a cached copy has to expire on its own for a redrawn icon ever |
| +// to reach anyone. |
| +func serveIcon(w http.ResponseWriter, r *http.Request) { |
| + b, err := fs.ReadFile(brandFS, "brand/icon.svg") |
| + if err != nil { |
| + http.NotFound(w, r) |
| + return |
| + } |
| + w.Header().Set("Content-Type", iconContentType) |
| + w.Header().Set("Cache-Control", "public, max-age=86400") |
| + http.ServeContent(w, r, "icon.svg", time.Time{}, bytes.NewReader(b)) |
| +} |
| + |
| // editorBundleContentType is set explicitly (http.ServeContent would |
| // otherwise guess from the ".js" extension via the OS mime registry, |
| // which on some platforms maps it to "application/javascript" and on |
| @@ -90,9 +140,9 @@ func serveEditorBundle(w http.ResponseWriter, r *http.Request) { |
| http.ServeContent(w, r, editorBundleName, time.Time{}, bytes.NewReader(b)) |
| } |
| -// staticHandler composes this app's own committed static assets |
| -// (today, only the editor bundle, at its one fixed, resolved-at-init |
| -// path) with fallback — render.go's rastrillo.Assets tree (the |
| +// staticHandler composes this app's own committed static assets (the |
| +// editor bundle, at its one fixed resolved-at-init path, and the tab |
| +// icon at iconPath) with fallback — render.go's rastrillo.Assets tree (the |
| // vendored design-system CSS/JS) — into the single handler app.go |
| // mounts at "/static/*". Both trees share that URL prefix, so this is |
| // one composed handler rather than two competing route registrations |
| @@ -108,9 +158,13 @@ func serveEditorBundle(w http.ResponseWriter, r *http.Request) { |
| func staticHandler(fallback http.Handler) http.Handler { |
| editorPath := editorScriptPath() |
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| - if r.URL.Path == editorPath { |
| + switch r.URL.Path { |
| + case editorPath: |
| serveEditorBundle(w, r) |
| return |
| + case iconPath: |
| + serveIcon(w, r) |
| + return |
| } |
| fallback.ServeHTTP(w, r) |
| }) |
diff --git a/internal/docs/assets_test.go b/internal/docs/assets_test.go| index 9cfcec2..72b95d7 100644 |
| --- a/internal/docs/assets_test.go |
| +++ b/internal/docs/assets_test.go |
| @@ -1,6 +1,9 @@ |
| package docs |
| import ( |
| + "bytes" |
| + "encoding/xml" |
| + "io" |
| "io/fs" |
| "net/http" |
| "net/http/httptest" |
| @@ -29,6 +32,21 @@ func TestExactlyOneEditorBundleIsEmbedded(t *testing.T) { |
| if got := editorScriptPath(); got != "/static/"+found[0] { |
| t.Errorf("editorScriptPath() = %q, want %q", got, "/static/"+found[0]) |
| } |
| + |
| + // And nothing else in static/ at all. That directory is the editor |
| + // build's output: .amadan/ci.d/10-gate rebuilds into it and fails |
| + // on any dirt whatsoever there, then restores it with git checkout |
| + // and git clean — so an extra file both reddens the gate and is |
| + // deleted by it. The tab icon was written here first and lost |
| + // exactly that way, which is why this is a test and not a note. |
| + // Hand-authored assets go in brand/ (brandFS). |
| + if len(entries) != 1 { |
| + var names []string |
| + for _, e := range entries { |
| + names = append(names, e.Name()) |
| + } |
| + t.Errorf("static/ holds %v; it must hold only the editor build's output — put hand-authored assets in brand/", names) |
| + } |
| } |
| // The bundle must be served without a session, or a signed-out reader |
| @@ -56,3 +74,81 @@ func TestEditorBundleIsServed(t *testing.T) { |
| t.Errorf("Cache-Control = %q, want immutable (the filename carries the content hash)", cc) |
| } |
| } |
| + |
| +// The tab icon must be served, unauthenticated, with an image content |
| +// type — and the markup that links it must point at the constant this |
| +// package actually routes on. Those two drifting apart is the exact |
| +// failure this pins: a wrong href does not break a page, it quietly |
| +// restores the blank-page tab mark the icon exists to replace, which |
| +// nobody files a bug about and no other test here would notice. |
| +func TestIconIsServedAndLinkedConsistently(t *testing.T) { |
| + app, err := NewForTest(t) |
| + if err != nil { |
| + t.Fatalf("NewForTest: %v", err) |
| + } |
| + // No session on this request, on purpose: the sign-in page needs |
| + // the icon as much as any signed-in screen does, and it is |
| + // reached without one. |
| + req := httptest.NewRequest(http.MethodGet, iconPath, nil) |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, req) |
| + |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("GET %s = %d, want 200", iconPath, rec.Code) |
| + } |
| + if ct := rec.Header().Get("Content-Type"); !strings.Contains(ct, "image/svg+xml") { |
| + t.Errorf("Content-Type = %q, want image/svg+xml", ct) |
| + } |
| + if body := rec.Body.String(); !strings.Contains(body, "<svg") { |
| + t.Errorf("body does not look like an SVG: %.80q", body) |
| + } |
| + |
| + head, err := fs.ReadFile(viewsFS, headTemplate) |
| + if err != nil { |
| + t.Fatalf("read %s: %v", headTemplate, err) |
| + } |
| + if wantHref := `href="` + iconPath + `"`; !strings.Contains(string(head), wantHref) { |
| + t.Errorf("%s does not contain %s — the <link rel=icon> href and iconPath have drifted", headTemplate, wantHref) |
| + } |
| +} |
| + |
| +// The icon must be well-formed XML, because SVG is XML and a browser |
| +// will not render one that is not — it fails closed, showing the same |
| +// blank tab mark as no icon at all. |
| +// |
| +// This exists because the first version of icon.svg was not: its |
| +// comment named a CSS custom property with the two leading hyphens a |
| +// custom property has, and two consecutive hyphens are illegal inside |
| +// an XML comment. Everything a request-level check can see was |
| +// correct — 200, image/svg+xml, a plausible Content-Length, the |
| +// expected bytes — and the file was still unparseable. Only rendering |
| +// it showed anything wrong, which is exactly the failure shape |
| +// AGENTS.md warns about: a verification that cannot tell "no" from |
| +// "I can't see it". |
| +// |
| +// Decoding every token, rather than checking the root element, is the |
| +// point: a malformed comment or attribute halfway down is precisely |
| +// what a shallower check misses. |
| +func TestIconIsWellFormedXML(t *testing.T) { |
| + b, err := fs.ReadFile(brandFS, "brand/icon.svg") |
| + if err != nil { |
| + t.Fatalf("read icon: %v", err) |
| + } |
| + dec := xml.NewDecoder(bytes.NewReader(b)) |
| + var root string |
| + for { |
| + tok, err := dec.Token() |
| + if err == io.EOF { |
| + break |
| + } |
| + if err != nil { |
| + t.Fatalf("icon.svg is not well-formed XML, so no browser will render it: %v", err) |
| + } |
| + if se, ok := tok.(xml.StartElement); ok && root == "" { |
| + root = se.Name.Local |
| + } |
| + } |
| + if root != "svg" { |
| + t.Errorf("icon.svg root element = %q, want svg", root) |
| + } |
| +} |
diff --git a/internal/docs/brand/icon.svg b/internal/docs/brand/icon.svg| new file mode 100644 |
| index 0000000..52f4376 |
| --- /dev/null |
| +++ b/internal/docs/brand/icon.svg |
| @@ -0,0 +1,44 @@ |
| +<!-- CARLOS Docs' tab icon. Drawn here rather than taken from an icon |
| + set: rastrillo ships no page or document glyph (rastrillo/icons.go |
| + carries thirteen, none of them this), and a Unicode character |
| + standing in for a mark is not an icon. |
| + |
| + A sheet with a turned corner, on a tile in the day theme's accent |
| + (#2464e0, the light value of the rst-accent custom property in |
| + ui/themes/day.css). Taken as a literal because an SVG served as a |
| + static file cannot read a CSS custom property, and because this |
| + mark must not repaint with a reader's colour scheme the way the |
| + chrome around it does. |
| + |
| + Sized for 16px first: one filled tile, one white silhouette, one |
| + turned corner, and two rules for the text. Everything is at least |
| + 1.5 units across at a 32 unit viewBox, so nothing thins below a |
| + device pixel at tab size. The rules are what make it read as a |
| + document rather than a blank file, and they stop at two because a |
| + third closes the gap between them into grey mush. |
| + |
| + Why brand/ and not static/, which is where this file was first |
| + written: internal/docs/static/ is build output, not a place to |
| + keep things. The gate rebuilds the editor bundle into it and then |
| + fails on any dirt at all in that directory |
| + (.amadan/ci.d/10-gate's "the editor bundle must match a fresh |
| + build"), before restoring it with git checkout and git clean. |
| + A hand written file there is therefore not merely untidy: it |
| + turns the gate red, and then the gate's own cleanup deletes it. |
| + It did, once. Hand authored assets belong beside the build |
| + output, never inside it. |
| + |
| + Note for whoever edits this comment: an XML comment may not |
| + contain two consecutive hyphens, so a CSS custom property cannot |
| + be written here with its leading pair. The first version of this |
| + file did exactly that, and the result was a well formed HTTP 200 |
| + serving a document no browser could parse: right status, right |
| + Content-Type, right byte count, broken image. assets_test.go's |
| + TestIconIsWellFormedXML exists because of it. --> |
| +<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32" width="32" height="32"> |
| + <rect width="32" height="32" rx="7" fill="#2464e0"/> |
| + <path d="M9.5 8.5A2 2 0 0 1 11.5 6.5h6.6L23.5 11.9V23.5a2 2 0 0 1-2 2h-10a2 2 0 0 1-2-2Z" fill="#ffffff"/> |
| + <path d="M18.1 6.5 23.5 11.9h-5.4Z" fill="#a9c4f4"/> |
| + <rect x="12.9" y="15.6" width="7.6" height="1.7" rx="0.85" fill="#2464e0"/> |
| + <rect x="12.9" y="19.2" width="5.2" height="1.7" rx="0.85" fill="#2464e0"/> |
| +</svg> |
diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go| index 778790d..7c8f5b0 100644 |
| --- a/internal/docs/handlers.go |
| +++ b/internal/docs/handlers.go |
| @@ -188,6 +188,24 @@ type showPage struct { |
| Title string |
| 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 |
| + // 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 |
| + // go back *to*. |
| + // |
| + // It deliberately reuses docs.list.title rather than adding a |
| + // docs.show.back of its own: this is a breadcrumb naming its |
| + // destination screen, and a breadcrumb that can drift from the |
| + // title of the screen it points at is a translation bug waiting |
| + // to happen in eleven locales nobody in this repo reads. One key, |
| + // one string, and the link can never disagree with the page it |
| + // leads to. |
| + BackLabel string |
| Sqid string // views/show.html's [docs-editor] mount point's data-sqid, the editor bundle's own base for /d/{sqid}/updates |
| EditorScript string // views/show.html's <script src>, from editorScriptPath() — never a literal, so it can never drift from what staticHandler actually serves |
| @@ -247,6 +265,7 @@ func (h *handlers) show(w http.ResponseWriter, r *http.Request) { |
| Title: doc.Title, |
| HTML: html, |
| SignOutLabel: rastrillo.T(r, "docs.signout.label"), |
| + BackLabel: rastrillo.T(r, "docs.list.title"), |
| Sqid: doc.Sqid, |
| EditorScript: editorScriptPath(), |
| SaveRetrying: rastrillo.T(r, "docs.editor.status.retrying"), |
diff --git a/internal/docs/list_test.go b/internal/docs/list_test.go| index a047183..eb254b0 100644 |
| --- a/internal/docs/list_test.go |
| +++ b/internal/docs/list_test.go |
| @@ -197,3 +197,131 @@ func TestUnknownDocument404s(t *testing.T) { |
| t.Errorf("status %d, want 404 — a document you cannot see does not exist", rec.Code) |
| } |
| } |
| + |
| +// A document page must offer a way back to the list. Before |
| +// [docs-back] landed, GET /d/{sqid} rendered sign-out and no link at |
| +// all, so arriving by a bookmark or a mailed link — the two ways of |
| +// reaching a document that do not pass through GET / on the way — |
| +// stranded a person on it with nothing but the browser's own back |
| +// button, which such an arrival gives nothing to go back to. |
| +// |
| +// Asserted on the rendered page rather than the template's bytes: the |
| +// href has to survive the handler populating it, not merely be |
| +// written somewhere in views/. |
| +func TestDocumentPageLinksBackToTheList(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "reader@example.com") |
| + doc, err := app.Store().Create(ctx, owner, "Board minutes") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + cookies := signInCookies(t, app, "reader@example.com") |
| + |
| + req := httptest.NewRequest(http.MethodGet, "/d/"+doc.Sqid, nil) |
| + for _, c := range cookies { |
| + req.AddCookie(c) |
| + } |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, req) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("status %d", rec.Code) |
| + } |
| + |
| + body := rec.Body.String() |
| + if !strings.Contains(body, `<a docs-back href="/">`) { |
| + t.Error("no [docs-back] link to / on the document page — a document reached directly has no route to the list") |
| + } |
| + |
| + // The label is docs.list.title, deliberately shared with the screen |
| + // it points at rather than given a docs.show.back of its own |
| + // (showPage.BackLabel's doc comment). Assert that by rendering the |
| + // list and comparing the two, rather than by looking for the |
| + // literal "Documents": a direct App.Handler() call installs no |
| + // locale middleware, so rastrillo.T here returns the key verbatim |
| + // (render.go's Locales comment) and the English string never |
| + // appears. Comparing the two pages holds the invariant that |
| + // actually matters — the link and its destination's own title are |
| + // one string — and holds it in every locale rather than only in the |
| + // one this test happens to run in. |
| + listReq := httptest.NewRequest(http.MethodGet, "/", nil) |
| + for _, c := range cookies { |
| + listReq.AddCookie(c) |
| + } |
| + listRec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(listRec, listReq) |
| + if listRec.Code != http.StatusOK { |
| + t.Fatalf("list status %d", listRec.Code) |
| + } |
| + listTitle := betweenTags(t, listRec.Body.String(), "<h1>", "</h1>") |
| + backLabel := betweenTags(t, body, `<a docs-back href="/">`, "</a>") |
| + if backLabel == "" || backLabel != listTitle { |
| + t.Errorf("back link label = %q, want the list's own <h1> (%q)", backLabel, listTitle) |
| + } |
| +} |
| + |
| +// betweenTags returns the text between the first open and the next |
| +// close after it, for the small structural assertions above. Not a |
| +// general HTML parser and not trying to be one — it is used only on |
| +// markup this package itself writes, two lines apart. |
| +func betweenTags(t *testing.T, body, open, close string) string { |
| + t.Helper() |
| + i := strings.Index(body, open) |
| + if i < 0 { |
| + return "" |
| + } |
| + rest := body[i+len(open):] |
| + j := strings.Index(rest, close) |
| + if j < 0 { |
| + return "" |
| + } |
| + return rest[:j] |
| +} |
| + |
| +// Every row in the list must be openable across its whole area, not |
| +// only across the title's own text box. Measured before this landed: |
| +// a 990×61 row whose only target was a 268×16 link — 7% of it — with |
| +// the thumbnail, the part of a document row a person aims at most |
| +// readily, inert. The 16px height also cleared WCAG 2.2 AA's 24×24 |
| +// minimum (SC 2.5.8) only via that criterion's Spacing exception, |
| +// which spec §10.3 ("24×24 targets", "no exemptions") is a poor fit |
| +// for. |
| +// |
| +// This pins the markup contract the CSS overlay depends on — the row |
| +// carrying [docs-row] and its anchor carrying [docs-row-link] — since |
| +// the stretched-link rule is inert if either attribute is dropped, |
| +// and dropping one is silent: the page still renders, the link still |
| +// works, and only the target shrinks back to the title's text. |
| +func TestListRowsAreClickableAcrossTheWholeRow(t *testing.T) { |
| + app, _ := docs.NewForTest(t) |
| + ctx := context.Background() |
| + owner := app.TestPerson(t, "reader@example.com") |
| + if _, err := app.Store().Create(ctx, owner, "Board minutes"); err != nil { |
| + t.Fatal(err) |
| + } |
| + cookies := signInCookies(t, app, "reader@example.com") |
| + |
| + req := httptest.NewRequest(http.MethodGet, "/", nil) |
| + for _, c := range cookies { |
| + req.AddCookie(c) |
| + } |
| + rec := httptest.NewRecorder() |
| + app.Handler().ServeHTTP(rec, req) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("status %d", rec.Code) |
| + } |
| + |
| + body := rec.Body.String() |
| + if !strings.Contains(body, "<div rst-row docs-row>") { |
| + t.Error("the row is missing [docs-row] — the stretched-link overlay has nothing positioned to cover") |
| + } |
| + if !strings.Contains(body, "<a docs-row-link ") { |
| + t.Error("the row's anchor is missing [docs-row-link] — the overlay rule will not match it") |
| + } |
| + // The overlay must be declared too: the attributes above are |
| + // inert markup without the rule that stretches the anchor over |
| + // the row. |
| + if !strings.Contains(body, "[docs-row-link]::after") { |
| + t.Error("no stretched-link rule on the page — the row attributes are present but nothing enlarges the target") |
| + } |
| +} |
diff --git a/internal/docs/render.go b/internal/docs/render.go| index 5f221cd..3512e4f 100644 |
| --- a/internal/docs/render.go |
| +++ b/internal/docs/render.go |
| @@ -158,6 +158,13 @@ const DefaultLocale = "en" |
| //go:embed views/*.html |
| var viewsFS embed.FS |
| +// headTemplate is the app-wide "head" definition newRenderer parses |
| +// into the base template — the shell's own app slot in <head>, which |
| +// today carries the tab icon. Deliberately outside pageTemplates |
| +// below: it defines "head", never "content", so it is not a page and |
| +// must not be cloned per page. |
| +const headTemplate = "views/head.html" |
| + |
| // pageTemplates maps a page name to its file under views/. |
| var pageTemplates = map[string]string{ |
| "list": "views/list.html", |
| @@ -201,6 +208,16 @@ func newRenderer() (*renderer, error) { |
| return nil, fmt.Errorf("docs: parse rastrillo shell %q: %w", shellLayout, err) |
| } |
| + // This app's own "head" definition, parsed into base *after* the |
| + // shell so it overrides the shell's empty {{block "head"}} |
| + // default, and before the per-page clones below so all three |
| + // pages inherit the one definition rather than each carrying a |
| + // copy of the icon link. Not in pageTemplates: it defines no |
| + // "content" and is not a page. |
| + if base, err = base.ParseFS(viewsFS, headTemplate); err != nil { |
| + return nil, fmt.Errorf("docs: parse head template: %w", err) |
| + } |
| + |
| pages := make(map[string]*template.Template, len(pageTemplates)) |
| for name, path := range pageTemplates { |
| page, err := base.Clone() |
diff --git a/internal/docs/views/head.html b/internal/docs/views/head.html| new file mode 100644 |
| index 0000000..feaffe5 |
| --- /dev/null |
| +++ b/internal/docs/views/head.html |
| @@ -0,0 +1,25 @@ |
| +{{/* head — this app's slot in the shell's <head>. |
| + |
| + rastrillo's three shells all end their head with |
| + {{block "head" .}}{{end}}, described in ui/layouts/column.html as |
| + "the app's own slot in <head>: a favicon, a meta tag, one more |
| + stylesheet". This file redefines that block once, in the base |
| + template every page is cloned from (render.go's newRenderer), so |
| + all three pages inherit it and there is exactly one place the |
| + icon is declared. A page that ever needs head content of its own |
| + defines "head" in its own template and overrides this, which is |
| + ordinary block behaviour and needs nothing added here. |
| + |
| + Before this existed the app declared no icon at all, so every |
| + browser fell back to asking for /favicon.ico, got a 404, and |
| + showed the generic blank page mark — on a document editor, where |
| + the tab strip is the thing a person navigates between open |
| + documents with, and where two sibling apps (Sheets, and the |
| + suite home) sit in the same tab strip under the same suite. |
| + |
| + The href is a literal rather than {{asset}}: icon.svg is served |
| + by assets.go's own staticHandler at a fixed, unhashed iconPath, |
| + 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}} |
diff --git a/internal/docs/views/list.html b/internal/docs/views/list.html| index d1565f5..abe4a05 100644 |
| --- a/internal/docs/views/list.html |
| +++ b/internal/docs/views/list.html |
| @@ -64,6 +64,46 @@ |
| color: var(--rst-text-muted); |
| font-size: var(--rst-fs-sm); |
| } |
| + /* [docs-row] + [docs-row-link]::after — the whole row opens the |
| + document, not just the title's own text box. |
| + Measured before this landed: a row is 990×61 and the title link |
| + inside it was 268×16, so 7% of what looks like one clickable |
| + object actually was one. The thumbnail — the part of a document |
| + row a person is most likely to aim at — did nothing at all, and |
| + the 16px-tall link cleared WCAG 2.2 AA's 24×24 minimum (SC 2.5.8) |
| + only by leaning on that success criterion's own Spacing |
| + exception, which is a thin defence in an app whose spec (§10.3) |
| + writes "24×24 targets" with "no exemptions" beside it. |
| + The overlay is the ordinary stretched-link idiom: one anchor, |
| + still the row's only tab stop and still carrying the document |
| + title as its accessible name, with an absolutely positioned |
| + ::after covering the row's padding box. Nothing is duplicated for |
| + a screen reader and no second link is invented, which is the |
| + thing that goes wrong when a row is made clickable by wrapping it |
| + in an anchor instead. |
| + What it costs: the timestamp under the overlay is no longer |
| + selectable text. Worth a 61px target on a row whose only purpose |
| + is to be opened. |
| + Focus stays on the title's own text rather than expanding to the |
| + row, so the ring points at what is actually being activated. */ |
| + [docs-row] { |
| + position: relative; |
| + } |
| + [docs-row-link]::after { |
| + content: ""; |
| + position: absolute; |
| + inset: 0; |
| + } |
| + /* A hover tint is the row's only signal that all of it is live — |
| + without it the enlarged target is real but invisible, which is |
| + the version of this fix that tests green and still feels broken. |
| + Pointer devices only: on touch there is no hover state to read, |
| + and :hover there sticks after a tap. */ |
| + @media (hover: hover) { |
| + [docs-row]:hover { |
| + background: var(--rst-surface-2); |
| + } |
| + } |
| /* [docs-header-actions]: the header's one flex child opposite the |
| titles — [rst-page-header]'s space-between rule places it there |
| with no layout rule of its own beyond this; everything it holds |
| @@ -76,15 +116,48 @@ |
| gap: var(--rst-sp-3); |
| } |
| /* [docs-create-form]: the title field and its submit button as one |
| - inline row. */ |
| + inline row — nowrap, deliberately, and this is the part that took |
| + two attempts. |
| + A wrapping flex line does not shrink its items to fit; it breaks |
| + first and shrinks only what still does not fit afterwards. So |
| + with wrap on, the button dropped to a second line and the field — |
| + now alone on the first one, with room to grow — sprang back to |
| + its 14rem maximum, which is how a fix aimed at making the header |
| + shorter measured exactly as tall as before it (175px, unchanged). |
| + nowrap is what forces the field to give up width to its button |
| + instead of the button giving up the line. |
| + The button itself is flex: none so the whole shortfall comes out |
| + of the field. Shared proportionally, both would shrink, and |
| + "New document" would start wrapping or clipping inside a control |
| + that had no reason to be narrower. */ |
| [docs-create-form] { |
| display: flex; |
| - flex-wrap: wrap; |
| + flex-wrap: nowrap; |
| gap: var(--rst-sp-2); |
| } |
| + [docs-create-form] button { |
| + flex: none; |
| + } |
| + /* The title field shares its line with the submit button instead of |
| + holding a fixed 14rem and pushing it onto a line of its own. |
| + Measured at 320px — the reflow width spec §10.3 commits to — |
| + before this changed: a fixed 14rem (224px) field could not sit |
| + beside a 131px button in a 288px content column, so the field, |
| + "New document" and "Sign out" each took a full row and the header |
| + stood 175px tall, putting the first document 223px down a 720px |
| + viewport. Just under a third of the screen spent on chrome before |
| + a single thing a person came here to read. |
| + flex-basis 10rem with room to shrink is what lets the field and |
| + its button share one line in that column; the 14rem cap keeps the |
| + desktop header exactly as it was, since the form still sizes to |
| + its max-content there. min-inline-size: 0 is load-bearing — a |
| + flex item's automatic minimum size is its content, so without it |
| + an <input> refuses to shrink below its default size and the row |
| + overflows rather than fitting. */ |
| [docs-create-form] [rst-input] { |
| - inline-size: 14rem; |
| - max-inline-size: 100%; |
| + flex: 1 1 10rem; |
| + min-inline-size: 0; |
| + max-inline-size: 14rem; |
| } |
| </style> |
| <header rst-page-header> |
| @@ -106,9 +179,9 @@ |
| {{range .Groups}} |
| <h2 docs-group-label>{{.Label}}</h2> |
| {{range .Rows}} |
| - <div rst-row> |
| + <div rst-row docs-row> |
| <span docs-thumb aria-hidden="true"></span> |
| - <span rst-row-main><a href="{{.Href}}">{{.Title}}</a></span> |
| + <span rst-row-main><a docs-row-link href="{{.Href}}">{{.Title}}</a></span> |
| <span docs-row-when>{{.When}}</span> |
| </div> |
| {{end}} |
diff --git a/internal/docs/views/show.html b/internal/docs/views/show.html| index cc5637e..8a147a0 100644 |
| --- a/internal/docs/views/show.html |
| +++ b/internal/docs/views/show.html |
| @@ -36,6 +36,35 @@ |
| holds it against this file too. */}} |
| {{define "title"}}{{.Title}}{{end}} |
| {{define "content"}}<style> |
| + /* [docs-back]: the route to the document list, above the title. |
| + Chrome, so it takes the app's theme tokens rather than the |
| + canvas's literals. Muted at rest and full-strength on hover and |
| + focus, so it reads as secondary to the document's own title |
| + without dropping under the 4.5:1 body-text floor at rest. |
| + min-block-size states the 24×24 floor (WCAG 2.2 AA SC 2.5.8) |
| + outright rather than hoping padding adds up to it: measured, the |
| + first version of this rule — --rst-fs-sm plus a --rst-sp-1 |
| + padding either side — came to 22px, because --rst-fs-sm is |
| + 12.5px here and not the 14px it was assumed to be. A target size |
| + derived from two tokens is a target size that changes when either |
| + token does, silently. inline-flex is what makes min-block-size |
| + apply to a link at all and centres the label inside it. */ |
| + [docs-back] { |
| + display: inline-flex; |
| + align-items: center; |
| + min-block-size: 24px; |
| + padding-block: var(--rst-sp-1); |
| + font-size: var(--rst-fs-sm); |
| + line-height: 1.15; |
| + color: var(--rst-text-muted); |
| + text-decoration: none; |
| + } |
| + [docs-back]:hover, |
| + [docs-back]:focus-visible { |
| + color: var(--rst-text); |
| + text-decoration: underline; |
| + text-underline-offset: 0.2em; |
| + } |
| /* [docs-canvas] is the tray the page sits in — it is chrome around |
| paper, so it stays close to the surrounding theme's tone, but it |
| still opts out of the scheme (color-scheme: light) because its |
| @@ -247,6 +276,22 @@ |
| </style> |
| <header rst-page-header> |
| <div rst-page-header-titles> |
| + {{/* [docs-back] is this page's only in-app navigation, and until |
| + it landed there was none: the header below held sign-out and |
| + nothing else, so arriving here by a bookmark or a mailed link |
| + left a person with no route to their own list at all. It sits |
| + inside [rst-page-header-titles] rather than beside it because |
| + [rst-page-header]'s space-between rule wants at most two |
| + top-level children (titles, one action area) — the same |
| + constraint list.html's [docs-header-actions] comment records |
| + from the other direction. |
| + |
| + A breadcrumb, not an arrow: the label is the destination |
| + screen's own name (showPage.BackLabel, which *is* |
| + docs.list.title), so it needs no glyph to be understood and |
| + nothing about it has to be mirrored for the RTL locales in |
| + this app's twelve. */}} |
| + <a docs-back href="/">{{.BackLabel}}</a> |
| <h1>{{.Title}}</h1> |
| </div> |
| {{/* Sign-out control (POST /signout, auth.go) — same reasoning as |
diff --git a/internal/docs/views/signin.html b/internal/docs/views/signin.html| index 8517e0e..80bc0a9 100644 |
| --- a/internal/docs/views/signin.html |
| +++ b/internal/docs/views/signin.html |
| @@ -26,26 +26,69 @@ |
| bytes at all — only the {{template}} call does, which is exactly |
| what that test walks. */}} |
| {{define "title"}}{{.Title}}{{end}} |
| -{{define "content"}} |
| +{{define "content"}}<style> |
| + /* [docs-signin] gives this screen one measure, shared by the |
| + callouts and the box below them. |
| + Measured before it existed: the box filled the shell's whole |
| + 992px column, and the single email field inside it stretched to |
| + 950px with its submit button sitting flush against the underside |
| + — no gap at all — so the eye had to travel the full width of the |
| + field and come back to find the control that acts on it. A |
| + one-field form rendered at the width of a data table is the |
| + shape of markup nobody has laid out yet, and this is the app's |
| + front door: the one screen a person meets before they have any |
| + reason to extend it credit. |
| + Left-aligned rather than centred, so the box still hangs off the |
| + same axis as the "Sign in" heading above it and the rest of the |
| + app's screens. */ |
| + [docs-signin] { |
| + max-inline-size: 32rem; |
| + } |
| + /* The field and its button share a line, with real space between |
| + them. flex-basis 12rem with room to shrink and min-inline-size: 0 |
| + is what keeps that true down to the 320px reflow width spec §10.3 |
| + commits to — an <input>'s automatic minimum size is its content |
| + width, so without the explicit 0 it refuses to shrink and the row |
| + overflows instead of fitting. Below that the button wraps under |
| + the field rather than either one being squeezed unusable. */ |
| + [docs-signin-form] { |
| + display: flex; |
| + flex-wrap: wrap; |
| + align-items: center; |
| + gap: var(--rst-sp-3); |
| + } |
| + [docs-signin-form] [rst-input] { |
| + flex: 1 1 12rem; |
| + min-inline-size: 0; |
| + } |
| +</style> |
| <header rst-page-header> |
| <div rst-page-header-titles> |
| <h1>{{.Title}}</h1> |
| </div> |
| </header> |
| -{{if .Sent}} |
| -{{template "callout" dict "Body" .Sent "Tone" "positive"}} |
| -{{end}} |
| -{{if .ErrorMessage}} |
| -{{template "callout" dict "Body" .ErrorMessage "Tone" "negative" "Alert" true}} |
| -{{end}} |
| -<div rst-box> |
| - <div rst-box-head> |
| - <p>{{.Body}}</p> |
| +<div docs-signin> |
| + {{/* The callouts live inside [docs-signin] with the box, not above |
| + it at the shell's full width: a confirmation or an error about |
| + this form is part of the form's own column, and one that ran |
| + twice the width of the thing it referred to read as a banner |
| + about the app rather than an answer to what was just |
| + submitted. */}} |
| + {{if .Sent}} |
| + {{template "callout" dict "Body" .Sent "Tone" "positive"}} |
| + {{end}} |
| + {{if .ErrorMessage}} |
| + {{template "callout" dict "Body" .ErrorMessage "Tone" "negative" "Alert" true}} |
| + {{end}} |
| + <div rst-box> |
| + <div rst-box-head> |
| + <p>{{.Body}}</p> |
| + </div> |
| + <form docs-signin-form method="post" action="/signin"> |
| + <input type="hidden" name="force" value="1"> |
| + <input rst-input type="email" name="address" placeholder="{{.AddressLabel}}" aria-label="{{.AddressLabel}}" autocomplete="email" required> |
| + <button rst-btn="primary" type="submit">{{.SubmitLabel}}</button> |
| + </form> |
| </div> |
| - <form method="post" action="/signin"> |
| - <input type="hidden" name="force" value="1"> |
| - <input rst-input type="email" name="address" placeholder="{{.AddressLabel}}" aria-label="{{.AddressLabel}}" autocomplete="email" required> |
| - <button rst-btn="primary" type="submit">{{.SubmitLabel}}</button> |
| - </form> |
| </div> |
| {{end}} |