test(payouts): pin the notes Save control to the standalone hit target - #158
Conversation
#149 raised this button to the 36px standalone grade and shipped spec coverage only for the separate `/admin/accounts` note form, which CodeRabbit flagged on that PR. Nothing on `/payouts/[id]` asserted the height, so a regression to `.btn--micro` would leave the whole suite green while an operator on a touch screen — the one holding the device one-handed in a fleet — gets a 28px target for the control that saves what the payout was for. Measured against `Finalize` rather than as a bare `toBe(36)`, so the number means "the same grade this page's own standalone controls use". The comparison cannot be `mark paid`: the notes form renders only on a draft operation and `mark paid` only on a finalized one, so no page state carries both grades at once. Verified to pin: with the control reverted to `.btn btn--micro` the spec fails "Expected: 36, Received: 28".
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 10 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 (1)
Comment |
|
@coderabbitai review |
|
1 participant
Round six of an unattended design sweep, and the only one that came from the automated reviewer rather than from the sweep's own backlog. One sentence: pin
/payouts/[id]'s notesSavecontrol to the 36px standalone hit target, which nothing asserted.Based on
main, not on the stack. #154 and #155 are still open and still stack on each other; this touches onlye2e/payouts.spec.ts, which neither of them goes near, so it merges in any order relative to them.Why
CodeRabbit's review of #149 said it plainly: that PR raised this control to the standalone grade and shipped spec coverage only for the separate
NoteFormon/admin/accounts.src/app/payouts/[id]/notes-form.tsxis a different component with a different comment explaining the same decision, and no test held it.The cost. An operator working a payout on a phone — the case this grade exists for — gets a 28px target for the control that saves what the fleet was, if the class ever drifts back. Every other gate stays green while it happens: the change is one word in a
className, invisible to typecheck, lint, unit tests and format.What the spec does
e2e/payouts.spec.ts:2452. It measuresSave notesagainstFinalizerather than asserting a baretoBe(36), so the number means "the same grade this page's own standalone controls use" instead of restating a constant.The obvious comparison —
Save notesagainst a per-rowmark paidat 28px — is impossible, and the docblock records why: the notes form renders only while the operation is a draft (page.tsx:161) andmark paidonly once it is finalized. No page state shows both grades at once.Verified to pin, not merely to pass. With
notes-form.tsxreverted to.btn btn--micro, the spec failsExpected: 36, Received: 28. The source file was restored from that mutation andgit statusconfirms the diff is the spec alone.Gates
Run on this branch in the worktree, quoted:
tsc --noEmit— exit 0eslint .— exit 0prettier --check .— "All matched files use Prettier code style!"npm test— 77 files, 1145 tests passednpx playwright test— 233 passed (4.0m), exit 0, 233✓lines counted in the captured log. That ismain's 232 plus exactly the one spec this branch adds.Not run here:
docker build .andscripts/check-node-version.sh— environment-bound. CI runs both on this PR; treat them as unchecked until it does.What this round did not do
CodeRabbit's other two findings on the merged PRs were not acted on, and both are judgement calls a human should make rather than an unattended run:
.st--okshould inherit--ink-dimfor passive success states. This asks to partly undo round one's "each Status tone gets its own shape, not just its own colour". Reversing a shipped decision from this same run is exactly what this loop is told to skip and report.audit loglink is still.btn--micro. Superseded: fix(payouts): resume from the row just paid, not the top of the roster #151 put the drawer's note button back to.btn--microdeliberately, so that drawer is now internally consistent at 28px and raising the one link would recreate the inconsistency. The reasoning is innote-form.tsx:55-76, and the open question underneath it — whether aDisclosure as="row"drawer counts as "in-row" for DESIGN.md — is still one for a human.