-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5414
HelloOjasMutreja (contributor, fork) · OPEN, approved + auto-merge armed · view on GitHub
Verdict: approve and merge
Head reviewed: 1c20165bcf410c5eb7f15657fdf0f24c1cbdff25.
Reading a code sample, you lose the brackets, commas, semicolons and the line-number
gutter — all painted in the disabled-control grey WCAG exempts precisely because a
disabled control is not meant to be read. 2.03:1 on raw defaults, down to 1.94:1 in
a container="section" block.
The shared default stops borrowing --color-text-disabled; neutral, chocolate and
matcha get dedicated pairs since they define their own palette. The other 207 lines
are the guard, the changeset and the generated CLI mirror — evidence, not decisions.
Everyone on the next release, both schemes. Punctuation and gutter become readable
while staying visibly quieter than identifiers. Butter, gothic, stone and y2k do not
move. All 16 cells pass after, including the container="section" variant the guard
does not measure.
No API change. Theme targets: none — only the value behind an existing token.
- API — no. Visual — yes, deliberate.
- Theme — a user theme with no syntax palette inherits secondary instead of disabled; all seven first-party themes define their own, so none is caught.
Nothing. Four static token values; the guard never ships.
Real Chromium, bare story iframe, all opened with the read tool:
shots-rG-5414/cb/{before,after}__neutral__{dark,light}.png, plus
__stone__dark.png showing an already-passing theme untouched.
Measured (6100 vs 65414): none 2.03/4.01→5.99/8.55 · chocolate
3.06/2.56→4.84/6.12 · matcha 3.83/2.73→5.19/7.02 · neutral
2.42/2.53→4.89/7.57 · butter/gothic/stone/y2k unchanged and passing.
container-section: neutral dark 1.94→5.79. Every number in the PR body
reproduced exactly.
Low-risk on all three tests; a deliberate recolour, so not an unattended post.
approve and merge — no findings. Three things looked for and not found: the
container="section" hole (measured, none); CLI template drift (generated and
guarded by check-cli-theme-bundle.test.mjs, byte-identical); a second edit for the
no-theme path (highlightStyles.ts derives its :root block from
syntaxTokenDefaults).
One system note, deliberately not raised on the PR: the guard never reads
syntaxTokenDefaults, so the widest-reaching line is the one it cannot catch a
regression in — but check-badge-contrast.test.mjs, the pattern it was asked to
follow, has the same shape. Ours to close for both, if at all.
Thanks — reproduced every number from real pixels: seven themes, both schemes, and transparent-background
container="section". gonzoblasco's two points are settled. Merging once Vercel clears.
Posted as drafted.