fix(a11y): design sweep round 1 — shared-component consistency by guarzo · Pull Request #148 · guarzo/authGD

guarzo · 2026-08-06T09:12:22Z

Round one of an unattended design sweep. Shared-component consistency work only — every change here is to a component or rule that many surfaces render, which is why it goes first: later rounds re-review surfaces that already have these fixes in them, rather than re-finding them once per page.

The sweep re-grounded from scratch (18 reports across 9 surfaces, critique + audit each, 175 findings) rather than carrying forward the 2026-08-06 run's backlog, because #128, #145 and that run's own ten rounds had reshaped the surfaces those findings were written against.

What changed

.st::before gets a shape per tone, not just a colour (globals.css)
Status pills carried their meaning in hue alone. A red-green colourblind operator scanning the admin accounts table could not tell ok from bad without reading the word beside it — which is exactly the scan the table exists to make unnecessary. Neutral is a thin bar, ok a circle, warn a triangle, bad a square, off unchanged. width: 0.5em is preserved in every rule because the mono column advance at globals.css:1680-1687 depends on it. WCAG 1.4.1.

ui.tsx:223-225 already claimed "the glyph and the word both carry the meaning, so colour is never the only signal." That was true of the intent and false of the CSS; it is true of both now.

ConfirmSubmit reserves its width with a CSS ghost instead of ch arithmetic (confirm-submit.tsx, globals.css)
The old reservation was minWidth: max(label.length, confirmLabel.length) + 4 in ch. ch measures the "0" glyph advance and excludes letter-spacing, which this control sets to 0.1em and which accumulates once per character pair — so the shortfall scales with label length while the +4 fudge was flat. /admin/accounts cleared by ~2px; "Replace roster" fell ~23px short from the same constant. A destructive control therefore resized under the pointer at the moment it armed, which is the #112 disarm mechanism.

Replaced with an inline grid: a ::before carrying content: attr(data-ghost-label) holds the longer of the two labels at grid-area: 1/1, the visible text sits in a sibling span at the same cell. content: attr() is not a DOM node, so it never reaches textContent and is invisible to Playwright's getByText/toHaveText traversal — which matters because admin.spec.ts:1922 asserts toHaveText("freeze") directly on one of these buttons, and a hidden real <span> would have concatenated into it (the repo documents that hazard at e2e/admin.spec.ts:569-571).

Sign out gets the same hit target as the links beside it (globals.css, e2e/shell.spec.ts)
DESIGN.md rations the 28px .btn--micro grade to admin table rows and nowhere else. Sign out is .btn--quiet .btn--micro in the header, where every one of the four nav links is already min-height: 2.25rem — raised there by an earlier sweep that pinned the geometry in shell.spec.ts and did not carry the fix across to the one control in the bar that is not a link. So the smallest target in the header, on all ten pages that render one, was the button that ends the session.

Scoped override rather than dropping .btn--micro from the markup, because dropping it would not have worked: sign out is .btn--quiet .btn--micro, and .btn--quiet sets min-height: 1.75rem on its own account, so removing the micro class leaves the box where it was. .inline-edit--standalone .btn--quiet is written as an override for the same reason, while account/page.tsx:372-381 — no --quiet in play — could just drop the class. (The type is not the constraint: .btn and .btn--micro set the identical font-size: var(--t-label). An earlier draft of this PR body and of the CSS comment claimed otherwise; b908db3 corrects the comment.) The new spec asserts both halves — 36px tall, and the same computed font size as the links — and was verified to fail at 28px with the CSS reverted.

ConfirmCost owns its own register (confirm-submit.tsx, 4 call sites, globals.css)
.confirm-cost had exactly one rule and it was layout. Everything about how the cost sentence looked came from className="dim" passed by hand, identically, by all four call sites — a required argument dressed as an optional one. A fifth call site that forgot it would have rendered the cost at full body size and full ink, reading as primary copy at the moment the reveal puts it in front of someone deciding whether to unlink their Discord or delete an operation.

Skipped, and why

ConfirmGroup steals focus to an unlabelled wrapper (confirm-group.tsx:78-80). The finding is real: after a drawer-scoped confirm settles, focus moves to a tabIndex={-1} div. On /admin/sync the pressed Re-run button is still mounted and still enabled, so a keyboard admin's next Tab starts somewhere they never chose.

The obvious fix — only steal focus if document.activeElement === document.body, i.e. only when the pressed control disabled or unmounted itself — is wrong here, and no gate catches why. ConfirmGroup renders <Notice live={false}>, and ui.tsx:307 drops the role entirely when live is false. The focus move is not a courtesy layered on a live region; it is this component's only announcement channel. Making the focus conditional makes the announcement conditional, and it drops out on precisely the call site the fix was aimed at: Re-run keeps focus, the effect returns early, and the confirmation is announced by nothing. That converts a focus-order annoyance into a silent success — WCAG 4.1.3, worse than the defect.

