Marcos Paulo
0a12e9cbcc
The 880px and 1050px media queries are in the built sheet and inert: Astro's scoped `.tree-node[data-astro-cid-...]` outranks a rule ported verbatim as `.tree-node`. Attempt 4 got everything else right and passed the acceptance test in this brief, which was mine to get wrong. Replaces that test with computed-style-diff.mjs, and records the finding it produced: main is already at 52 differences, because importing a global responsive.css into an Astro page never fully worked either. Tagged rejected/15e-attempt-4. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
262 lines
12 KiB
Markdown
262 lines
12 KiB
Markdown
# Task 15e — Retire responsive.css
|
|
|
|
**Agent**: `design-system-keeper` · **Model**: `agy` (Gemini 3.1 Pro — vision)
|
|
**Depends on**: 15d, 16 · **Blocks**: 20 **Worktree**:
|
|
`.agents/scripts/worktree.sh start 15e responsive-css`
|
|
|
|
## Goal
|
|
|
|
`responsive.css` is 30 KB and mostly served `/full-guide/`. Once 15d and 16 have
|
|
landed, port what the Astro pages still need into component styles or
|
|
`tokens.css`, prove the remainder dead, and delete it.
|
|
|
|
**Proof is screenshots, not reading.** A rule that looks unused because no
|
|
selector matches at 1600 px may be the only thing holding the 560 px layout
|
|
together.
|
|
|
|
## Method
|
|
|
|
1. Build. Screenshot every migrated route at 560 / 800 / 1100 / 1600 px.
|
|
2. Remove `responsive.css` from the build entirely.
|
|
3. Screenshot again. Every diff is a rule you must port.
|
|
4. Port it into the owning component's `<style>`, or — if it is a real token —
|
|
into `tokens.css`, which **you own**. No other agent may add tokens.
|
|
5. Repeat until the diffs are empty.
|
|
|
|
## Constraints
|
|
|
|
- Allowed breakpoints are 560 / 800 / 1100 / 1600 / 2200 px.
|
|
`.agents/scripts/check-tokens.mjs` rejects others.
|
|
- Do not delete the file while any legacy page still loads it. Check which
|
|
routes have actually been migrated at the time you run; 20 is the cutover.
|
|
- No raw hex, no `font-size: Npx` outside `tokens.css`.
|
|
- You are also the owner of the ~190 accumulated `/* token-gap: ... */` markers.
|
|
Resolving them is **not** in this brief — do not start. Report the count so it
|
|
can be scheduled.
|
|
|
|
## Done when
|
|
|
|
- [ ] Screenshot diffs empty at all four widths without `responsive.css`
|
|
- [ ] Ported rules live with the component that needs them, or in `tokens.css`
|
|
- [ ] `responsive.css` deleted, and nothing references it
|
|
- [ ] `pnpm run gate` green; 42 assertions intact
|
|
|
|
## Amendment — the screenshot step now actually works
|
|
|
|
When this brief was written, `.agents/scripts/visual-regression.mjs` threw
|
|
`playwright ... install its project dependency` for everyone: the browsers were
|
|
cached in `~/.cache/ms-playwright` but the node package was never a dependency.
|
|
Every "screenshots match" checkbox in this plan was unsatisfiable. `playwright`
|
|
is now a devDependency (`c2a046d`) and a headless screenshot has been verified
|
|
to work. **Take the screenshots.** "I read the rules and they look unused" is
|
|
not the proof this brief asks for.
|
|
|
|
All eight routes are migrated as of `edd03f9` — `index`, `models`, `agents`,
|
|
`skills`, `summary`, `rules`, `full-guide`, `skills-review`. Screenshot all of
|
|
them, not just `/full-guide/`.
|
|
|
|
## Amendment — the token-gap queue is already closed
|
|
|
|
The brief says "~190 accumulated `/* token-gap: ... */` markers … do not start".
|
|
Task 02c ran and took them from 154 to **17**. Do not reopen that work either;
|
|
the 17 that remain are deliberate.
|
|
|
|
Two things 02c got wrong that you must not repeat. It closed six markers by
|
|
pointing the value at a palette token with a _different_ value — `#5b7098` at
|
|
`var(--accent)` (`#7c78a8`), `#9eb0bb` at `var(--muted)` (`#697b89`) — after its
|
|
own marker comments had recorded "no token matches". That shipped a silent
|
|
redesign the gate could not see. `edd03f9` repaired it by adding exact on-dark
|
|
tokens: `--ink-muted`, `--ink-line`, `--ink-code`, `--accent-paper`,
|
|
`--accent-paper-active`, `--accent-surface`, and four exact-alpha overlays
|
|
`--white-14/23/25/31`. Use those when you need a colour on `--ink` or
|
|
`--accent`; the light-background palette is the wrong family there.
|
|
|
|
If you must port a colour or size that has no token, add an **exact** one to
|
|
`tokens.css` — you own it — or leave the raw value under a `token-gap:` marker.
|
|
Never round to a near neighbour.
|
|
|
|
## Amendment — how this gets verified
|
|
|
|
The gate does not read your output: all 42 assertions in `scripts/verify.mjs`
|
|
read the legacy files, so deleting `responsive.css` and breaking every mobile
|
|
layout passes it cleanly. Before you claim done, diff the **built** CSS
|
|
(`dist/_astro/*.css`) against `main` and enumerate every declaration that
|
|
disappeared. Each one is either present elsewhere or a regression.
|
|
|
|
## Attempt 1 stopped short — and the goal as written is not reachable yet
|
|
|
|
Preserved as `rejected/15e-attempt-1` (`d2312d7`). Start by reading that diff:
|
|
it ported rules into `ChapterHero`, `FleetDiagram`, `RouteTable`, `WorktreeMap`,
|
|
`LanguageToggle`, `ReadingProgress`, `agents.astro`, `full-guide.astro` and
|
|
`tokens.css`, and it added `pixelmatch`, `pngjs` and `postcss` plus a working
|
|
screenshot-compare harness. Reuse all of it. It never finished the comparison
|
|
loop and never committed on its own.
|
|
|
|
It also deleted `responsive.css` while **`full-guide/index.html` still links
|
|
it**. That page is the live site until task 20 cuts over; deleting the
|
|
stylesheet strips it. `src/components/islands/GuideSelector.astro` still
|
|
references it too.
|
|
|
|
So the "delete it" checkbox in this brief cannot be honestly ticked before the
|
|
cutover. Do not tick it, and do not delete the file. **The deliverable is
|
|
everything up to the deletion:**
|
|
|
|
- Every rule `responsive.css` provides to an Astro route lives with the
|
|
component that needs it, or in `tokens.css`.
|
|
- No `.astro` file and no Astro layout links `responsive.css` any more.
|
|
- `full-guide/index.html` still links it, unchanged, and still renders exactly
|
|
as it does today.
|
|
- A list, in this file, of the rules that remain in `responsive.css` solely for
|
|
the legacy page — that list is the deletion checklist task 20 will execute.
|
|
|
|
Amend the "Done when" boxes to match before you start, and say in your final
|
|
report that the file is intentionally still present.
|
|
|
|
### One rule to port properly while you are in there
|
|
|
|
`.route-meter span` animates `height: var(--score)` — a layout property, which
|
|
task 18 was meant to eliminate. 18 changed it in `responsive.css` and that
|
|
change was reverted (`0bca43c`) because it belongs in the component, not in a
|
|
legacy file. When you port this rule, port it as
|
|
`transform: scaleY(var(--score)); transform-origin: bottom` with `height: 100%`.
|
|
`--score` is set inline as a percentage (`--score:92%`) by `GuideSelector.astro`
|
|
and `full-guide.astro`; a percentage in `scaleY()` is valid and was verified
|
|
rendering correctly at `matrix(1, 0, 0, 0.92, 0, 0)`. Leave the legacy
|
|
`responsive.css` copy of the rule alone.
|
|
|
|
## Attempt 3 rejected: two breakpoints were replaced with dead tokens
|
|
|
|
Tagged `rejected/15e-attempt-3` (`d5f5514`). The gate passed with 84 assertions
|
|
intact, and the work is still wrong. Recover anything reusable from the tag; do
|
|
not build on it.
|
|
|
|
**The disqualifying defect.** `responsive.css` carries
|
|
`@media (max-width: 880px)` — which collapses `.verify-layers` and
|
|
`.verify-cta-grid` to one column and `.verify-intro` to a single column — and
|
|
`@media (max-width: 1050px)`, which narrows `.tree-node` to 145px, puts
|
|
`.tree-detail` on two columns, and collapses `.skill-explorer`. Neither was
|
|
ported. Both are absent from the built CSS:
|
|
|
|
```
|
|
grep -oh 'max-width:[0-9]*px' dist/_astro/*.css | sort -u
|
|
```
|
|
|
|
returns every other breakpoint and not those two. On the built site the
|
|
verification section stays multi-column down to 560px and the worktree tree
|
|
never narrows.
|
|
|
|
What made it look finished is worse than the omission. `audit-ui.mjs` compares
|
|
the built CSS against `.agents/snapshots/built-css-values.json`, so dropping
|
|
those two media queries fails the value contract. Attempt 3 answered that by
|
|
adding
|
|
|
|
```css
|
|
/* Retained as exact legacy dimensions for the built-CSS value contract;
|
|
neither is used as a media query. */
|
|
--legacy-audit-width-880: 880px;
|
|
--legacy-audit-width-1050: 1050px;
|
|
```
|
|
|
|
Two variables, referenced by nothing, whose only function is to put the strings
|
|
`880px` and `1050px` back into the built sheet so the check that exists to catch
|
|
this exact loss reports success. The comment says so outright. **Port the media
|
|
queries. Never satisfy a value contract with a value nothing uses** — if a check
|
|
is in the way, the check is telling you something.
|
|
|
|
**Three more things to fix on the next attempt.**
|
|
|
|
1. **No `--raw-<hex>` tokens.** Attempt 3 added roughly fifty of them:
|
|
`--raw-9eabb4: #9eabb4`, `--raw-ffffff2d: #ffffff2d`, and so on. A token
|
|
named after its own value carries no meaning, so it is not a token — it is a
|
|
lookup table that satisfies `check-tokens.mjs` while defeating the point of
|
|
having one. If a ported rule needs a colour that the palette has no name for,
|
|
either it is one of the existing tokens (use it) or it is a genuine gap (name
|
|
it for its role, or leave the literal and add a token-gap marker — see
|
|
`.agents/rules/theming.md`).
|
|
|
|
2. **Leave `--white-31` alone.** Attempt 3 rewrote it from
|
|
`rgb(255 255 255 / 31.3725%)` to `#ffffff50` and added a second
|
|
`--white-31-alpha` holding the original. The two are numerically equal; the
|
|
churn reverses task 02c, which set these six on-dark tokens to their exact
|
|
values on purpose, and leaves a duplicate behind.
|
|
|
|
3. **Do not commit screenshots.** Attempt 3 added 128 PNGs under
|
|
`.agents/snapshots/15e-before/` and `15e-after/`, 35 MB, to a repository
|
|
whose entire pack is under 400 KB. Take them, compare them, report the
|
|
comparison, and leave them out of the commit.
|
|
|
|
**Acceptance, restated.** Every breakpoint in `responsive.css` that serves an
|
|
Astro route appears in the built CSS, in a rule that does the same thing it did
|
|
before:
|
|
|
|
```
|
|
diff <(grep -oh '(max\|min)-width:[0-9]*px' responsive.css | sort -u) \
|
|
<(grep -ohE '(max|min)-width:[0-9]*px' dist/_astro/*.css | sort -u)
|
|
```
|
|
|
|
Anything only the legacy page needs may be missing from the built sheet — say
|
|
which, and why, in the final report.
|
|
|
|
## Attempt 4 rejected: the ported rules are in the sheet and do nothing
|
|
|
|
Tagged `rejected/15e-attempt-4` (`b6b6200`). This attempt fixed everything
|
|
attempt 3 got wrong — the 880px and 1050px media queries are ported, the tokens
|
|
are named for their roles rather than their hex values, `--white-31` is
|
|
untouched, `responsive.css` is unchanged and still linked by
|
|
`full-guide/index.html`, no `.astro` file imports it, the route-meter is
|
|
`transform: scaleY()`, the four off-scale breakpoints carry honest token-gap
|
|
markers, no screenshots were committed, and the gate passes with 84 assertions.
|
|
The acceptance test in this brief passes on it.
|
|
|
|
**It is still wrong, and the acceptance test was the problem.**
|
|
|
|
Astro scopes a component's styles. `WorktreeMap.astro`'s rules compile to
|
|
`.tree-node[data-astro-cid-lsutp3lb] { width: 180px }`, specificity 0,2,0. A
|
|
rule ported verbatim out of `responsive.css` arrives as
|
|
`@media (max-width: 1050px) { .tree-node { width: 145px } }`, specificity 0,1,0.
|
|
The breakpoint is in the built sheet, the media query matches, the selector
|
|
matches — and the declaration loses. At 1050px the node stays 180px wide.
|
|
Measured:
|
|
|
|
```
|
|
node .agents/scripts/computed-style-diff.mjs full-guide --widths 880,1050
|
|
```
|
|
|
|
56 differences on attempt 4. Diffing breakpoints between `responsive.css` and
|
|
`dist/_astro/*.css` cannot see any of it, which is my error, not the agent's: I
|
|
wrote that test.
|
|
|
|
**Port rules into the component that owns the selector**, so they compile with
|
|
the same scope as the base rule they are overriding. A rule that has no owning
|
|
component belongs in a global sheet, but then the base rule it overrides has to
|
|
be global too.
|
|
|
|
### The baseline is not zero, and that is the real finding
|
|
|
|
The same command reports **52 differences on `main`**, before this task changes
|
|
anything. `full-guide.astro` currently imports `responsive.css`, and that import
|
|
has never fully worked, for the identical reason: a global stylesheet cannot
|
|
override scoped component styles. The responsive layer has been partly inert in
|
|
the Astro build for as long as it has been imported, on `.tree-node`, `.branch`,
|
|
`.worker-card`, `.tree-stage`, `.tree-lab`, `.hero`, `.fleet`, `.chapter-links`
|
|
and more.
|
|
|
|
So this task is not "keep parity". Parity is already broken, and porting the
|
|
rules properly is what fixes it.
|
|
|
|
### Acceptance, restated again
|
|
|
|
```
|
|
node .agents/scripts/computed-style-diff.mjs full-guide
|
|
```
|
|
|
|
- Must report **fewer than 52** differences at 880px and 1050px — that is the
|
|
`main` baseline, and anything above it is a regression.
|
|
- Drive it to **zero** where you can. Every difference you leave must be listed
|
|
in your final report with the reason it is not fixable inside this task's
|
|
scope.
|
|
- Keep everything attempt 4 got right; the list above is not a set of problems,
|
|
it is the standard to match.
|
|
- The breakpoint diff against the built CSS stays as a _necessary_ check. It is
|
|
no longer a _sufficient_ one.
|