feat(payouts): owner walkthrough session 1 — the /payouts/[id] block - #178
Conversation
Findings 1.1-1.8 from docs/design-walkthrough.md, the block carrying both
P0s. Acceptance criteria met: the battle-report edit control is visible at
rest and reachable on touch; Finalize's consequence is readable before
arming; the notes save confirms visibly; the flat-value path is
discoverable without a screen reader.
1.1 .btn--quiet's transparent rest state gave InlineEdit's "edit" trigger
no visible affordance. It was never hover-only display — it renders at
--ink-faint always — so this is contrast, not visibility. Adds a
--rule-strong border under .inline-edit only.
1.2 ConfirmCost's cost sentence was .visually-hidden permanently, so no
sighted operator ever read it. That is R4's failure (information living
only in the AT channel), not an ordering problem. Adds a third
visibility mode, "visible": rendered plainly at rest, no reveal step.
"reveal" stays wrong here — prose appearing mid-press reads as an error,
not a cost — and the <td> reflow constraint behind "hidden" is untouched.
1.3 The notes save produced no sighted feedback. Adds a "· saved"
confirmation in one role="status" span, both channels through one node.
1.4 "Add another paste" named only the appraise path while the accessible
name also named the flat-value escape hatch — R4 in the other
direction.
1.5 Drops the items table's non-varying "Price source" column, folding
unresolved/manual into the item-name cell.
1.6 Inlines the payment list at N=1 rather than collapsing one row behind a
disclosure.
1.7 Moves the frozen-roster paragraph beside the roster it explains.
1.8 Surfaces primaryStage as a "· next:" hint on the lifecycle control.
Verification: format:check, typecheck, lint, build and check-node-version
all clean; unit suite 83 files / 1281 tests passing (same file count as
before, so nothing was silently dropped); e2e 304 passed / 1 failed.
That one failure is `notes save ... twice running`, and it is pre-existing
and flaky on main, not caused by this branch — measured at n=5, main itself
fails it 1/5 while the variants here fail 2/5 to 2/10. An instrumented probe
shows the second Save emits no POST: useSubmitGuard latches inFlight on
click and releases it only from an effect that must observe a pending=true
render then a pending=false one, so when the action settles before React
commits the pending render the latch never releases and later clicks are
preventDefault'ed. That is a real lost-save bug, not only a test artifact,
and playwright.config.ts sets no retries. Left unfixed deliberately:
submit-guard.ts is a shared primitive behind every submit button and is
outside this block's scope.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 49 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Comment |
1 participant
Session 1 of
docs/design-walkthrough.md— the/payouts/[id]block, which carries both P0s.Acceptance criteria (from the walkthrough), all met: the battle-report edit control is visible at rest and reachable on touch; Finalize's consequence is readable before arming; the notes save confirms visibly; the flat-value path is discoverable without a screen reader.
The two P0s
1.1 — the
edittrigger had no affordance. The walkthrough described it as hover-only. It isn't:.btn--quietrenders at--ink-faintwith a transparent border at all times, so this is a contrast/affordance problem rather than a display one — which also means it affected everyInlineEdittrigger on the page, not just the battle-report one. Fixed with a--rule-strongborder scoped to.inline-editonly (verified not a no-op against the.btn/.btn--quiet/:hovercascade, and verified not to leak to the three sibling controls that are not descendants).1.2 — Finalize's cost sentence was hidden from sighted operators permanently. It was
.visually-hiddenalways, so the copy explaining what Finalize does told nobody. Reported as an ordering problem; it is actually R4 (information living only in the assistive-tech channel). Adds a thirdConfirmCostmode,"visible"— rendered plainly at rest, no reveal step, still thearia-describedbytarget."reveal"stays wrong here because prose appearing mid-press reads as an error rather than a cost, and the<td>-reflow constraint behind"hidden"(#108/#111/#112) is explicitly not relaxed —DESIGN.md's R2 is amended to record that.The rest
1.3 notes save confirmation (
"· saved", onerole="status"node carrying both channels) · 1.4 accessible name vs visible label parity on "Add another paste" · 1.5 drops the non-varying "Price source" column · 1.6 inlines the payment list at N=1 · 1.7 moves the frozen-roster paragraph beside its roster · 1.8 surfacesprimaryStageas a· next:hint.All eight have e2e coverage.
Verification
format:checktypechecklintnpm testnpm run buildcheck-node-version.shnpm run test:e2eThe unit file count matches the pre-change count, so no test file was silently dropped by a load failure.
The one e2e failure is pre-existing, and it hides a real bug
notes save from an always-open textarea, twice runningfails intermittently. It is flaky onmain, not caused by this branch. Measured at n=5 per variant:mainAn earlier 3-run comparison pointed the other way; at higher n it doesn't hold.
An instrumented probe shows what's happening: the second Save emits no POST at all.
useSubmitGuardlatchesinFlighton click and releases it only from an effect that must observe apending === truerender followed by apending === falseone. When the action settles before React commits the pending render, that transition is missed, the latch never releases, and every subsequent click ispreventDefaulted.So this is a user-facing lost-save bug, not just a test artifact: save, then save again quickly, and the second save silently does nothing.
playwright.config.tssets noretries, so CI onmainis already intermittently red from it.Left unfixed deliberately —
submit-guard.tsis a shared primitive behind every submit button in the app, and fixing it is well outside this block's scope. Flagging for a separate change.Also worth a look
notes-form.tsxgates its confirmation on comparing the textarea against the value the action actually submitted, rather than adirtyflag. The flag version had a real defect: it cleared unconditionally when the action resolved, but the textarea stays editable during the round trip, so typing through a save left"· saved"standing over text the server never received.manualbadge into the item-name cell, slightly beyond that finding's literal text.ConfirmCost's"hidden"mode now has zero callers — keep with coverage, or delete?