A correct fix has to move live in step with the focus decision: announce via the region when the control survives, via focus when it does not. That is not mechanical — ui.tsx:282-285 argues a second announcement is worse than none, and flipping role onto an already-rendered node holding its text is the inverse of the "region born holding text" hazard that same docblock is built around. Two defensible readings, nothing in DESIGN.md or docs/settled-design-decisions.md settling it. Needs a human.

Backlog item retired, not fixed. payout-detail-audit reported that the first payment on /payouts/[id] freezes the operation with "nothing saying so." e2e/payouts.spec.ts:2161-2205 passes and asserts the opposite — the warning is on the control's accessible description at rest and on arm, and is deliberately hidden from sighted users (#mark-paid-cost width asserted <= 1px), which is the reveal-on-arm case #112 tried and reverted. The claim did not survive verification.

What this run cannot do at all

The loop can find structural problems and cannot fix them — shape and craft both need a human this run does not have. Named here rather than omitted, because a PR full of consistency fixes that stays silent about these implies the app's problem is inconsistent letter-spacing:

/admin/audit's target filter omits alts (critical). Filtering the audit log by a member returns only events recorded against their main, so an admin investigating a specific pilot sees a partial history and has no indication it is partial. Needs a query change plus IA for how alts are represented in the filter.
open info redirect throws the operator to the top of the page — on a 60-person roster, back to the skip link after every open.
The error boundary apologises for correct behaviour — a permissions bounce renders "Something broke."
docs/settled-design-decisions.md has itself drifted in two places. :155 says InlineEdit must reject via redirect, but inline-edit.tsx:18-27 reversed that deliberately — obeying the settled list as written would delete the reason StringFieldEditState exists. Separately, row 140 and errors.ts:59-62 call share_format/share_range unreachable while page.tsx:383 still renders their form. The settled list is an input to every future sweep, so drift in it is worth more than most findings.
e2e/login.spec.ts:69-82 cannot fail. .launch { overflow: hidden } suppresses the scrollWidth measurement the assertion reads, so the spec passes regardless of the overflow it exists to catch.

Gates

Run on this branch's tip, quoted:

tsc --noEmit — clean
eslint . — clean
prettier --check . — "All matched files use Prettier code style!"
npm test — 77 files, 1134 tests passed (matches the known-good baseline; a lower file count would mean a load failure silently dropped tests)
npm run build — succeeded, 19 routes
npm run test:e2e — 226 passed (3.7m)

Not run here, deferred to CI: docker build . and scripts/check-node-version.sh. Both are environment-bound and slow; CI runs them on this PR. Treat them as unchecked until it does.

Merge order

This is the base of a stack. Merge in order, and prefer a merge commit or rebase over squash — squashing this PR rewrites the commits later rounds are based on, and they will then show conflicts against changes that are, in substance, already there.

Summary by CodeRabbit

UI Improvements
- Improved confirmation controls so labels size consistently without affecting submission behavior.
- Refined confirmation cost messaging for clearer, more consistent presentation.
- Added distinct visual shapes for neutral, successful, warning, failed, and inactive status markers.
- Standardized the sign-out button’s height and typography with active navigation links.
Bug Fixes
- Corrected styling inconsistencies across account and payout confirmation screens.

`.st::before` set one filled dot for every tone; `.st--off::before` was the only variant. For ok / warn / bad / neutral, colour was the only non-textual signal, which WCAG 1.4.1 does not allow. A member with red/green colour deficiency scanning a payouts list at 1am sees an identical dot beside every operation and has to read each row's word to tell a paid operation from an unpaid one; at payouts/page.tsx:167-190 three branches render wording that differs only by tone, so there the distinction was unrecoverable. Neutral is now a thin bar, ok a circle, warn a triangle, bad a square, off the hollow circle it already was. Width stays 0.5em in every rule — only height and radius change — so the mono advance the comment at globals.css:1680-1687 depends on never moves. The docblock at ui.tsx:223-225 already claimed the glyph carried the meaning. This makes that true; it was a design intent the CSS never implemented.

