style(payouts): the detail page sweep (task B) - #96
Conversation
Twelve-item design sweep of /payouts/[id], re-verified against main after PR #90 rewrote the surface. - Reprice route: an unresolved-only Notice, deliberately not firing on the sub-cent pools whose line totals are already correct. - Roster: sticky head + sticky first column under a `tall` Scroller, a paid/owed count in the RuleHead aside, and `.log__empty-text` at both empty states. - Copy amount moves under the amount it copies; the raw string still goes to the clipboard, never through fmtIsk. - Finalize/Unlock relocate below the roster with a sticky-header-safe control row. - `delete pool` gains an armedClassName and a per-row subject name. - fmtIsk at nine display sites plus the payment history. - Two comments citing "delete the operation" as a recovery route corrected: no such path exists, only deletePool.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe payout detail page now formats ISK values, supports inline corp-share editing, updates roster and finalization controls, and presents warnings with ChangesPayout UI and display
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Task B of the design sweep:
/payouts/[id].$impeccable clarify→quieter→distillin one session, since all three edit the same file.Re-verification pass (all twelve, before any edit)
The task file's
:NNNcitations were written againstf79479aand are stale after #90 and #93, so every item was re-found by searching for the construct it describes rather than by line number. Line numbers below are current, post-change.page.tsx:167-168, rendered as the RosterRuleHeadaside:688-697tone="warn", so amber runs backwardspage.tsx:891page.tsx:410-460Noticeatpage.tsx:477-502page.tsx:796-797Noticeat:430,:446,:704,:719RuleHead:688,Scroller label:796Finalize~600 rendered lines above the shares it freezespage.tsx:1010-1051CopyAmountButtontwo columns from its amount.copy-result; only column-adjacency remained. Nowpage.tsx:872.inline-formisdisplay: inlineglobals.css:2283— cited at2078-2080, actually there; #93 shifted itpage.tsx:193fmtIsk;.log__empty-textat both empty statesfmtIsksites (8 inpage.tsx, 1 inpayment-history.tsx); empty states:393,:1001One claim in the task file is wrong. Item 8 says
.btn-row--controlsis "unused". It is not — it lives atglobals.css:1518(not1316-1321) andadmin/sync/page.tsx:324has used it since #68. The item's substance still stands; only the "unused" characterisation doesn't. The relocation follows the existing precedent rather than activating dead CSS.Both rulings honoured
firstPaymentuntouched; per-rowmark paiddoes not arm. Took the sticky-header fix (item 5) as the substitute. No evidence surfaced that would overturn this.revertkeeps itsarmedClassName. Fixed upward instead:delete poolgainedarmedClassName="btn btn--micro btn--danger"and a real per-row subject name (delete pool {index+1}/confirm delete pool {index+1}), which still starts with the visible label "delete", keeping the WCAG 2.5.3 match.What changed
clarify
RuleHeadaside, mirroringpayout-view.ts'spaidCountsemantics — an excluded row leaves both numerator and denominator, so an all-excluded roster reads0/0, not0/N. Suppressed entirely when the roster is empty, since "0/0" reads as stalled rather than empty..btn-row--controls, plus a.dimsentence explaining what Unlock does — it previously carried no copy at all.quieter
unpaiddropstone="warn"to neutral. Same callDESIGN.mdmakes for cryo — colour only where someone scans for it..inline-formfromdisplay: inlineto a wrapping flex row, so the field and its micro button share a line instead of running into the prose beside them.distill
Notice.Scroller label="Roster" tall+log--dense log--sticky-head log--sticky-colon the roster table.CopyAmountButtonmoves under the amount it copies. The rawp.amountstill goes to the clipboard, never throughfmtIsk. It is wrapped in a plain<div>because.stackis a grid and the component renders two siblings — left loose, the button and its result line take separate grid rows, and the result line reserves 5rem even while empty, putting an empty row under every unpaid participant.fmtIskat nine display sites;.log__empty-textat both empty states; the ISO date in.mono.The two wrong comments, corrected as instructed:
page.tsx(corp-share<dd>) andservices/payouts.ts:127-131both cited "deleting the operation" as a recovery route. Verified against every delete call insrc/— 15 of them, targetingsession,oauthTransaction,outbox,wandererAclObservation,contactSyncState,character,account,payoutParticipant, andlootPool(payout-loot.ts:242). None targetspayoutOperation. Both comments now say there is no route to delete an operation, only a loot pool within one.Deliberate behavior change, flagged
The two roster warnings (
duplicateUnresolvedNames,crossStateClashes) previously hand-typedrole="alert". As<Notice tone="warn">they now renderrole="status"— assertive to polite. This is the primitive's documented contract, not an oversight:ui.tsx:242-248saysNoticeexists precisely because 8+ hand-rolled sites had drifted betweenalert,statusand no role, and thatbadis the only tone that interrupts. Item 6 names this exact drift as the thing to eliminate. Calling it out because it is a real change in announcement priority.What I skipped, and why
<colgroup>on the new roster table. Review flagged thatadmin/accounts/page.tsx:215-218pairs the sticky kit with a bare<col />for the scanning anchor andlog__col--fitfor the rest, and this table has no equivalent. Real, but not applied: two of the roster's four non-Name columns now hold a form and a.stackrather than the "single badge, date, or button pair" that precedent assumes, sowidth: 1%there is a layout change I can't verify without a visual pass — and it is outside the twelve items. Follow-up. The same follow-up would naturally replace the hardcodedcolSpan={5}at both empty states with a derivedCOLUMN_COUNT, as the accounts page does.page.tsx:478reads... && !canEdit && operation.status === "finalized" && ..., butcanEditisaccess.isOperator && operation.status === "draft" && !locked(:142), sofinalizedalready implies!canEdit. Left in as an explicit precondition rather than stripped, since removing it is a judgement call about redundancy rather than a fix. Flagging it so a reviewer can decide.No
mark paidpromoted to gold; the two pool warnings remain separate;ConfirmArmScopestays one-per-table; thecanEditlifecycle mirror is intact;PaymentHistorystill says "unknown" and still returnsnullon empty;CopyAmountButton's synchronous-throw guard is untouched; notext-decoration: noneon body-cell links; no em dash removed.Verification
All five run in this worktree, output quoted verbatim.
Clean, no output.
All three are pre-existing
no-img-elementwarnings (ui.tsx:99,login/page.tsx:36,login/page.tsx:75). None new.Test updates were assertion-only, both for
fmtIsk's comma grouping: four ISK strings ine2e/payouts.spec.tsand one intests/payment-history.test.ts.tsconfig.jsonandAGENTS.mdare unmodified after the e2e run —git statusis clean apart from the six intended files.Where to focus review
page.tsx:477-502) — it fires on unresolved items only and deliberately not on sub-cent ones. The sub-cent warning says the line total is real and already counted, so offering a route back there would tell an operator to unlock a finalized operation to correct a number that is already right.role="alert"→role="status"change on the two roster warnings..stack/grid interaction in the Amount cell.Based on
e3a4ab2. #94 landed onmainduring this work; no file overlap, so no rebase.Summary by CodeRabbit
ISKsuffix throughout payout details and payment history.