Skip to content

style(design-sweep): the seven shared-style items (session 1B) - #93

Merged
guarzo merged 3 commits into
mainfrom
worktree-design-sweep-1b-styles
Aug 5, 2026
Merged

style(design-sweep): the seven shared-style items (session 1B)#93
guarzo merged 3 commits into
mainfrom
worktree-design-sweep-1b-styles

Conversation

@guarzo

@guarzo guarzo commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Session 1B of the design sweep: the seven shared-style items, in one pass over
src/app/globals.css and DESIGN.md. No .tsx is touched — session 1A owns
the components.

What changed

# Item Change
A5 / C3 The column moves on nav .page--narrow no 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-note and .footnote > span.
A6 / C5 Form fields collapsing .stack > :where(.field) stretches them to --measure; adds .field--short (12rem) and the missing .field:focus-visible.
A4 / C4 WCAG 2.4.11 scroll-margin-top: 2.5rem on sticky-header targets, and per-table scroll-margin-left on pinned-column targets, tracked through both narrow breakpoints.
Tier B batch img,svg{height:auto}; --rule--rule-strong on 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--quiet folded into the 28px in-row grade; .row-toggle given a 28px target (was 21.7px, under the 24px 2.5.8 floor).
typeset The label register Thirteen selectors' duplicated mono/--t-label/uppercase/600 folded into one --- Label register --- rule, plus four --track-* tokens replacing literal tracking at 21 sites.
colorize --signal-warn Retuned hue 70 → 50. At 70 it sat 18° from --gold/--tier-flygd at 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.
responsive The unserved band One @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() never
reproduces 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 the
second draft was measuring Playwright's scroll rather than the browser's. The test
now triggers scrollIntoView({block: "nearest"}) — the alignment scroll-margin
actually 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-quiet contrast finding in the dispatch is wrong, and I did not
act 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-hi never paints there —
.btn--danger-quiet:hover:not(:disabled) declares its own translucent background
later at equal specificity than .btn--quiet:hover's. No change needed.

Out of scope — for the sessions that own these

  • ui.tsx:55-57 says "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.
  • .st declares no font-weight and so renders at 400 while .btn and .tier
    declare 600. That is the exact defect the label register exists to close, but the
    dispatch excludes .st beyond its tracking token, so it is flagged, not fixed.
  • .st--lead fails 1.4.12 — about 312px of content in a 288px box at 320px. Not
    among the seven items.
  • Twelve :nth-child ordinal declarations couple the CSS to column order; fixing
    it needs .tsx.
  • --t-display is unreferenced and .launch__title runs at a literal 1.5rem →
    Session E.
  • The dispatch names .log--accounts; no such class exists — the accounts table
    is log log--dense log--sticky-head log--sticky-col (admin/accounts/page.tsx:209).
    Used .log--dense.
  • .dim-ink ships unreferenced — the data cells that should take it are .tsx.

Judgment calls

  • .btn--quiet folded into the 28px grade rather than documented as a third size.
    Caveat: it is used standalone at payouts/[id]:318 and admin/audit:350, which
    now sit at 28px against DESIGN.md's 36px-for-standalone line.
  • .shell__nav a joined the register beyond the enumerated list — leaving it at 400
    beside a .shell__register moved to 600 would rebuild the split inside one bar.
  • .push__next was enumerated but is deliberately not in the register: it renders
    "next 14:32" beside an .ago reading "3m ago", so folding it in would uppercase and
    embolden one half of a two-item metadata row. It keeps its own properties.
  • Ruling C5's selector as literally written (.stack > .field, (0,2,0)) outranks its
    own companion .field--short (0,1,0), so every short field in a stacked form would
    silently take the full measure back. Wrapped in :where() — the same device C3
    uses. This refines the mechanism; it does not re-adjudicate the ruling.
  • The only-wide/only-narrow swap is scoped to .log--audit, because the bare
    utilities also drive the sync runs table, whose floor is tuned separately.

Verification

npm run typecheck    clean
npm run format:check All matched files use Prettier code style!
npm run lint         ✖ 3 problems (0 errors, 3 warnings)   [pre-existing no-img-element]
npm test             Test Files 72 passed (72) / Tests 880 passed (880)
npm run test:e2e     155 passed (2.6m)

npm test ran before the final comment-only CSS edits; no src/**/*.ts changed in
this branch, and the unit suite does not load globals.css. Everything else above
was 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's justify-items: start collapsing every form control to its intrinsic width, so a date, a percentage and a wallet division all rendered ~20 characters wide — was fixed independently on main by #90, which introduced .form-stack / .form-stack__field and moved all five payout forms off .stack entirely.

