Skip to content

style(payouts): the detail page sweep (task B) - #96

Merged
guarzo merged 1 commit into
mainfrom
worktree-payouts-detail-sweep-B
Aug 5, 2026
Merged

style(payouts): the detail page sweep (task B)#96
guarzo merged 1 commit into
mainfrom
worktree-payouts-detail-sweep-B

Conversation

@guarzo

@guarzo guarzo commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Task B of the design sweep: /payouts/[id]. $impeccable clarifyquieterdistill in one session, since all three edit the same file.

Re-verification pass (all twelve, before any edit)

The task file's :NNN citations were written against f79479a and 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.

# Item Verdict before editing Current location
1 No paid/total count on the detail page open derivation page.tsx:167-168, rendered as the Roster RuleHead aside :688-697
2 Unpaid rows are tone="warn", so amber runs backwards open page.tsx:891
3 The two pool warnings trapped inside the Scroller already fixed by #90 — dropped, not re-fixed already outside the Scroller at page.tsx:410-460
4 Reprice-after-finalize is invisible open new Notice at page.tsx:477-502
5 Roster table drops every scanning affordance open page.tsx:796-797
6 Four hand-rolled notices open, all four now Notice at :430, :446, :704, :719
7 One list, three names open RuleHead :688, Scroller label :796
8 Finalize ~600 rendered lines above the shares it freezes open relocated to page.tsx:1010-1051
9 CopyAmountButton two columns from its amount partly closed by #90 #90 had already fixed the participant-scoped accessible name and removed the 11/6/11 label swap via .copy-result; only column-adjacency remained. Now page.tsx:872
10 .inline-form is display: inline open globals.css:2283 — cited at 2078-2080, actually there; #93 shifted it
11 Computed ISO date in the proportional face open page.tsx:193
12 Adopt fmtIsk; .log__empty-text at both empty states open nine fmtIsk sites (8 in page.tsx, 1 in payment-history.tsx); empty states :393, :1001

One claim in the task file is wrong. Item 8 says .btn-row--controls is "unused". It is not — it lives at globals.css:1518 (not 1316-1321) and admin/sync/page.tsx:324 has 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

  • Ruling 1firstPayment untouched; per-row mark paid does not arm. Took the sticky-header fix (item 5) as the substitute. No evidence surfaced that would overturn this.
  • Ruling 2revert keeps its armedClassName. Fixed upward instead: delete pool gained armedClassName="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

  • Paid/owed count in the Roster RuleHead aside, mirroring payout-view.ts's paidCount semantics — an excluded row leaves both numerator and denominator, so an all-excluded roster reads 0/0, not 0/N. Suppressed entirely when the roster is empty, since "0/0" reads as stalled rather than empty.
  • A reprice notice for the finalized-and-unpriced state, with two branches: one pointing at Unlock for those who can, one naming who can for those who can't.
  • Finalize/Unlock relocated below the roster with .btn-row--controls, plus a .dim sentence explaining what Unlock does — it previously carried no copy at all.
  • Roster naming settled: section and region both "Roster", rows "participant".

quieter

  • unpaid drops tone="warn" to neutral. Same call DESIGN.md makes for cryo — colour only where someone scans for it.
  • .inline-form from display: inline to a wrapping flex row, so the field and its micro button share a line instead of running into the prose beside them.

distill

  • All four hand-rolled notices → Notice.
  • Scroller label="Roster" tall + log--dense log--sticky-head log--sticky-col on the roster table.
  • CopyAmountButton moves under the amount it copies. The raw p.amount still goes to the clipboard, never through fmtIsk. It is wrapped in a plain <div> because .stack is 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.
  • fmtIsk at nine display sites; .log__empty-text at both empty states; the ISO date in .mono.

The two wrong comments, corrected as instructed: page.tsx (corp-share <dd>) and services/payouts.ts:127-131 both cited "deleting the operation" as a recovery route. Verified against every delete call in src/ — 15 of them, targeting session, oauthTransaction, outbox, wandererAclObservation, contactSyncState, character, account, payoutParticipant, and lootPool (payout-loot.ts:242). None targets payoutOperation. 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-typed role="alert". As <Notice tone="warn"> they now render role="status" — assertive to polite. This is the primitive's documented contract, not an oversight: ui.tsx:242-248 says Notice exists precisely because 8+ hand-rolled sites had drifted between alert, status and no role, and that bad is 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

  • Item 3 — already fixed by fix(payouts): align the operation forms, and stop four of them shouting at once #90. Dropped per the task file rather than re-fixed differently.
  • <colgroup> on the new roster table. Review flagged that admin/accounts/page.tsx:215-218 pairs the sticky kit with a bare <col /> for the scanning anchor and log__col--fit for 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 .stack rather than the "single badge, date, or button pair" that precedent assumes, so width: 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 hardcoded colSpan={5} at both empty states with a derived COLUMN_COUNT, as the accounts page does.
  • A redundant guard I introduced. The reprice notice at page.tsx:478 reads ... && !canEdit && operation.status === "finalized" && ..., but canEdit is access.isOperator && operation.status === "draft" && !locked (:142), so finalized already 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 paid promoted to gold; the two pool warnings remain separate; ConfirmArmScope stays one-per-table; the canEdit lifecycle mirror is intact; PaymentHistory still says "unknown" and still returns null on empty; CopyAmountButton's synchronous-throw guard is untouched; no text-decoration: none on body-cell links; no em dash removed.

Verification

All five run in this worktree, output quoted verbatim.

> authgd@0.1.0 typecheck
> tsc --noEmit

Clean, no output.

✖ 3 problems (0 errors, 3 warnings)

All three are pre-existing no-img-element warnings (ui.tsx:99, login/page.tsx:36, login/page.tsx:75). None new.

Checking formatting...
All matched files use Prettier code style!
 Test Files  73 passed (73)
      Tests  886 passed (886)
   Start at  21:23:59
   Duration  166.07s (transform 1.63s, setup 0ms, collect 28.35s, tests 118.47s, environment 20ms, prepare 5.44s)
  160 passed (3.3m)

Test updates were assertion-only, both for fmtIsk's comma grouping: four ISK strings in e2e/payouts.spec.ts and one in tests/payment-history.test.ts.

tsconfig.json and AGENTS.md are unmodified after the e2e run — git status is clean apart from the six intended files.

Where to focus review

  1. The reprice notice's predicate (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.
  2. The role="alert"role="status" change on the two roster warnings.
  3. The .stack/grid interaction in the Amount cell.

Based on e3a4ab2. #94 landed on main during this work; no file overlap, so no rebase.

Summary by CodeRabbit

  • Improvements
    • Payout amounts now display with localized comma formatting and the ISK suffix throughout payout details and payment history.
    • Payout pages show clearer paid and owed participant counts, including exclusions.
    • Added inline editing for the corporation share percentage.
    • Improved loot and roster warnings, including guidance for resolving unresolved loot after finalization.
    • Enhanced roster readability with denser scrolling, sticky headers, clearer empty states, and convenient copy controls.
    • Updated delete, save, payment, finalize, and unlock controls with clearer labels and guidance.

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

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5a4d4605-6866-4a52-82b2-ba10aaae150e

📥 Commits

Reviewing files that changed from the base of the PR and between 782bf9c and 630593f.

📒 Files selected for processing (6)
  • e2e/payouts.spec.ts
  • src/app/globals.css
  • src/app/payouts/[id]/page.tsx
  • src/app/payouts/[id]/payment-history.tsx
  • src/services/payouts.ts
  • tests/payment-history.test.ts

📝 Walkthrough

Walkthrough

The payout detail page now formats ISK values, supports inline corp-share editing, updates roster and finalization controls, and presents warnings with Notice. Payment history, E2E checks, and unit tests now expect comma-grouped ISK output.

Changes

Payout UI and display

Layer / File(s) Summary
Amount display and corp-share editing
src/app/payouts/[id]/page.tsx, src/app/globals.css
The page formats payout and loot values with fmtIsk, adds corp-share editing, and updates inline form layout.
Warnings and finalization guidance
src/app/payouts/[id]/page.tsx, src/services/payouts.ts
Warnings use Notice, and finalized operations show guidance for unresolved loot and Unlock behavior.
Roster counts and payment controls
src/app/payouts/[id]/page.tsx
The roster shows paid and owed counts, dense sticky table styling, formatted amounts, and colocated copy controls.
Payment display validation
src/app/payouts/[id]/payment-history.tsx, tests/payment-history.test.ts, e2e/payouts.spec.ts
Payment history and assertions now use comma-grouped ISK values with the ISK suffix.

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

Possibly related PRs

  • guarzo/authGD#65: Introduced the payout detail page and related payment flows updated here.
  • guarzo/authGD#83: Added broader payout behavior coverage in the same page and test areas.
  • guarzo/authGD#91: Introduced the fmtIsk utility used by these payout displays.

Poem

A rabbit counts the ISK in rows,
With commas neat where each sum grows.
The corp share hops into its place,
Notices guide the payout race.
Unlock waits when loot remains—
Then tests confirm the grouped gains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies a payouts detail-page design sweep and matches the main changes in /payouts/[id].
Description check ✅ Passed The description thoroughly explains the changes, verification results, deliberate behavior changes, skipped scope, and review focus.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-payouts-detail-sweep-B
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch worktree-payouts-detail-sweep-B

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

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