feat(design-system): close gate 2's h-10 case with a ratcheted static check (#265) - #1984
Conversation
… check (#265) Gate 2 named two gaps: the rendered interactive enumeration, and fixed-height `h-10` controls. The first landed on main as 885c613 (PR #1962) — the deterministic phone-viewport min-h-tap audit on /forms. This closes the second. - New `interactiveTapFloorDeclarations` metric in check:design-system-contract: an interactive element (a, button, input, select, summary, textarea) that declares its OWN unprefixed `min-h-*` below the 48px token. Ratcheted at 41 across 17 files with per-path pins, so a new sub-floor control anywhere in src/** fails today on both the total and its own path. - Mutation-verified rather than assumed. Lowering one shortlist button to min-h-9 produced exactly: interactiveTapFloorDeclarations increased from 41 to 42 interactiveTapFloorDeclarations at src/components/services/services-navigator-page.tsx increased from 0 to 1 An earlier mutation attempt on a <Link> did NOT trip it, which is how the component-wrapper blind spot below was found rather than shipped unnoticed. - Scoped to min-h-* and deliberately not h-*/size-*. A short h-4 on an interactive element is routinely the visible box of a control whose hit area belongs to a tap-sized wrapper — SelectionCheckbox in differentials-home.tsx is exactly that, and ui-smoke asserts its label still meets the floor. Flagging those would pad the baseline with non-defects, which GATES.md §5 names as how a gate ends up switched off. A first measurement including h-*/size-* returned 48 findings; the principled min-h-* rule returns 41. - Unit coverage for the detector, including the negative cases: the repo's correct `min-h-12 sm:min-h-10` responsive release must not be flagged, and neither must a short height on a non-interactive element. Corrections landed alongside, all of which cost a reader time: - Gate 8's evidence cell said "27 conflicts across 15 files". The baseline says 25 across 12 and GATES.md §3 already said 25; PR #1942 paid three files down without updating this prose. Per docs/design-system/README.md the baseline outranks the document, so the prose was the defect. - legacyShadowAliases was pinned at 119 while measuring 118 on clean main — one unit of stale slack, found while working #163 and tightened here. - Gate 4's Status cell still read implemented-partial although its own evidence cell and the §3 row both declared it closed on 9 Aug. Gate 8 is NOT closed here and the reason is recorded in the ledger: its 25 conflicts are mostly focus:ring-4 indicators co-existing with a border, not the decorative double edges the rule was written for, so retiring them is a focus restyle across ~17 controls needing browser proof this container cannot produce. Gate 7 is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYphQZmsBBeqnidpSnAtjE
- Branch review record for 6de09c4. - issues:update (not done — #265 is only part-closed): gate 2 closed, gates 7 and 8 still open, with the stale 27/15 and 10-rows figures corrected and the gate-8 stop reason recorded in enough detail to pick up cold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XYphQZmsBBeqnidpSnAtjE
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 15 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddde893032
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review-and-fix summary
Codex did not merge or modify auto-merge. The existing merge workflow completed the PR. |
2 participants
Summary
Gate 2 named two gaps: the rendered interactive enumeration, and fixed-height
h-10controls. The first landed onmainas885c613(PR #1962) — the deterministic phone-viewportmin-h-tapaudit on/forms. This PR closes the second, and corrects three figures that were costing readers time.interactiveTapFloorDeclarationsmetric incheck:design-system-contract: an interactive element (a,button,input,select,summary,textarea) that declares its own unprefixedmin-h-*below the 48px token. Ratcheted at 41 across 17 files with per-path pins, so a new sub-floor control anywhere insrc/**fails today on both the total and its own path.min-h-9produced exactly:Worth reporting: an earlier mutation attempt targeted a
<Link>and did not trip the check. That is how the component-wrapper blind spot got found and documented rather than shipped unnoticed — the walker sees intrinsic lowercase tags only, and the pre-existinglegacyTapClassescheck has the same hole.min-h-*, deliberately noth-*/size-*. A shorth-4on an interactive element is routinely the visible box of a control whose hit area belongs to a tap-sized wrapper —SelectionCheckboxindifferentials-home.tsxis exactly that, andui-smokeasserts its label still meets the floor. Flagging those would pad the baseline with non-defects, whichGATES.md§5 names as how a gate ends up switched off. A first measurement includingh-*/size-*returned 48 findings; the principledmin-h-*rule returns 41, and I spot-checked the flagged sites (a 24px "show more tags" button, a 40px calculator reset with no responsive release) to confirm they are genuine.min-h-12 sm:min-h-10release must not be flagged, and neither must a short height on a non-interactive element.Corrections landed alongside:
GATES.md§3 already said 25 — PR Re-land the --shadow-tight retirement and close three design-token debt rows #1942 paid three files down without updating the prose. Perdocs/design-system/README.md's source ranking the baseline outranks the document, so the prose was the defect. Ledger#265carried the same stale figure and is corrected too.legacyShadowAliaseswas pinned at 119 while measuring 118 on cleanorigin/main. Verified by running the checker against an unmodified tree while working#163. Tightened to the measured value, closing one unit of stale ratchet slack.implemented-partialalthough its own evidence cell and the §3 row both declared it closed on 9 Aug.What is deliberately NOT in this PR
Gate 8 is not closed, and the reason is worth reading before someone assumes it was skipped for time. I inspected all 25 conflict sites. They are not 25 mechanical one-line edits:
focus:ring-4focus indicators co-existing with a border on inputs, selects and textareas (master-search-header5,formulation-builder3, plus singles acrossformulation-compare,dashboard-nav, favourites, specifiers,DocumentTagCloud,ui-primitives). Retiring those means restyling focus fromringtooutlineacross roughly 17 controls — an accessibility-visible change that needs focus-state proof in a browser.pwa-lifecycle's sharedcardClassName(border+ a persistentring-1 ring-[color:var(--ring-glass)]) is the decorative double edge the rule was written for, and that is one recipe counted five times.chromium-1194against a pinned1234,#255/#312), so that proof was unavailable; 8 of the 25 sit in files that open PRs Polish the responsive PWA install lifecycle #1976, feat(services): progressive referral rail and compact result rows (#163) #1982 and feat(favourites): search-led workspace, no ModeHome (#164) #1983 are currently editing; and pinning at zero without first widening theonePixelShadowSpreadsproperty filter would be a partly false close —design-system-contract-utils.mjsmatches--e[0-4]and--shadow-*but not--glow-primary,--glow-softor--ring-hairline, all of which carry0 0 0 1px.Gate 7 is untouched, as its own scope note in
#265describes. All of this is recorded in the ledger update so the next session starts from the finding rather than re-deriving it.Verification
npm run verify:pr-localEvery other metric was asserted unchanged before the baseline was rewritten, per the
GATES.md§5 discipline. The only two deltas were the new metric (41) and thelegacyShadowAliasestightening (119 → 118) described above.UI verification not run, and no browser gate is claimed:
check:playwright-browser-revisionfails closed here (chromium-1194vs pinned1234), and pointingPLAYWRIGHT_CHROMIUM_EXECUTABLE_PATHat the stale shell is forbidden byAGENTS.md. This PR adds no rendered UI — the change is a static analyzer, its baseline, its unit tests, and documentation — so the static gates above are the correct and sufficient coverage for it.Risk and rollout
min-h-*only) exists precisely to avoid that, and the negative-case unit tests pin it. The opposite risk — a false negative — is real and documented: component-wrapper tags are invisible to the walker.Notes
#293is resolved and needs no action here:885c613is onmain, and PR docs(issues): close #293 and #312 — confirmed merged on main #1977 is already open to close that row. I deliberately did not queue a second close for it.Generated by Claude Code