After the merge I checked every surviving .stack call site:

Site Contents
payouts/[id]/payment-history.tsx:40 <li> history lines
admin/accounts/page.tsx:407, 646, 654 status tokens / TokenState / ContactState
payouts/[id]/page.tsx:784 payment-state tokens
account/page.tsx:354 ContactState

Not one contains a .field. So .stack > :where(.field) and .field--short had zero reachable call sites on the merged tree, and .field--short had none in 1A's .tsx either. 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-visible is 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

npm run typecheck    clean
npm run format:check All matched files use Prettier code style!
npm run lint         ✖ 3 problems (0 errors, 3 warnings)   [pre-existing no-img-element]
npm test             Test Files 73 passed (73) / Tests 886 passed (886)
npm run test:e2e     157 passed (2.3m)

Both suites are larger than in the run above — the merge brought in #90#92's tests (e2e/submit-guard.spec.ts and 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

    • Improved keyboard focus visibility, including controls near sticky headers.
    • Increased interactive target sizes and clarified disabled and selected states.
    • Added visible busy indicators for buttons.
  • Responsive Design

    • Improved table behavior across mobile and desktop widths.
    • Preserved sticky headers while preventing unnecessary horizontal scrolling.
  • Visual Design

    • Refined warning colors, typography, spacing, labels, borders, and page layout consistency.
    • Improved drawer sizing and responsive content containment.
  • Quality

    • Added automated coverage for accessibility, responsive tables, and consistent page layout across routes.

…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.
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 715bdb91-4a61-4665-aeb0-9219264055c7

📥 Commits

Reviewing files that changed from the base of the PR and between 4ca52c6 and 5f78cc1.

📒 Files selected for processing (3)
  • DESIGN.md
  • e2e/audit.spec.ts
  • src/app/globals.css
📝 Walkthrough

Walkthrough

The 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.

Changes

Design, accessibility, and responsive layout

Layer / File(s) Summary
Typography and design tokens
DESIGN.md, src/app/globals.css
The design guidance and global styles add shared label rules, tracking tokens, warning color updates, image sizing, and value styling.
Control focus and hit targets
DESIGN.md, src/app/globals.css, e2e/admin.spec.ts
Disabled controls, focus states, selected markers, row targets, sticky-region margins, and focus regression coverage are updated.
Page-column geometry
src/app/globals.css, e2e/shell.spec.ts
Narrow surfaces cap child content while shell routes retain a centered 1248px page column.
Responsive audit tables
src/app/globals.css, e2e/audit.spec.ts
Audit tables add responsive breakpoints, narrow timestamps, sticky-column margins, and viewport coverage at 320px, 390px, 768px, and 1280px.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

  • guarzo/authGD#38: Extends earlier admin-table responsiveness and sticky/focus behavior.
  • guarzo/authGD#61: Overlaps with sticky-column and drawer geometry testing.
  • guarzo/authGD#11: Refines the same design-system rules in DESIGN.md and globals.css.

Poem

A rabbit checks the focus glow,
While tables shrink and columns flow.
Labels track in tidy lines,
Sticky headers guard the signs.
Page bounds hold their centered place—
Hop, the tests confirm the case!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the design sweep and the seven shared-style changes in this pull request.
Description check ✅ Passed The description provides detailed change rationale, verification results, scope boundaries, deployment context, and reviewer flags.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-design-sweep-1b-styles
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch worktree-design-sweep-1b-styles

Comment @coderabbitai help to get the list of available commands.

@guarzo
guarzo enabled auto-merge (squash) August 5, 2026 00:17
…-1b-styles

# Conflicts:
#	src/app/globals.css

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Set .st to weight 600.

Line 1127 changes the tracking token but leaves .st without font-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

📥 Commits

Reviewing files that changed from the base of the PR and between c61db0c and 4ca52c6.

📒 Files selected for processing (5)
  • DESIGN.md
  • e2e/admin.spec.ts
  • e2e/audit.spec.ts
  • e2e/shell.spec.ts
  • src/app/globals.css

Comment thread e2e/audit.spec.ts Outdated
Comment thread src/app/globals.css
Comment thread src/app/globals.css
@guarzo
guarzo disabled auto-merge August 5, 2026 00:33
…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.
@guarzo
guarzo enabled auto-merge (squash) August 5, 2026 00:43
@guarzo
guarzo disabled auto-merge August 5, 2026 00:46
@guarzo
guarzo merged commit e3a4ab2 into main Aug 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants