chore(deposits): dedupe split helper, extract OverflowMenu, fix silent revert errors

[steilerDev]

Summary

Post-#1404/#1405 cleanup arising from product-architect's reviews of PR #1407 (comment) and PR #1412 (comment). Bundles four medium-severity recommendations from the same review cycle:

1. Deduplicate the proportional deposit-split formula now inlined in four service files (refactor, no behaviour change).
2. Extract a shared OverflowMenu component from the 1363-line InvoiceDepositsSection.tsx (refactor, no behaviour change).
3. Replace an inline style={{}} JSX prop with a CSS-Module class (refactor, no behaviour change).
4. Surface API errors in the three deposit-revert handlers that currently swallow failures silently (bug fix, user-visible UX improvement).

Items 1-3 are behaviour-preserving; item 4 is a bug fix. Combined into one issue because all four are closely related cleanup from the same architect review cycle.

Findings

1. Extract splitByDeposits() helper to dedupe the proportional-split pattern

The proportional deposit-split formula is now inlined in four places:

• server/src/services/shared/depositAggregateUtils.ts -> computeDepositAwareAggregates()
• server/src/services/shared/depositAggregateUtils.ts -> computeStatusContribution()
• server/src/services/shared/depositAggregateUtils.ts -> aggregateInvoiceStatusBreakdown() (added in Invoice status summary breakdown is not deposit-aware (InvoicesPage header + Dashboard pipeline card) #1411)
• server/src/services/budgetSourceService.ts -> inline in computeDiscretionaryInvoiceAmount()

All four share a common shape:

• Group rows by invoice_id
• For each invoice, compute residual = max(0, invoice.amount - sum(deposits.amount))
• Bucket the residual under the parent invoice's status
• Bucket each deposit under its own status

Extract a single splitByDeposits() helper that returns the per-invoice split as a structured value, then have the four call sites reduce over those splits to produce their respective aggregate shapes.

2. Extract shared OverflowMenu component from InvoiceDepositsSection.tsx

client/src/pages/InvoiceDepositPage/InvoiceDepositsSection.tsx is ~1363 lines, and the per-row overflow menu (trigger button + menu items + keyboard navigation + focus management) is duplicated almost verbatim between DepositRow (desktop table) and DepositCard (mobile cards) — ~330 lines duplicated.

Extract a single shared OverflowMenu component to client/src/components/OverflowMenu/ per the project's "Component Reuse Policy" (per CLAUDE.md: "every new component must be built as a reusable shared component — no one-off implementations"). The component should expose:

• items: Array<{ label: string; onClick: () => void; ... }> (or similar) for the menu items
• triggerAriaLabel: string for accessibility
• Full WAI-ARIA Menu Button keyboard nav (ArrowUp/Down/Home/End/Escape, focus return on close) — same behaviour as the inline implementation today

Important

Per project policy, every new shared component requires a ux-designer visual spec before implementation begins. The orchestrator must launch the ux-designer agent for the visual spec phase prior to launching the frontend-developer.

3. Replace inline style={{}} on table row with CSS Module class

InvoiceDepositsSection.tsx contains an inline style={{ ... }} on a table row (architect flagged it as the same anti-pattern as the recent fd73bcad fix on the budget-line form). Move the inline declaration to a named class in InvoiceDepositsSection.module.css.

4. Surface API errors in revert handlers

In InvoiceDepositsSection.tsx, the handleRevertToPending, handleRevertToPaid, and handleStateConfirm functions catch API errors silently — the user gets no feedback when a transition fails (e.g., on INVALID_DEPOSIT_STATUS_TRANSITION or network errors). The PR-review by product-owner and architect both flagged this as a UX gap.

Surface the error to the user via the existing toast/banner pattern or via FormError in the relevant modal. For revert actions (which fire PATCH immediately without a modal), surface a transient error banner or toast at the section level. Use the existing translateApiError(code, tErrors) helper for code mapping.

Acceptance Criteria

• AC-1 A shared splitByDeposits(rows) function exists in server/src/services/shared/depositAggregateUtils.ts and is reused by computeDepositAwareAggregates, computeStatusContribution, aggregateInvoiceStatusBreakdown, AND computeDiscretionaryInvoiceAmount in budgetSourceService.ts. Behaviour preserved exactly — existing tests still pass without modification.
• AC-2 A reusable OverflowMenu shared component exists under client/src/components/OverflowMenu/. DepositRow and DepositCard in InvoiceDepositsSection.tsx both consume it instead of duplicating the menu code. Full WAI-ARIA Menu Button keyboard nav preserved (ArrowUp/Down/Home/End/Escape, focus return on close). Existing E2E tests for the deposit menu pass without modification.
• AC-3 No style={{}} JSX prop remains in InvoiceDepositsSection.tsx — moved to InvoiceDepositsSection.module.css with named classes.
• AC-4 The three revert handlers in InvoiceDepositsSection.tsx (handleRevertToPending, handleRevertToPaid, handleStateConfirm) surface API errors to the user. Server error codes are translated via translateApiError(code, tErrors). Network errors show a generic translated message. Use the existing FormError shared component for modal-based actions and an inline transient banner for the menu-driven Revert actions (which don't open a modal). Add at least one E2E test that verifies the error is visible after a forced failure (or, if E2E setup is too heavy, an integration/component test that simulates the API rejection).
• AC-5 All existing tests pass without modification (refactor is behaviour-preserving for AC-1 through AC-3). New tests added for AC-4 (error visibility).
• AC-6 No new dependencies, no schema changes, no API contract changes.
• AC-7 UX-designer approves the visual spec for the new OverflowMenu shared component before implementation begins.

Non-Goals

• No UX behaviour changes for items 1-3 (pure refactors).
• No new deposit-related features.
• The mobile bottom-sheet modal (separate UX issue, file separately if needed).
• Other UX-designer nits (focus ring tokens, count chip tokens, statusQuotation dark mode, tablet column hiding) — file as separate refinement issues if desired; these are not part of "architect's recommendations."

References

• PR feat(invoice,budget): invoice deposits UI + deposit-aware budget rollups (#1404, #1405) #1407 architect review: feat(invoice,budget): invoice deposits UI + deposit-aware budget rollups (#1404, #1405) #1407 (comment)
• PR fix(invoice): make status breakdown summary deposit-aware (#1411) #1412 architect review: fix(invoice): make status breakdown summary deposit-aware (#1411) #1412 (comment)
• Related closed work: Invoice deposits: detail-page UI (Deposits section, Final payment row, add/edit/delete + state toggles) #1404, Invoice deposits: budget rollup integration (residual under parent status, deposits under their own state) #1405, Invoice status summary breakdown is not deposit-aware (InvoicesPage header + Dashboard pipeline card) #1411
• Inline-style precedent fix: commit fd73bcad

[steilerDev]

[ux-designer]

Visual Specification — OverflowMenu Shared Component

Component path

client/src/components/OverflowMenu/
  OverflowMenu.tsx
  OverflowMenu.module.css
  index.ts          ← re-export

---

Naming decision

OverflowMenu is the correct name for this project. It is distinct from:

• ContextMenu — implies right-click / pointer-event invocation
• DropdownMenu — implies a nav/select affordance, not a per-row action tray
• KebabMenu — informal; describes the icon, not the pattern

OverflowMenu accurately describes a button that reveals a set of actions that could not fit inline. It matches the existing WAI-ARIA Menu Button pattern already implemented.

---

Props API

interface OverflowMenuItem {
  label: string;
  onClick: () => void;
  disabled?: boolean;
  variant?: 'default' | 'destructive';   // default: 'default'
  icon?: React.ReactNode;                 // optional leading icon
}

interface OverflowMenuProps {
  items: OverflowMenuItem[];
  triggerAriaLabel: string;              // required — must name the entity, e.g. "Actions for deposit 2024-06-01"
  triggerIcon?: React.ReactNode;         // default: the ⋮ character
  placement?: 'bottom-end' | 'top-end'; // default: 'bottom-end'
  disabled?: boolean;                    // disables the trigger button
  'data-testid'?: string;               // forwarded to the trigger button
}

Design intent: items is a flat list; the caller is responsible for filtering items based on entity state before passing them in. Dividers are not needed for the current consumers; add a { divider: true } discriminated union member only if a future consumer requires it.

---

Anatomy

┌──────────────────────────────────────┐
│  [trigger button]  ⋮  44×44 px       │
└──────────────────────────────────────┘
                            │  open →
                  ┌─────────────────────┐
                  │  [menuitem]  Edit   │  ← 36px desktop / 44px mobile
                  │  [menuitem]  Mark   │
                  │  [menuitem danger]  Delete │
                  └─────────────────────┘
                  min-width: 160px, right-aligned to trigger

---

Token Mapping

Element	Property	Token
Trigger button	color (idle)	var(--color-text-secondary)
Trigger button	font-size	var(--font-size-lg)
Trigger button	border-radius	var(--radius-md)
Trigger button	background (idle)	none (background: none)
Trigger button	background (hover)	var(--color-bg-tertiary)
Trigger button	color (hover)	var(--color-text-primary)
Trigger button	transition	var(--transition-button)
Trigger button	box-shadow (focus-visible)	var(--shadow-focus)
Trigger button	opacity (disabled)	0.5
Dropdown surface	background	var(--color-bg-primary)
Dropdown surface	border	1px solid var(--color-border-strong)
Dropdown surface	border-radius	var(--radius-md)
Dropdown surface	box-shadow	var(--shadow-md)
Dropdown surface	z-index	var(--z-dropdown)
Menu item (default)	color	var(--color-text-primary)
Menu item (default)	font-size	var(--font-size-sm)
Menu item (default)	background (hover)	var(--color-bg-secondary)
Menu item (default)	background (focus-visible)	var(--color-bg-secondary)
Menu item (default)	box-shadow (focus-visible)	var(--shadow-focus) — inset 3px ring
Menu item (default)	transition	var(--transition-button)
Menu item (destructive)	color	var(--color-danger-text-on-light)
Menu item (destructive)	background (hover)	var(--color-danger-bg)
Menu item (destructive)	box-shadow (focus-visible)	var(--shadow-focus-danger) — inset 3px ring
Optional leading icon	color	inherits from item color

Focus ring correction: The existing inline implementation uses box-shadow: inset 0 0 0 2px var(--color-primary) on .menuItem:focus-visible. This must be replaced with box-shadow: inset 0 0 0 3px var(--color-focus-ring) — matching the project's --shadow-focus semantic (3px ring, blue semi-transparent). Use the inset form so it does not overflow the dropdown surface clipping boundary. For the destructive item use inset 0 0 0 3px var(--color-focus-ring-danger).

The countChip note from the prior #1407 review was about a different element (the heading count badge). The menu trigger itself does not need --color-bg-tertiary / --color-text-muted — it is a ghost icon button that uses --color-text-secondary at rest and --color-bg-tertiary on hover, which is correct as-is.

---

Dimensions and Touch Targets

/* Trigger button */
min-width: 44px;
min-height: 44px;
padding: var(--spacing-2);

/* Menu item — desktop */
padding: var(--spacing-2-5) var(--spacing-3);   /* 10px 12px */
/* results in ~36px height at font-size-sm (14px) + line-height ~1.5 */

/* Menu item — mobile (≤ 767px) */
padding: var(--spacing-3) var(--spacing-4);      /* 12px 16px */
/* ensures each item reaches the 44px minimum touch target height */

/* Dropdown min-width */
min-width: 160px;

---

Placement Logic

placement controls the CSS top / bottom anchor of the dropdown panel relative to the trigger:

• 'bottom-end' (default): top: 100%; right: 0 — panel opens below, right-aligned
• 'top-end': bottom: 100%; right: 0 — panel opens above, right-aligned (use when trigger is near the bottom of a scroll container)

The placement prop maps directly to a CSS class on the dropdown <div> — no JS position calculation is needed for these two cases.

.menuBottom {
  top: 100%;
  right: 0;
}

.menuTop {
  bottom: 100%;
  right: 0;
}

---

DOM Structure and ARIA

<div className={styles.wrapper}>
  {/* Trigger */}
  <button
    type="button"
    className={styles.trigger}
    aria-haspopup="true"
    aria-expanded={isOpen}
    aria-label={triggerAriaLabel}
    data-testid={props['data-testid']}
    disabled={disabled}
  >
    {triggerIcon ?? '⋮'}
  </button>

  {/* Dropdown — conditionally rendered */}
  {isOpen && (
    <div
      role="menu"
      className={`${styles.menu} ${styles[placementClass]}`}
    >
      {items.map((item, i) => (
        <button
          key={i}
          type="button"
          role="menuitem"
          className={`${styles.item} ${item.variant === 'destructive' ? styles.itemDanger : ''}`}
          onClick={...}
          disabled={item.disabled}
        >
          {item.icon && <span className={styles.itemIcon} aria-hidden="true">{item.icon}</span>}
          {item.label}
        </button>
      ))}
    </div>
  )}
</div>

The wrapper must be position: relative so the absolutely-positioned menu aligns to it.

No aria-controls needed — the menu is a child of the trigger's container and conditionally mounted; assistive technologies locate it via the adjacent role="menu".

---

Keyboard Navigation

Preserving the existing WAI-ARIA Menu Button pattern exactly:

Key	Trigger (closed)	Inside menu (open)
Enter / Space	Open menu, focus first item	Activate item, close menu
ArrowDown	Open menu, focus first item	Move focus to next item (wraps)
ArrowUp	—	Move focus to previous item (wraps)
Home	—	Move focus to first item
End	—	Move focus to last item
Escape	—	Close menu, return focus to trigger
Tab	—	Close menu (natural tab-out; do not intercept Tab key)

Disabled items: Render with disabled attribute. They must be skipped by the arrow-key handler (querySelectorAll('[role="menuitem"]:not(:disabled)')).

Focus on open: When the menu opens (by any method), focus the first non-disabled menuitem. Use setTimeout(0) to defer focus until after the element is mounted in the DOM — matching the existing implementation.

---

Close-on-outside-click

The component must close when the user clicks outside the wrapper. Implement via a mousedown listener on document that checks whether the event target is contained within the wrapper ref. Clean up the listener on unmount and when the menu closes.

The existing implementation in InvoiceDepositsSection delegates open-state management to the parent via menuOpenId + onMenuToggle. The extracted component should own this state internally: const [isOpen, setIsOpen] = useState(false). This removes the 330-line duplication and prevents external state leakage.

---

Animations and Transitions

All interactive state transitions use var(--transition-button) (background-color + box-shadow).

The dropdown panel itself does not animate by default (consistent with other dropdowns in the codebase — SearchPicker, etc.). If an entrance animation is added later it must be guarded:

@media (prefers-reduced-motion: no-preference) {
  .menu {
    animation: menuFadeIn var(--transition-fast) ease;
  }
}

@keyframes menuFadeIn {
  from { opacity: 0; transform: translateY(-4px); }
  to   { opacity: 1; transform: translateY(0); }
}

For the initial extraction, skip the animation entirely. Add it in a refinement if desired.

---

Dark Mode

All referenced tokens have Layer 3 dark-mode overrides — no additional [data-theme="dark"] block is needed in OverflowMenu.module.css. Token cascade:

Token	Light	Dark
--color-text-secondary	#374151 (gray-700)	#cbd5e1 (slate-150)
--color-text-primary	#111827 (gray-900)	#f1f5f9 (slate-50)
--color-bg-tertiary	#f3f4f6 (gray-100)	#1e293b (slate-600)
--color-bg-primary	#ffffff	#1a1a2e (slate-800)
--color-bg-secondary	#f9fafb (gray-50)	#16213e (slate-700)
--color-border-strong	#d1d5db (gray-300)	#475569 (slate-400)
--color-shadow-md	rgba(0,0,0,0.1)	rgba(0,0,0,0.3)
--color-danger-text-on-light	#991b1b (red-700)	#fca5a5 (red-300)
--color-danger-bg	#fef2f2 (red-50)	rgba(239,68,68,0.1)

Contrast passes WCAG AA in both modes — no special treatment needed.

---

Responsive Behavior

The component renders identically on desktop, tablet, and mobile — the trigger is always a 44×44 px touch target. On mobile (≤ 767px) the menu items gain extra padding to reach the 44 px minimum touch target height (see Dimensions above). No structural layout changes.

---

Component Mapping

UI Element	Shared Component	Props / Notes
Overflow trigger + menu	OverflowMenu (new)	This component itself
Confirmation dialogs	Modal (existing)	Called by the parent page, not by OverflowMenu

---

E2E Locator Compatibility

The existing E2E tests use button[aria-haspopup="true"] for the trigger and role="menu" / role="menuitem" for the dropdown. The spec preserves both attributes exactly — the extraction is a drop-in replacement with no locator changes needed.

---

What is NOT in scope for this component

• Confirmation modals before destructive actions — these remain in the parent consumer
• Business logic (status transitions, API calls) — caller passes fully-resolved onClick handlers
• Tooltip on the trigger — callers can wrap in a <Tooltip> if needed

[github-actions]

🎉 This issue has been resolved in version 2.6.0-beta.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