fix(a11y): pin SPRAWL's chip back to its corner, and guard the sp-* scale - #291
Merged
andraderaul merged 1 commit intoAug 1, 2026
Conversation
…cale PR #288 described this review round in its body but merged without it. This lands the code the description already promised. SPRAWL's basemap chip named `absolute` before `TOUCH_TARGET_HEIGHT`, and the constant opens with `relative` to anchor its overlay. `cn` is tailwind-merge, which resolves a position conflict in favour of the last name — so the chip lost its corner and fell back into the flow with every target class present and correct, which is exactly why the className assertion sailed past it. The constants now document the ordering, and a test pins the position rather than the classes. The scale guard gains the `sp-*` spellings, step by step rather than by a prefix rule: it is a second scale under the same utilities and its ends do not match the base one, so `gap-3xl` is real while `p-sp-3xl` renders nothing, and only the step tells the two apart. The completeness test holds the new entries to the preset the same way it holds the old ones. `TOUCH_TARGET_ICON` joins the public surface — the height overlay plus the real width it tells you to pair with — after both canvases composed the pairing identically. Two callers, which is ADR 0014's bar. The target tests now assert against the constants instead of the classes they happen to expand to, so respelling a target cannot red them without a regression behind it. Also recorded rather than silently done: the byte-identical range-input stylesheets, and the `sp-*` row in CLAUDE.md's scale table. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
andraderaul
added a commit
that referenced
this pull request
Aug 1, 2026
…the formatter (#290) * fix(ci): let the vendored snapshot stay compact instead of losing to the formatter CI has been red on main since #289. The vendor bot serializes the dataset with a bare `JSON.stringify` — compact by an explicit decision recorded on `serialize`, since pretty-printing triples the committed size of a 1961-point array for a diff git shows either way — and biome's formatter rejects the single line that produces. The bot opened its PR, the PR merged, and CI went red on a diff no human wrote. Two rules disagreeing, so one has to give. The formatter yields: a vendored, machine-generated artifact is not source, and `dataset-*.json` leaves `files.includes`. The alternative — indenting the generator — would have overturned a decision the script documents, and reformatted 2700 lines to do it. Nothing about the data moves. `dataset-2026-07.json` is pretty-printed because a human reformatted it by hand once; both spellings are now legal and the bot's own output no longer needs that step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(a11y): pin SPRAWL's chip back to its corner, and guard the sp-* scale (#291) PR #288 described this review round in its body but merged without it. This lands the code the description already promised. SPRAWL's basemap chip named `absolute` before `TOUCH_TARGET_HEIGHT`, and the constant opens with `relative` to anchor its overlay. `cn` is tailwind-merge, which resolves a position conflict in favour of the last name — so the chip lost its corner and fell back into the flow with every target class present and correct, which is exactly why the className assertion sailed past it. The constants now document the ordering, and a test pins the position rather than the classes. The scale guard gains the `sp-*` spellings, step by step rather than by a prefix rule: it is a second scale under the same utilities and its ends do not match the base one, so `gap-3xl` is real while `p-sp-3xl` renders nothing, and only the step tells the two apart. The completeness test holds the new entries to the preset the same way it holds the old ones. `TOUCH_TARGET_ICON` joins the public surface — the height overlay plus the real width it tells you to pair with — after both canvases composed the pairing identically. Two callers, which is ADR 0014's bar. The target tests now assert against the constants instead of the classes they happen to expand to, so respelling a target cannot red them without a regression behind it. Also recorded rather than silently done: the byte-identical range-input stylesheets, and the `sp-*` row in CLAUDE.md's scale table. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#288 describes this review round in its own body — the
sp-*guard,TOUCH_TARGET_ICON, the position find. None of it is inmain. The description shipped; the code did not. This lands it.The one that is a real defect
TOUCH_TARGET_HEIGHTopens withrelative, because an::afteroverlay needs something to anchor to. SPRAWL's basemap chip named its ownabsolutebefore the constant:cnis tailwind-merge, which resolves a position conflict in favour of the last name. So the chip lostabsoluteand fell out of its corner back into the flow — with every target class present and correct, which is exactly why the className assertion sailed past it. Order swapped, and the new test asserts the position (absolutepresent,relativeabsent) rather than the target classes, because that is the axis that broke.Never shipped:
sprawl-touch-targets.mdis still unreleased, so the bug lived only between #288 and here. Both constants now carry the ordering rule, and the changeset spells it out for anyone reaching for them next.Every other caller positions via a parent — I checked all seven — so this was the only exposed site.
The
sp-*hole in the scale guardsp-*is a second scale under the same utilities, and its ends do not line up with the base one:2xs→3xlsp-*macroxs→2xlSo
gap-3xlis real,p-sp-3xlrenders nothing, and sharing the utilities is precisely what makes the second easy to reach for. The deny list now carries thesp-spellings step by step rather than by a prefix rule — the step is the only thing telling the two apart, so a prefix rule would either missp-sp-3xlor falsely rejectp-sp-2xl.The list is composed from two named halves now (
EXTRAPOLATED_STEPS×SECTION_ONLY_GAPS) instead of one hand-written array, and the completeness test holds the new entries to the preset exactly as it holds the old ones — definesp-3xlfor real and the list has to give it up. That test also stopped defaulting to{}: a scale whose shape moved now throws by name rather than quietly comparing against nothing, which passes while proving nothing.Dropped
2sm/2md/2lgwhile here — the multiplier prefix only ever attaches toxs/xl, nobody writes those, and a deny list is read less the more of it is theatre.TOUCH_TARGET_ICONBoth canvases had independently composed
TOUCH_TARGET_HEIGHT+min-w-[44px] inline-flex items-center justify-center. Two real callers is ADR 0014's bar, so it becomes one export. Its siblingTOUCH_TARGET_OVERLAYstays module-local — still one caller.The asymmetry is the point and is documented on it: height is an overlay, width is real. A centred overlay wide enough for a ~27px icon reaches into its neighbour's across a
gap-xs, and two overlapping targets are worse than one small target, because the press that lands stops being the one the user aimed at.Tests assert the constant, not its expansion
Five target tests compared against string literals like
after:h-[44px]. They now match against the exported constant, so respelling a target cannot red them without an actual regression behind it. SPRAWL's is the deliberate exception — it filtersrelativeout, since the chip's ownabsolutedisplaces it and either one anchors the overlay.Two ASCII/GLITCH tests that asserted the same element twice collapsed into one each.
Changeset bumps corrected
deck-kit-a11y-blockersanddeck-kit-tablist-and-alertswere markedminorfor what are pure fixes — no API moved. Both are nowpatch.deck-kit-touch-targetsstaysminorand absorbsTOUCH_TARGET_ICON, so deck-kit's net bump is unchanged; only the changelog reads honestly now.Recorded rather than silently done
input[type=range]stylesheets in ASCII and GLITCH. Extracting only the newest rule would split one control's styles across two files — worse than either end.tokens.cssis:root-only by its own decision, so element rules need a second stylesheet export and every app's import order re-checked. Its own change.CLAUDE.md's scale table splitsp-*onto its own row, since the shared-utilities-different-ends trap is the whole reason the guard needed widening. deck-kit'sCONTEXT.mdgets the matching note.TabStrip's roving-focus JSDoc →//. It sat above aconstarrow inside a component body, so it was surfacing as that local's hover text.Verification
npm run check— clean, 291 filesnpm run typecheck— clean across all 5 workspacesnpm run test:run— 2120 passed (286 + 363 + 436 + 92 + 943)npm run build— all three apps shipmin-width:44pxandheight:44pxin their built CSS, which is what proves the new classes survive the purge🤖 Generated with Claude Code
Why this PR shows no CI
ci.ymlgatespull_requestintomainandfeat/**only. Its comment already anticipates stacks — "without it the sub-PRs of a stack land entirely unchecked" — but the allow-list coversfeat/**, and this stack isfix/**. So Build and Test will not report here until #290 merges and GitHub retargets this tomain.Not a reason to merge it unchecked — the same commands CI runs are green locally on this exact branch, listed under Verification above. Worth deciding separately whether that allow-list should be
[main, 'feat/**', 'fix/**'], or just the base-branch wildcard.