sulthan
3a83161b1c
One commit (`af07314`), three strands of browser-UI work against one design system. `docs/design-system.md` was updated to match the CSS, not the reverse. ## Library (Reader-facing) Findings came out of a two-axis design review of the library surface; the fixes are the P1/P2 set plus the cheap P3s. - **`.chrome` sticks at `top: 0`.** Search and the tab row were unreachable three screens into a 300-item library — exactly where they earn their keep. Everything above them (`.topbar`, `.keyrow`, `.recent`) still scrolls away on purpose: another 150px of permanent chrome on an 844px phone costs more than re-scrolling for an icon reminder. - **One `:focus-visible` ring** (`2px solid var(--paper)`, offset 2px) on the nine controls that defined none and fell back to the UA blue — a colour tuned for neither branch of this palette. `.searchbar` keeps its `:focus-within` border recolour as a resting cue but no longer stands in for the ring. - **Mono labels lift 10px → 11px** everywhere (nine rules). PRODUCT.md names night reading and glare as the usage scene; 10px small-caps was the one place taste overrode the brief. 11px is now a documented floor. - **A card in flight past 2s says `Saving…` and carries `aria-busy`.** htmx sets neither, so the wait — up to its own 15s timeout, and this app is used on a phone in dead zones — was silent in both the visual and the assistive channel. Deliberately `--mute`, not `--ember`: ember means "new chapter" and nothing else. - **Titles clamp at 3 lines**; `.is-new .title` takes `width: fit-content`, or `-webkit-box` stretches the ember underline past the text it is supposed to be sized to. - `.libswitch a` reaches a real 44px under `(pointer: coarse)` — padding plus an 11px line landed at 43. ## /admin - Overview routes into Lanes when a lane is unhealthy, prefixes each figure with its column word on the phone layout that drops the `thead`, labels state cells for a screen reader, and has an empty state where the sites table previously assumed rows. - The admin shell picks up the library's chrome: htmx 15s timeout, the shared `#notice` slot, `#sr-announce`, `filter.js`. - `admin_render_test.go` and `card_render_test.go` render the templates directly, so markup regressions in either surface fail without a browser. ## Login `DISCORD_GUILD_NAME` (optional) names the community on the login screen and in the refusal message, so a stranger knows which Discord to ask for an invite. Unset degrades to a generic label. Neither form names the numeric guild id — that was never actionable, and the gate still reveals nothing about whether a given guild exists. ## Handlers `maxChapterNum` (9999) now bounds **both** typed-chapter paths. `uiChapter` and `adminSeriesCorrectLatest` each parsed a `float64` with no ceiling, so a hand-rolled POST stored `1e308` and every later reader of that row — the poller's `HasNewChapter` comparison, the display string — inherited it. Matches the `max` on the card's chapter input. The API PUT path is deliberately untouched: it carries the userscripts' own scraped numbers, not typed input. ## Verification - `cd backend && go test ./...` green (Docker-backed `pgtest`). `TestChapterOverrideRejectsBadInput` gained `"10000"` and `"1e5"` — both parse fine as `float64`, so they only fail if the bound exists. - Visual: 390×844 dark + light, 1000px and 1440px (`zoom: 1.2`) desktop, against the real templates + real CSS. Measured `chromeTop = 0` at `scrollY 950`, `2px solid rgb(242,236,229)` rings, `content: "Saving…"` at `opacity: 1` after 2.4s, `aria-busy` `true` during / cleared after, `libswitchH = 44` in a `hasTouch` context, no horizontal overflow at either width. - `detect.mjs` on `templates/`: `[]`, exit 0. ## Note on shape The three strands landed as one commit because `admin_series.go` and `style.css` each carry hunks from more than one of them; splitting cleanly would have needed hunk-level surgery. Say the word if you want it split before merge. Reviewed-on: #177 Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com> Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
117 lines
16 KiB
Markdown
117 lines
16 KiB
Markdown
---
|
||
target: Reader library web surface (app.html)
|
||
total_score: 32
|
||
max_score: 40
|
||
na_heuristics:
|
||
p0_count: 0
|
||
p1_count: 2
|
||
timestamp: 2026-08-27T15-38-17Z
|
||
slug: backend-internal-web-templates-app-html
|
||
---
|
||
Method: dual-agent (A: CritiqueDesignReview · B: CritiqueDetectorEvidence)
|
||
|
||
Target: the Reader library surface — `backend/internal/web/templates/{app,chrome,list,card,setup,icons}.html`, `static/style.css`, `static/filter.js`. Mode: **Operate**.
|
||
|
||
## Design Health Score
|
||
|
||
| # | Heuristic | Score | Key Issue |
|
||
|---|-----------|-------|-----------|
|
||
| 1 | Visibility of System Status | 3 | `card.htmx-request::before` (style.css:880-889) animates a silent bar for up to the full 15s htmx timeout (app.html:12) with no "saving" text and no `aria-busy`. |
|
||
| 2 | Match System / Real World | 4 | Solid. Copy is situational, not generic: "Chapter you're on" vs "Latest known" (card.html:96-106); four distinct empty states (list.html:11-29). |
|
||
| 3 | User Control and Freedom | 3 | Confirm+Cancel and Esc-to-close cover accidental opens (filter.js:152-200); a committed Remove has no undo. |
|
||
| 4 | Consistency and Standards | 3 | `:focus-visible` rings are defined for every `.actions` cell, the chapter input and confirm buttons (style.css:703-826) but for **no** navigation control — `.tabs a`, `.libswitch a`, `.ghost`, `.login-card button`, `.empty .clear-search` fall back to the UA ring. |
|
||
| 5 | Error Prevention | 4 | `step="any"` chapter guard, confirm-gate on every move-out-of-list, `hx-disabled-elt` on every mutating control, Cancel focused on Remove (filter.js:159-164). |
|
||
| 6 | Recognition Rather Than Recall | 3 | The `.keyrow` legend (chrome.html:34-46) exists precisely because the 5-icon strip is unlabelled — and it scrolls away, at 10px, in exactly the long-list case it was built for. |
|
||
| 7 | Flexibility and Efficiency | 2 | No bulk actions, no sort, no shortcut beyond Esc. A 300-series library is triaged one card, two taps at a time. |
|
||
| 8 | Aesthetic and Minimalist Design | 3 | Hairline-only, one-measure, disciplined — but the minimalism is paid for in legibility: 9 distinct functional labels compute to **10px** at 390px (Assessment B), below any UI-text floor. |
|
||
| 9 | Error Recovery | 4 | `reasonFrom()` surfaces the server's literal 400 text (filter.js:270-292); distinct 401/400/network/timeout copy; auto-`scrollIntoView` to the message. |
|
||
| 10 | Help and Documentation | 3 | In-context `<details class="setup">` covers install and rotation; nothing in the library view explains what dim/italic, ember, or brass mean without first finding the keyrow. |
|
||
| **Total** | | **32/40** | **Good — solid foundation, weak areas are specific and cheap** |
|
||
|
||
## Design Specificity Verdict
|
||
|
||
**Authored, not assembled.** This could not ship unchanged on another product, and that is rare.
|
||
|
||
**LLM assessment (Assessment A, unanchored):** the specificity is load-bearing, not decorative. `--ember` is a contract, not an accent — every "new chapter" surface shares that one token (`.is-new .title` 604-610, `.tabs .tab-new` 409-412, `.foot-rule` 529-536, `.libswitch a.active` 663-667) and destruction is deliberately split onto a duller `--danger` family so a remove-confirm is never mistaken across the room for an unread chapter (style.css:77-83). The `:root` block carries per-pair contrast measurements, and the light branch **re-derives** every hue instead of reusing dark values (137-195) — evidence the "both branches touched together" law is honoured, not asserted. Three purpose-chosen faces, self-hosted because Bromite blocks `fonts.googleapis.com` (style.css:1-5). One `--measure: 760px` column, no cards, no shadows, and in 1075 lines exactly one stray corner (`.tabs a:hover` 2px radius, line 402). The gaps are not taste failures; they are unfinished passes — sticky positioning, focus rings on nav, and a legibility floor.
|
||
|
||
**Deterministic scan:** `detect.mjs --json backend/internal/web/templates` → **0 findings, exit 0**, and Assessment B validated that zero three ways (synthetic bad HTML fired 2 rules; `--no-config` reproduced the zero; no `config.json` exists). Note the scan's blind spot: the detector did **not** resolve the linked `/static/style.css`, so a clean markup scan says nothing about the tokens. The runtime pass is where everything was found.
|
||
|
||
**Visual overlays:** injection **succeeded** — but against a standalone render, not the live app. Assessment B found `web.go:125` parses templates with a plain `template.ParseFS` and no FuncMap, so it rendered the repo's real templates + real CSS from a throwaway program in `/tmp/bmrender`, served it, and injected `detect.js`. Overlay confirmed present; screenshots at `/tmp/bmrender/shots/` (390×844 light, dark, dark+overlay). **20 runtime findings, none of which the static scan could see:** `undersized-ui-text` ×18, `wide-tracking` ×1, `overused-font` ×1. Every card action measured **66.8 × 46 px** — clears 44px on both axes. No horizontal overflow at 390px (an earlier 499px reading was the overlay's own label boxes). Both servers stopped, no Docker started, no repo file changed.
|
||
|
||
False positives I accept from B: 9 of the 18 `undersized-ui-text` hits are the same `.hint` rule counted once per row inside `hidden` chapter forms — one defect, not nine. `overused-font` on Instrument Serif is the rule's share-of-text heuristic misreporting a deliberate display serif. `wide-tracking` anchors to the mono meta line, not body copy. **The remaining 9 are real and unanimous with A's typography read:** `Manga`, `Novels`, `Userscripts`, `Read`, `Fav`, `Chapter`, `Archive`, `Delete`, `Continue reading` all render at 10px.
|
||
|
||
## Overall Impression
|
||
|
||
This is the best-argued small codebase I have critiqued in a while — the CSS comments read like design review notes, and the taste is consistent. What works is the discipline: one heat signal, one column, hairlines, situational copy, and a confirm model that distinguishes reversible from final and even routes focus differently for each.
|
||
|
||
What does not work is that the design's own mitigations are undermined by two omissions it never got around to. The card strip needs the legend to be readable; the legend is 10px and scrolls away. The tab row is the primary navigation; it is the one row with no focus ring. Both are 20-line fixes.
|
||
|
||
**Single biggest opportunity: make the chrome persistent and lift the 10px floor.** Those two changes convert a well-reasoned desktop-shaped design into one that actually works one-handed at night on a phone — which PRODUCT.md says is the entire point.
|
||
|
||
## What's Working
|
||
|
||
1. **Ember law is real.** One reserved token for "unread chapter", nothing else borrows it, destruction gets its own duller family. This is why a dense icon-heavy list stays scannable to a tired reader: any warm colour means *something to read*. Most design systems claim this; this one enforces it in a comment and then actually holds the line for 1075 lines.
|
||
2. **Confirm-row focus routing.** `toggleConfirmRow` focuses the affirmative button for the reversible action (Archive) and **Cancel** for the irreversible one (Remove) — filter.js:159-164, paired with `.confirm-row.calm` grey vs the danger wash (style.css:786-826). A one-line detail that measurably lowers accidental destruction. Nobody notices this; everybody benefits from it.
|
||
3. **State coverage is exhaustive and honest.** Four distinct empty states by real cause, verbatim server 400 text surfaced inline instead of "something went wrong", a chapter form that disambiguates your progress from the published latest, and a cover slot with hatch + monogram + `onerror="this.remove()"` covering both "never acquired" and "404s after storage" (card.html:9-19). Verified in B's render: the broken-cover row hit `ERR_NAME_NOT_RESOLVED` and the monogram fallback executed.
|
||
|
||
## Priority Issues
|
||
|
||
**[P1] Nothing is sticky, on a mobile-first surface built for hundreds of series**
|
||
- **Why it matters:** `position: sticky` appears **nowhere** in style.css (verified). Casey, one-handed, three screens into a 300-item library, must scroll to the top to switch tabs, search, or re-read the icon legend — the exact moment search and legend earn their existence. This also silently defeats the keyrow, which is the whole mitigation for the unlabelled 5-icon strip.
|
||
- **Fix:** `.chrome` (searchbar + tabs) → `position: sticky; top: 0; background: var(--ink);` with `padding-top: env(safe-area-inset-top)` and a `z-index` above cards; keep `.recent`/`.keyrow` non-sticky if header height is tight, or sticky the keyrow instead and accept a shorter tab row.
|
||
- **Suggested command:** `/impeccable adapt`
|
||
|
||
**[P1] Navigation controls have no `:focus-visible`; nine functional labels render at 10px**
|
||
- **Why it matters:** two separate legibility/a11y regressions in the same layer. (a) Every `.actions` cell, the chapter input and confirm buttons define 2px token-coloured rings (style.css:703-826); `.tabs a`, `.libswitch a`, `.ghost`, `.login-card button` and `.empty .clear-search` define none, so keyboard focus on the primary navigation falls back to a UA ring — likely bright blue, against a palette explicitly tuned for low glare. `.search` goes further and zeroes its outline (372), delegating to a 1px `border-bottom-color` change — materially weaker than every other control in the file. (b) B measured 10px computed on `.libswitch a`, `.keyrow .pair span`, `.recent h2`, `.setup summary`, `.hint`. 10px mono, at night, on a phone, is not a minimalism choice a night-reading brief supports.
|
||
- **Fix:** add `:focus-visible { outline: 2px solid var(--paper); outline-offset: 2px; }` to the four nav selectors, matching the established pattern; give `.search` a real ring instead of the hairline delegation. Raise the 10px steps to 11-12px (the `min-width: 720px` block already does exactly this for `.keyrow .pair span` at line 1024 — the mobile branch is the one that needs it more, not less).
|
||
- **Suggested command:** `/impeccable audit` then `/impeccable typeset`
|
||
|
||
**[P2] Five icon-only actions per card, at rest, in both list states**
|
||
- **Why it matters:** `.actions` renders Play, Fav, Chapter, Archive/Restore, Remove (card.html:48-87) — one over the ≤4 working-memory ceiling, on every card. At rest all five are `var(--mute)` grey; per-action colour is a *response* to interaction, not a resting cue. The design already knows this, which is why the keyrow exists — and the keyrow is 10px and scrolls away (P1). Fix P1 and this drops to P3; leave P1 and this is five monochrome glyphs versus a distracted thumb.
|
||
- **Fix:** either resolve via P1 (sticky + legible legend) and keep five, or move Chapter-override behind the same disclosure affordance as Archive/Remove so the resting strip is Play, Fav, Archive, Remove.
|
||
- **Suggested command:** `/impeccable layout`
|
||
|
||
**[P2] Up to 15 seconds of silent busy state**
|
||
- **Why it matters:** `.card.htmx-request::before` animates a bar; there is no interim text, no `aria-busy`, and no live-region message until the outcome lands. app.html:8-12 documents the 15s cap and its own reasoning is "a phone that walks into a dead zone" — so the slow path is anticipated, and it is the one path with no feedback. To a distracted user this reads as "did my tap register?" for an uncomfortably long stretch; to a screen reader user there is literally nothing between the tap and the result.
|
||
- **Fix:** after ~2s of `htmx-request`, reveal a "Saving…" line in the card's existing `.error-inline` slot (or a sibling) and set `aria-busy="true"` on the article.
|
||
- **Suggested command:** `/impeccable harden`
|
||
|
||
**[P3] Long titles clamp in the strip but not in the list**
|
||
- **Why it matters:** `.recent-title` clamps to 2 lines (style.css:512-517); `.title` gets only `overflow-wrap: anywhere` (592-601) despite the adjacent comment documenting a real 90-character scraped title. Horizontal overflow is defended; a card growing four lines tall and breaking the list's rhythm is not. Titles are scraped `og:title` — the length is not under your control.
|
||
- **Fix:** apply the same `-webkit-line-clamp: 2` to `.title`, or decide the list shows titles in full and say why the two components diverge.
|
||
- **Suggested command:** `/impeccable polish`
|
||
|
||
## Persona Red Flags
|
||
|
||
**Casey (distracted, one-handed, mobile — the stated primary user)**
|
||
- No sticky chrome: every tab switch or search after scrolling is a full reach to the top of the viewport.
|
||
- 10px `Manga`/`Novels` library switch and 10px keyrow labels — the two things you glance at, set at the smallest size on the page.
|
||
- Five same-weight grey glyphs per card demand deliberate recognition, not a colour-coded glance.
|
||
- *Working:* `.actions { flex: 1 0 100% }` puts the strip full-width at the bottom of each card's content — correct thumb zone. Every cell measured 66.8 × 46 px.
|
||
|
||
**Sam (screen reader / keyboard / low vision)**
|
||
- `.tabs a` and `.libswitch a` have no custom focus ring; `.search:focus-visible { outline: none }` delegates to a 1px hairline recolour on the one control most in need of being findable.
|
||
- No `aria-busy`/live cue during the in-flight window — announcements exist before nothing and after everything.
|
||
- *Working, despite appearances:* cover `alt=""` + `aria-hidden` monogram is correctly decorative because the sibling `<h3 class="title">` carries the name; every icon button has both `aria-label` and `title`; mutations announce through `#sr-announce`.
|
||
|
||
**Riley (edge-case stress tester)**
|
||
- Chapter `max="9999"` is a client-only cosmetic guard — server validation (`web.go:564-569`) rejects negative/NaN/Inf but no upper bound, so a direct POST stores an arbitrary magnitude.
|
||
- `closeCardPanels` (filter.js:118-128) enforces one open panel *per card*, not per page: a confirm-remove can sit open on card 3 while a chapter form is open on card 40.
|
||
- 300 series: no pagination, no virtualization, no bulk action, no sticky nav — sequential two-tap cleanup only.
|
||
- *Working:* 0-series is handled with four distinct states including a true first-run with install links; missing covers are handled from both directions.
|
||
|
||
## Minor Observations
|
||
|
||
- `.keyrow .full { display: inline; }` (style.css:1025) targets a `.full` class that exists nowhere in `chrome.html` — dead rule, delete it.
|
||
- `.tabs a:hover { border-radius: 2px 2px 0 0; }` (402) is the only corner rounding in the sheet, an unexplained exception to the stated no-corners law.
|
||
- `.libswitch a` under `@media (pointer: coarse)` computes ~42px tall (1066-1068) against the file's own claim at 1061-1063 that "every control is already 44".
|
||
- Restore fires instantly while Archive is confirm-gated — defensible as "reversible needs no confirm", but Restore *is* the reversal of Archive, so the asymmetry deserves a sentence in the CSS comment where the rule is stated.
|
||
- The detector cannot see `style.css` through a `<link href="/static/...">`. Any future detector run on this repo must target rendered HTML in a browser, not the templates directory, or it will report clean and mean nothing.
|
||
|
||
## Questions to Consider
|
||
|
||
1. If a permanent legend is required to make the card strip legible, is 5 icon-only actions the right density for a phone — or is Chapter-override one disclosure away from a 4-icon strip that needs no legend?
|
||
2. Why does neither the tab row nor the search box stay reachable once a Reader is three screens into the library the Updated badge exists to triage?
|
||
3. Ember means "new chapter" everywhere except the wordmark and the login screen (style.css:238-241). Is a law with a carved-out exception still a law, or does the brand need its own accent instead of borrowing the one reserved signal?
|
||
4. The 10px steps read as confidence — restraint pushed to its limit. On a surface whose brief names night reading and glare as an explicit personal requirement, is that restraint or is it the one place taste overrode the brief?
|