style(design-sweep): the seven shared-style items (session 1B) - #93
Conversation
…p backlog One pass over globals.css and DESIGN.md, per the 1B dispatch. No .tsx touched. - .page--narrow caps its children via `> :where(*)` instead of the column, so every route's page box is one width and the H1 stops moving 144px sideways on a nav click. - .stack > :where(.field) stretches stacked form fields to --measure; adds .field--short and the missing .field:focus-visible. - scroll-margin on sticky-header and pinned-column targets (WCAG 2.4.11). - Tier B batch: img/svg height:auto; --rule -> --rule-strong on three control boundaries; :not(:disabled) on pressed/current buttons; .dim-ink; a glyph marker as the non-colour second signal on selected chips; .btn--quiet folded into the 28px in-row grade; .row-toggle given a 28px target. - A label register: thirteen selectors' duplicated mono/uppercase/600 folded into one rule, plus four --track-* tokens. - --signal-warn retuned to hue 50, away from --gold/--tier-flygd in OKLab. - One 64rem stop for the audit table, closing the unserved 641-1040px band. Three e2e specs guard the geometry: the page column's rect on every route, the audit table at 768/1280, and focus clearing the pinned header.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR updates design tokens, label typography, page-column geometry, focus visibility, control hit targets, sticky-table behavior, responsive audit tables, and related end-to-end tests. ChangesDesign, accessibility, and responsive layout
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
…-1b-styles # Conflicts: # src/app/globals.css
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/globals.css (1)
1121-1129: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSet
.stto weight 600.Line 1127 changes the tracking token but leaves
.stwithoutfont-weight. The shared register intentionally excludes.st, so normal contexts continue to render it at 400. The design specification identifies that result as a defect.Proposed fix
.st { display: inline-flex; align-items: center; gap: 0.4em; font-family: var(--font-mono), ui-monospace, monospace; font-size: var(--t-label); + font-weight: 600; letter-spacing: var(--track-value); text-transform: uppercase; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/globals.css` around lines 1121 - 1129, Update the `.st` CSS rule to explicitly set `font-weight` to 600, alongside its existing typography declarations, so it renders with the specified weight independently of the shared register.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/audit.spec.ts`:
- Around line 514-518: Extend the viewport-width cases in the audit test loop to
include both boundary values 1024 and 1025, alongside the existing 768 and 1280
cases. Keep the current narrow calculation and assertions unchanged so the test
explicitly covers both sides of the declared 64rem breakpoint.
In `@src/app/globals.css`:
- Around line 246-263: Update the shared label typography selector by removing
`.btn-row__stamp`, then add a local `.btn-row__stamp` rule with the existing
mono font and label size but font-weight 400 and no text transformation,
preserving its timestamp value styling.
- Around line 28-38: Adjust --signal-warn in the globals.css color-token
definition so its distance from identity is at least its distance from
--signal-bad, then update the accompanying comment’s numeric distances and claim
to match the resulting color. If the criterion is intentionally not enforced,
revise the identical acceptance criterion and claim in DESIGN.md instead.
---
Outside diff comments:
In `@src/app/globals.css`:
- Around line 1121-1129: Update the `.st` CSS rule to explicitly set
`font-weight` to 600, alongside its existing typography declarations, so it
renders with the specified weight independently of the shared register.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d7ba622f-01d2-40fa-86f6-c7ff4c4dc0d6
📒 Files selected for processing (5)
DESIGN.mde2e/admin.spec.tse2e/audit.spec.tse2e/shell.spec.tssrc/app/globals.css
…orrect two claims The 64rem stop closed 641-1024 and left 1025-1049 forcing up to 17px of horizontal scroll -- the same defect, in a window no hand-picked test width landed in. Widened to 66rem and added the boundary cases that catch it. Also: --signal-warn's comment drew a conclusion its own numbers contradict, and .btn-row__stamp is a value, not a register label.
Session 1B of the design sweep: the seven shared-style items, in one pass over
src/app/globals.cssandDESIGN.md. No.tsxis touched — session 1A ownsthe components.
What changed
.page--narrowno longer caps the column. It caps its children:.page--narrow > :where(*). The:where()is load-bearing — at zero specificity a component that wants its own width still wins, which a bare> *at (0,1,0) would have taken away from.notice,.table-noteand.footnote > span..stack > :where(.field)stretches them to--measure; adds.field--short(12rem) and the missing.field:focus-visible.scroll-margin-top: 2.5remon sticky-header targets, and per-tablescroll-margin-lefton pinned-column targets, tracked through both narrow breakpoints.img,svg{height:auto};--rule→--rule-strongon the three control boundaries (.scroller, the pinned column edge,.strip);:not(:disabled)on pressed/current buttons; a colour-only.dim-ink; a▪marker as the non-colour second signal on selected filter chips (1.4.1);.btn--quietfolded into the 28px in-row grade;.row-togglegiven a 28px target (was 21.7px, under the 24px 2.5.8 floor).--t-label/uppercase/600 folded into one--- Label register ---rule, plus four--track-*tokens replacing literal tracking at 21 sites.--signal-warn--gold/--tier-flygdat near-identical L and C — 0.057 apart in OKLab, not enough to tell a gold FlyGD badge from an amber CRYO token two columns away. 50 doubles that while staying 0.146 from--signal-bad.@media (max-width: 64rem)stop for the audit table only. 641–1039px previously got the full desktop table against a 62rem floor: 0px of forced scroll at 639px, 399px at 641px. Now 95px at 641 and none at 768.Three e2e specs guard the geometry rather than the declarations: the page column's
rect on all seven shell routes, the audit table at 768/1280, and a control
scrolled to the top of the pinned region.
Two things worth a reviewer's attention
The focus-not-obscured test was green for the wrong reason, twice. Chromium's
programmatic focus scroll centres an off-screen element, so
.focus()neverreproduces the obscured case at all — the first draft landed the control at y=675
in a region whose header ends at 436, and passed with the CSS deleted. Playwright's
boundingBox()then compounded it by scrolling the element into view itself, so thesecond draft was measuring Playwright's scroll rather than the browser's. The test
now triggers
scrollIntoView({block: "nearest"})— the alignmentscroll-marginactually governs, and the one non-centring engines use for sequential focus
navigation — reads its rects inside a single
evaluate, and carries three guards.Verified both ways: 442px with the rule, 402px without it, against a header
ending at 436px. WebKit is still untested and all three reviewers flagged it.
The
.btn--danger-quietcontrast finding in the dispatch is wrong, and I did notact on it. It asserts 3.78:1 / 3.50:1 for the text pair. Recomputing OKLCH →
OKLab → linear sRGB → WCAG gives 5.40 / 4.82 / 4.18 / 4.73. The dispatch's figures
reproduce exactly as a linear-space composite; browsers composite translucent
paint in gamma-encoded sRGB, so 5.40/4.73 are correct and are the text pair. The
only sub-4.5 figure is 4.18 on
--hull-hi, and--hull-hinever paints there —.btn--danger-quiet:hover:not(:disabled)declares its own translucent backgroundlater at equal specificity than
.btn--quiet:hover's. No change needed.Out of scope — for the sessions that own these
ui.tsx:55-57says "the two narrow routes"; there are five (error.tsx:140,not-found.tsx:41,account/page.tsx:150,payouts/[id]/not-found.tsx:52,payouts/new/page.tsx:51). 1A's file — requesting the correction, not making it..stdeclares nofont-weightand so renders at 400 while.btnand.tierdeclare 600. That is the exact defect the label register exists to close, but the
dispatch excludes
.stbeyond its tracking token, so it is flagged, not fixed..st--leadfails 1.4.12 — about 312px of content in a 288px box at 320px. Notamong the seven items.
:nth-childordinal declarations couple the CSS to column order; fixingit needs
.tsx.--t-displayis unreferenced and.launch__titleruns at a literal 1.5rem →Session E.
.log--accounts; no such class exists — the accounts tableis
log log--dense log--sticky-head log--sticky-col(admin/accounts/page.tsx:209).Used
.log--dense..dim-inkships unreferenced — the data cells that should take it are.tsx.Judgment calls
.btn--quietfolded into the 28px grade rather than documented as a third size.Caveat: it is used standalone at
payouts/[id]:318andadmin/audit:350, whichnow sit at 28px against DESIGN.md's 36px-for-standalone line.
.shell__nav ajoined the register beyond the enumerated list — leaving it at 400beside a
.shell__registermoved to 600 would rebuild the split inside one bar..push__nextwas enumerated but is deliberately not in the register: it renders"next 14:32" beside an
.agoreading "3m ago", so folding it in would uppercase andembolden one half of a two-item metadata row. It keeps its own properties.
.stack > .field, (0,2,0)) outranks itsown companion
.field--short(0,1,0), so every short field in a stacked form wouldsilently take the full measure back. Wrapped in
:where()— the same device C3uses. This refines the mechanism; it does not re-adjudicate the ruling.
only-wide/only-narrowswap is scoped to.log--audit, because the bareutilities also drive the sync runs table, whose floor is tuned separately.
Verification
npm testran before the final comment-only CSS edits; nosrc/**/*.tschanged inthis branch, and the unit suite does not load
globals.css. Everything else abovewas run against the committed tree.
Merged
main(2026-08-04)Absorbed #90, #91 (session 1A) and #92. One conflict, in
src/app/globals.css, immediately after.field:hover— both sides had inserted a different block there. Resolved by keeping both, then checking whether they overlapped semantically. They did, and that changed what item 2 ships:Item 2 (A6 / ruling C5) is now two thirds of what it was
C5's defect —
.stack'sjustify-items: startcollapsing every form control to its intrinsic width, so a date, a percentage and a wallet division all rendered ~20 characters wide — was fixed independently onmainby #90, which introduced.form-stack/.form-stack__fieldand moved all five payout forms off.stackentirely.After the merge I checked every surviving
.stackcall site:payouts/[id]/payment-history.tsx:40<li>history linesadmin/accounts/page.tsx:407, 646, 654TokenState/ContactStatepayouts/[id]/page.tsx:784account/page.tsx:354ContactStateNot one contains a
.field. So.stack > :where(.field)and.field--shorthad zero reachable call sites on the merged tree, and.field--shorthad none in 1A's.tsxeither. I removed both rather than ship two mechanisms for one defect, or dead CSS carrying a comment that explains a specificity interaction between two selectors that no longer match anything.To be explicit about the boundary the dispatch draws: this is not re-adjudicating C5. C5's ruling stands and its defect is fixed — by
.form-stack, whose own comment states the same diagnosis in the same terms. What the merge removed was my mechanism, not the ruling..field:focus-visibleis unaffected and still ships. It is item 2's other half, independent of.stack, and it remains the only rule that delivers DESIGN.md's long-standing "gold border on focus plus the global focus ring".Verification, re-run against the merged tree
Both suites are larger than in the run above — the merge brought in #90–#92's tests (
e2e/submit-guard.spec.tsand 6 unit tests). Everything passes, including 1A's component changes running against this branch's stylesheet, which is the combination neither branch could exercise alone.Summary by CodeRabbit
Accessibility
Responsive Design
Visual Design
Quality