The reservation was `Math.max(label.length, confirmLabel.length) + 4` in `ch`. That cannot hold: `ch` is the advance of the "0" glyph alone, while the label carries `letter-spacing` the browser inserts between every character pair, so the shortfall scales with label length and a flat +4 cannot track it. It overshot short labels and undershot long ones — /admin/accounts cleared by ~2px by accident, "Replace roster" fell ~23px short. An operator who clicks the right-hand sixth of "Replace roster" arms it, the button shrinks out from under their stationary pointer, pointerLeave fires, the control disarms, and a destructive action reads as dead — so they press it again. That is the trap #112 fixed elsewhere, reintroduced by arithmetic. The wider label is now rendered as CSS generated content in flow but `visibility: hidden`, sharing grid cell 1/1 with the real text, so the browser measures it with the button's own font, weight, case transform and letter-spacing. `content: attr()` is not a DOM node, so it cannot reach `textContent` — several call sites assert exact button text with `toHaveText`, which does read hidden real elements and would have picked up a duplicate. The visible text carries its own `grid-area: 1 / 1`. As an anonymous grid item it was auto-placed into row 2, which reserved the width correctly and doubled the button's height; measured in a browser, not inferred.

A member ending their session on a phone at 1am aims at the smallest control in the header — the only one in the bar that is not a link, on all ten pages that render a header. `.shell__signout`'s button is `.btn--quiet .btn--micro`, which carries `min-height: 1.75rem`. DESIGN.md rations that 28px grade to `.btn--micro` in admin table rows and nowhere else; every link beside it is already `min-height: 2.25rem` with `padding: var(--s-2) var(--s-3)`, raised there by an earlier sweep that pinned the geometry in e2e/shell.spec.ts and did not carry the same fix across to the button. Scoped override on `.shell__signout .btn` rather than dropping `.btn--micro` from the markup, following `.inline-edit--standalone .btn--quiet`: the micro grade also carries `font-size: var(--t-label)`, which is what makes sign out read as one of the nav's own labels, so removing the class to fix the box would have changed the type. The new spec asserts both halves — 36px tall, and the same computed font size as the links — and fails at 28px with the CSS reverted.

`.confirm-cost` had exactly one rule in globals.css and it was layout (`.facts__lead > .confirm-cost`, flex-basis). Everything about how the cost sentence *looks* came from `className="dim"` passed by hand — by all four call sites, identically, with no other value ever passed. That made `className` a required argument dressed as an optional one: the component decided when the sentence becomes visible but not what it looked like once it got there. A fifth call site that forgot the prop would have rendered the cost at full body size and full ink — reading as primary copy at the exact moment the reveal puts it in front of a member deciding whether to unlink their Discord or delete an operation. `.confirm-cost` now carries `.dim`'s two declarations itself and the passthrough is gone. No rendered text changes; the class list on each span loses only `dim`.

Review misread the ConfirmSubmit docblock as claiming `visibility: hidden` is what stops Playwright picking up a duplicate. It does not, and the comment did not quite say so — "would otherwise pick up a duplicate" had no explicit antecedent, so the sentence read as being about the hidden box rather than about the alternative that was rejected. The real property is that `content: attr()` is never a DOM node, at any visibility. What that rules out is a second real `<span>`, which `toHaveText` would read (e2e/admin.spec.ts:569-571) and concatenate into the button text several call sites assert exactly. Comment only.

coderabbitai · 2026-08-06T09:12:49Z

📝 Walkthrough

Walkthrough

The change updates confirmation control sizing, centralizes confirmation cost styling, removes redundant dimming classes, refines status marker shapes, sets the shell sign-out height to 36px, and adds an end-to-end sizing test.

Changes

Confirmation and control styling

Layer / File(s)	Summary
Confirmation control sizing `src/app/_components/confirm-submit.tsx`, `src/app/globals.css`	`ConfirmCost` removes its `className` prop and uses `.confirm-cost`. `ConfirmSubmit` uses generated CSS content to reserve space for the longer label.
Confirmation cost call sites `src/app/account/page.tsx`, `src/app/payouts/[id]/lifecycle-submit.tsx`, `src/app/payouts/[id]/page.tsx`	Confirmation cost usages remove the `dim` class while retaining their content and behavior.
Shell and status presentation `src/app/globals.css`, `e2e/shell.spec.ts`	The sign-out control uses a 36px hit area. Status markers use distinct shapes for each state. The end-to-end test checks sign-out height and font size.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

guarzo/authGD#59: Extends the confirmation UI refined by this change.
guarzo/authGD#128: Shares changes to confirmation controls, shell sizing, and global styling.
guarzo/authGD#149: Shares standalone control sizing and 36px geometry tests.

Poem

Labels hold steady, widths align,
Costs use one styling line.
Status shapes show their state,
Sign-out stands at thirty-six straight.
Tests watch each control in place.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description explains the shared accessibility changes, verification results, deferred checks, skipped findings, and merge-order requirements.
Title check	✅ Passed	The title uses the required Conventional Commit form and clearly identifies the accessibility-focused shared-component consistency work.
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.

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

coderabbitai

Actionable comments posted: 1

🤖 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 `@src/app/globals.css`:
- Around line 1594-1596: Update the .st--ok style to keep non-actionable success
states neutral by using --ink-dim for its text color, while preserving the
existing circle marker and other styling.

🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f358301d-377b-4c2b-a651-49bb400e8b56

📥 Commits

Reviewing files that changed from the base of the PR and between d297388 and 4848c40.

📒 Files selected for processing (6)

e2e/shell.spec.ts
src/app/_components/confirm-submit.tsx
src/app/account/page.tsx
src/app/globals.css
src/app/payouts/[id]/lifecycle-submit.tsx
src/app/payouts/[id]/page.tsx

The comment on `.shell__signout .btn` said dropping `.btn--micro` from the markup would have changed the type. It would not: `.btn` sets `font-size: var(--t-label)`, the identical value `.btn--micro` sets, so the type register is the same either way. The override is still the right shape, for a reason the comment did not give: sign out is `.btn--quiet .btn--micro`, and `.btn--quiet` sets its own `min-height: 1.75rem`. Removing the micro class would have left the box exactly where it was — which is also why `.inline-edit--standalone .btn--quiet` is written as an override rather than a class removal, while `account/page.tsx:372-381` could just drop `--micro` and be done. Comment-only. The rule and the spec pinning it are unchanged.

main moved under this run: #150, #152, #153 and #156 merged while the sweep was working, and #153 landed in `confirm-submit.tsx`, which round one had rewritten. GitHub reported #148 CONFLICTING/DIRTY. The conflict was two independent edits to one file, resolved by keeping both rather than choosing: - Round one replaced the `ch`-arithmetic width reservation with CSS generated content (`data-ghost-label`). #153's docblock paragraph describing the old `ch` reservation is superseded and dropped, along with its `widthCh` constant, which nothing renders any more. - #153 added the `confirm` prop, the below-the-hooks throw, and the `confirm && ctx?.armedId === id` arming test, all of which are about component-type stability in the pay flow and untouched by round one. Kept verbatim, comments included. Verified rather than assumed: #153's own regression spec — "a second row's mark paid button survives the first payment's arm-to-plain transition" (`e2e/payouts.spec.ts:2212`) — passes on the merge.

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/app/_components/confirm-submit.tsx (1)
342-345: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reserve the pending label width.

At src/app/_components/confirm-submit.tsx, Line 402 renders pendingLabel. At Line 345, ghostLabel considers only label and confirmLabel. If a caller supplies a longer pending label, the button still changes width during submission. Reserve every rendered label, or constrain pendingLabel to fit the ghost label.

Also applies to: 400-404
🤖 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/_components/confirm-submit.tsx` around lines 342 - 345, Update the
ghostLabel calculation in the confirm-submit component to account for
pendingLabel as well as label and confirmLabel, selecting the longest rendered
label so the button width remains stable during submission. Preserve the
existing monospace-length approach and pendingLabel rendering behavior.

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

Outside diff comments:
In `@src/app/_components/confirm-submit.tsx`:
- Around line 342-345: Update the ghostLabel calculation in the confirm-submit
component to account for pendingLabel as well as label and confirmLabel,
selecting the longest rendered label so the button width remains stable during
submission. Preserve the existing monospace-length approach and pendingLabel
rendering behavior.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 86e1e09e-14d2-44a9-80d3-ace7233292d7

📥 Commits

Reviewing files that changed from the base of the PR and between 4848c40 and 3733d96.

📒 Files selected for processing (3)

src/app/_components/confirm-submit.tsx
src/app/account/page.tsx
src/app/globals.css

guarzo added 5 commits August 6, 2026 03:56

coderabbitai Bot requested changes Aug 6, 2026

View reviewed changes

Comment thread src/app/globals.css

guarzo mentioned this pull request Aug 6, 2026

fix(a11y): stop the 28px hit-target grade escaping the admin table rows #149

Merged

coderabbitai Bot approved these changes Aug 6, 2026

View reviewed changes

This was referenced Aug 6, 2026

fix(payouts): resume from the row just paid, not the top of the roster #151

Merged

fix(admin): arm grant the way its own opposite is armed #154

Merged

fix(admin): open the date column newest-first #155

Merged

coderabbitai Bot reviewed Aug 6, 2026

View reviewed changes

guarzo merged commit 3fad950 into main Aug 6, 2026
7 checks passed

This was referenced Aug 6, 2026

test(payouts): pin the notes Save control to the standalone hit target #158

Merged

fix(a11y): let the tier button group wrap when the press adds auto #161

Merged

Navigation Menu

fix(a11y): design sweep round 1 — shared-component consistency - #148

fix(a11y): design sweep round 1 — shared-component consistency#148
guarzo merged 7 commits into
mainfrom
design-sweep/2026-08-06b-1

guarzo commented Aug 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

guarzo commented Aug 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Skipped, and why

What this run cannot do at all

Gates

Merge order

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

guarzo commented Aug 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading