From a6ee64ba493de7e8fd5bf24d3fd2d1ea6b787046 Mon Sep 17 00:00:00 2001 From: Frank Steiler Date: Mon, 11 May 2026 21:33:38 +0200 Subject: [PATCH 1/2] chore(invoice,budget): dedupe split helper, extract OverflowMenu, surface revert errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the architect's medium-severity recommendations from the #1407 and #1412 reviews. Four scopes: 1. Extract splitByDeposits() helper in depositAggregateUtils.ts and reuse across the 4 call sites that inlined the proportional-split + dedup pattern (computeDepositAwareAggregates, computeStatusContribution, aggregateInvoiceStatusBreakdown, and computeDiscretionaryInvoiceAmount in budgetSourceService.ts). Behaviour-preserving — existing tests pass unmodified. 2. Extract a shared OverflowMenu component (client/src/components/ OverflowMenu/). DepositRow and DepositCard both consume it instead of duplicating ~330 lines of menu code. Full WAI-ARIA Menu Button keyboard nav, mobile 44px touch targets, design-token-only CSS, dark mode handled by the token cascade. Same aria-haspopup/role attributes as the inline implementation — existing E2E locators still work. 3. Replace inline style={{}} on the opacity transition with a CSS module class (.tableRowMutating), matching the prior fd73bcad fix. 4. Surface API errors in handleRevertToPending, handleRevertToPaid, and handleStateConfirm. Menu-driven reverts show a section-level FormError banner (auto-dismiss 6s). State-confirm modal shows FormError inside the dialog. Two new i18n keys for network-error fallbacks; existing translateApiError() covers coded server errors. Fixes #1413 Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) Co-Authored-By: Claude backend-developer (Haiku 4.5) Co-Authored-By: Claude frontend-developer (Haiku 4.5) Co-Authored-By: Claude translator (Sonnet 4.6) Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) --- .../OverflowMenu/OverflowMenu.module.css | 123 ++++ .../OverflowMenu/OverflowMenu.test.tsx | 470 +++++++++++++ .../components/OverflowMenu/OverflowMenu.tsx | 153 +++++ client/src/components/OverflowMenu/index.ts | 1 + client/src/i18n/de/budget.json | 4 +- client/src/i18n/en/budget.json | 4 +- .../InvoiceDepositsSection.module.css | 88 +-- .../InvoiceDepositsSection.test.tsx | 176 ++++- .../InvoiceDepositsSection.tsx | 640 +++++------------- .../InvoiceDetailPage/InvoiceDetailPage.tsx | 1 - e2e/tests/invoices/invoice-deposits.spec.ts | 96 +++ server/src/services/budgetSourceService.ts | 77 +-- .../shared/depositAggregateUtils.test.ts | 171 +++++ .../services/shared/depositAggregateUtils.ts | 281 ++++---- 14 files changed, 1569 insertions(+), 716 deletions(-) create mode 100644 client/src/components/OverflowMenu/OverflowMenu.module.css create mode 100644 client/src/components/OverflowMenu/OverflowMenu.test.tsx create mode 100644 client/src/components/OverflowMenu/OverflowMenu.tsx create mode 100644 client/src/components/OverflowMenu/index.ts diff --git a/client/src/components/OverflowMenu/OverflowMenu.module.css b/client/src/components/OverflowMenu/OverflowMenu.module.css new file mode 100644 index 00000000..281f2bd6 --- /dev/null +++ b/client/src/components/OverflowMenu/OverflowMenu.module.css @@ -0,0 +1,123 @@ +.wrapper { + position: relative; + display: flex; + align-items: center; + justify-content: center; +} + +.trigger { + background: none; + border: none; + padding: var(--spacing-2); + min-width: 44px; + min-height: 44px; + display: flex; + align-items: center; + justify-content: center; + cursor: pointer; + color: var(--color-text-secondary); + font-size: var(--font-size-lg); + border-radius: var(--radius-md); + transition: var(--transition-button); +} + +.trigger:hover:not(:disabled) { + background-color: var(--color-bg-tertiary); + color: var(--color-text-primary); +} + +.trigger:focus-visible { + outline: none; + box-shadow: var(--shadow-focus); +} + +.trigger:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +@media (prefers-reduced-motion: reduce) { + .trigger { + transition: none; + } +} + +.menu { + position: absolute; + background-color: var(--color-bg-primary); + border: 1px solid var(--color-border-strong); + border-radius: var(--radius-md); + box-shadow: var(--shadow-md); + z-index: var(--z-dropdown); + min-width: 160px; + overflow: hidden; +} + +.menuBottom { + top: 100%; + right: 0; + margin-top: var(--spacing-1); +} + +.menuTop { + bottom: 100%; + right: 0; + margin-bottom: var(--spacing-1); +} + +.item { + display: flex; + width: 100%; + padding: var(--spacing-2-5) var(--spacing-3); + background: none; + border: none; + text-align: left; + cursor: pointer; + color: var(--color-text-primary); + font-size: var(--font-size-sm); + transition: var(--transition-button); + align-items: center; + gap: var(--spacing-2); +} + +.item:hover:not(:disabled) { + background-color: var(--color-bg-secondary); +} + +.item:focus-visible { + outline: none; + background-color: var(--color-bg-secondary); + box-shadow: inset 0 0 0 2px var(--color-primary); +} + +.item:disabled { + opacity: 0.5; + cursor: not-allowed; +} + +@media (prefers-reduced-motion: reduce) { + .item { + transition: none; + } +} + +.itemIcon { + display: flex; + align-items: center; + justify-content: center; + flex-shrink: 0; +} + +.itemDanger { + color: var(--color-danger-text-on-light); +} + +.itemDanger:hover:not(:disabled) { + background-color: var(--color-danger-bg); +} + +@media (max-width: 767px) { + .item { + padding: var(--spacing-3) var(--spacing-4); + } +} diff --git a/client/src/components/OverflowMenu/OverflowMenu.test.tsx b/client/src/components/OverflowMenu/OverflowMenu.test.tsx new file mode 100644 index 00000000..9cadf80e --- /dev/null +++ b/client/src/components/OverflowMenu/OverflowMenu.test.tsx @@ -0,0 +1,470 @@ +/** + * @jest-environment jsdom + */ +import { describe, it, expect, jest, beforeEach } from '@jest/globals'; +import { render, screen, fireEvent, act, waitFor } from '@testing-library/react'; +import { OverflowMenu, type OverflowMenuItem } from './OverflowMenu.js'; + +// ─── CSS Module note ────────────────────────────────────────────────────────── +// identity-obj-proxy returns the class key itself as the class name. +// So styles.itemDanger === 'itemDanger', styles.menuTop === 'menuTop', etc. +// ───────────────────────────────────────────────────────────────────────────── + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +function buildItems(count = 2, overrides: Partial[] = []): OverflowMenuItem[] { + return Array.from({ length: count }, (_, i) => ({ + id: `item-${i}`, + label: `Item ${i}`, + onClick: jest.fn<() => void>(), + ...overrides[i], + })); +} + +function renderMenu(props: Partial[0]> & { items?: OverflowMenuItem[] } = {}) { + const items = props.items ?? buildItems(3); + return render( + , + ); +} + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +describe('OverflowMenu', () => { + beforeEach(() => { + // Use real timers unless a test overrides this + jest.useRealTimers(); + }); + + // ─── Scenario 1: Renders trigger ────────────────────────────────────────── + + describe('Scenario 1: renders trigger button', () => { + it('renders a button with aria-haspopup="true"', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + expect(trigger).toHaveAttribute('aria-haspopup', 'true'); + }); + + it('trigger has aria-expanded="false" initially', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + }); + + it('menu is not in DOM initially', () => { + renderMenu(); + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + }); + + // ─── Scenario 2: Opens on click ─────────────────────────────────────────── + + describe('Scenario 2: opens on click', () => { + it('click on trigger renders role="menu"', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + fireEvent.click(trigger); + expect(screen.getByRole('menu')).toBeInTheDocument(); + }); + + it('trigger aria-expanded becomes "true" when open', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + fireEvent.click(trigger); + expect(trigger).toHaveAttribute('aria-expanded', 'true'); + }); + + it('second click closes the menu again', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + fireEvent.click(trigger); + fireEvent.click(trigger); + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + }); + + // ─── Scenario 3: Renders all items ─────────────────────────────────────── + + describe('Scenario 3: renders all menu items', () => { + it('renders one menuitem per item provided', () => { + const items = buildItems(4); + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + expect(screen.getAllByRole('menuitem')).toHaveLength(4); + }); + + it('each menuitem renders with the correct label', () => { + const items = buildItems(2); + items[0]!.label = 'Edit item'; + items[1]!.label = 'Delete item'; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + expect(screen.getByRole('menuitem', { name: /edit item/i })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: /delete item/i })).toBeInTheDocument(); + }); + }); + + // ─── Scenario 4: Destructive variant ───────────────────────────────────── + + describe('Scenario 4: destructive variant', () => { + it('destructive item has CSS class containing "itemDanger"', () => { + const items: OverflowMenuItem[] = [ + { id: 'del', label: 'Delete', onClick: jest.fn<() => void>(), variant: 'destructive' }, + ]; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + const delBtn = screen.getByRole('menuitem', { name: 'Delete' }); + // identity-obj-proxy returns 'itemDanger' as the class name + expect(delBtn.className).toContain('itemDanger'); + }); + + it('default variant item does NOT have "itemDanger" class', () => { + const items: OverflowMenuItem[] = [ + { id: 'edit', label: 'Edit', onClick: jest.fn<() => void>(), variant: 'default' }, + ]; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + const editBtn = screen.getByRole('menuitem', { name: 'Edit' }); + expect(editBtn.className).not.toContain('itemDanger'); + }); + }); + + // ─── Scenario 5: Disabled item ──────────────────────────────────────────── + + describe('Scenario 5: disabled item', () => { + it('disabled item has the disabled attribute', () => { + const items: OverflowMenuItem[] = [ + { id: 'act', label: 'Action', onClick: jest.fn<() => void>(), disabled: true }, + ]; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + const btn = screen.getByRole('menuitem', { name: 'Action' }); + expect(btn).toBeDisabled(); + }); + + it('clicking a disabled item does not call onClick', () => { + const onClick = jest.fn<() => void>(); + const items: OverflowMenuItem[] = [ + { id: 'act', label: 'Disabled Action', onClick, disabled: true }, + ]; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + fireEvent.click(screen.getByRole('menuitem', { name: 'Disabled Action' })); + expect(onClick).not.toHaveBeenCalled(); + }); + }); + + // ─── Scenario 6: Click item calls onClick and closes menu ───────────────── + + describe('Scenario 6: click item calls onClick and closes menu', () => { + it('clicking an enabled item calls its onClick handler', () => { + const onClick = jest.fn<() => void>(); + const items: OverflowMenuItem[] = [{ id: 'go', label: 'Go', onClick }]; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + fireEvent.click(screen.getByRole('menuitem', { name: 'Go' })); + expect(onClick).toHaveBeenCalledTimes(1); + }); + + it('clicking an item closes the menu', () => { + const items = buildItems(1); + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + fireEvent.click(screen.getByRole('menuitem', { name: 'Item 0' })); + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + }); + + // ─── Scenario 7: Escape closes menu ────────────────────────────────────── + + describe('Scenario 7: Escape key closes menu and returns focus to trigger', () => { + it('Escape key closes the menu', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + fireEvent.click(trigger); + expect(screen.getByRole('menu')).toBeInTheDocument(); + + fireEvent.keyDown(screen.getByRole('menu'), { key: 'Escape' }); + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + + it('after Escape, trigger has aria-expanded="false"', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + fireEvent.click(trigger); + fireEvent.keyDown(screen.getByRole('menu'), { key: 'Escape' }); + expect(trigger).toHaveAttribute('aria-expanded', 'false'); + }); + }); + + // ─── Scenario 8: ArrowDown navigation ───────────────────────────────────── + + describe('Scenario 8: ArrowDown focuses next item, wraps at end', () => { + it('ArrowDown moves focus to next item', async () => { + renderMenu({ items: buildItems(3) }); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + fireEvent.click(trigger); + + const items = screen.getAllByRole('menuitem'); + items[0]!.focus(); + + fireEvent.keyDown(screen.getByRole('menu'), { key: 'ArrowDown' }); + expect(document.activeElement).toBe(items[1]); + }); + + it('ArrowDown wraps from last item to first', () => { + renderMenu({ items: buildItems(3) }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + + const items = screen.getAllByRole('menuitem'); + items[2]!.focus(); + + fireEvent.keyDown(screen.getByRole('menu'), { key: 'ArrowDown' }); + expect(document.activeElement).toBe(items[0]); + }); + }); + + // ─── Scenario 9: ArrowUp navigation ────────────────────────────────────── + + describe('Scenario 9: ArrowUp focuses previous item, wraps at start', () => { + it('ArrowUp moves focus to previous item', () => { + renderMenu({ items: buildItems(3) }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + + const items = screen.getAllByRole('menuitem'); + items[2]!.focus(); + + fireEvent.keyDown(screen.getByRole('menu'), { key: 'ArrowUp' }); + expect(document.activeElement).toBe(items[1]); + }); + + it('ArrowUp wraps from first item to last', () => { + renderMenu({ items: buildItems(3) }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + + const items = screen.getAllByRole('menuitem'); + items[0]!.focus(); + + fireEvent.keyDown(screen.getByRole('menu'), { key: 'ArrowUp' }); + expect(document.activeElement).toBe(items[2]); + }); + }); + + // ─── Scenario 10: Home / End ────────────────────────────────────────────── + + describe('Scenario 10: Home/End keys', () => { + it('Home key focuses first item', () => { + renderMenu({ items: buildItems(4) }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + + const items = screen.getAllByRole('menuitem'); + items[3]!.focus(); + + fireEvent.keyDown(screen.getByRole('menu'), { key: 'Home' }); + expect(document.activeElement).toBe(items[0]); + }); + + it('End key focuses last item', () => { + renderMenu({ items: buildItems(4) }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + + const items = screen.getAllByRole('menuitem'); + items[0]!.focus(); + + fireEvent.keyDown(screen.getByRole('menu'), { key: 'End' }); + expect(document.activeElement).toBe(items[3]); + }); + }); + + // ─── Scenario 11: Outside click closes menu ────────────────────────────── + + describe('Scenario 11: outside mousedown closes the menu', () => { + it('mousedown on document.body closes the menu', async () => { + renderMenu(); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + expect(screen.getByRole('menu')).toBeInTheDocument(); + + await act(async () => { + fireEvent.mouseDown(document.body); + }); + + await waitFor(() => { + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + }); + + it('mousedown inside the wrapper does NOT close the menu', () => { + const { container } = renderMenu(); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + + // Fire mousedown on the wrapper itself (inside) + fireEvent.mouseDown(container.firstChild as Element); + expect(screen.getByRole('menu')).toBeInTheDocument(); + }); + }); + + // ─── Scenario 12: Disabled trigger ─────────────────────────────────────── + + describe('Scenario 12: disabled trigger does not open menu', () => { + it('clicking a disabled trigger does not render the menu', () => { + renderMenu({ disabled: true }); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + expect(trigger).toBeDisabled(); + fireEvent.click(trigger); + expect(screen.queryByRole('menu')).not.toBeInTheDocument(); + }); + }); + + // ─── Scenario 13: triggerIcon prop ─────────────────────────────────────── + + describe('Scenario 13: triggerIcon prop', () => { + it('renders the default "⋮" trigger icon when no triggerIcon prop given', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + expect(trigger.textContent).toContain('⋮'); + }); + + it('renders a custom trigger icon when triggerIcon prop is provided', () => { + renderMenu({ triggerIcon: X }); + const icon = screen.getByTestId('custom-icon'); + expect(icon).toBeInTheDocument(); + expect(icon.textContent).toBe('X'); + }); + }); + + // ─── Scenario 14: placement="top-end" ──────────────────────────────────── + + describe('Scenario 14: placement="top-end" applies menuTop class', () => { + it('menu element has "menuTop" class when placement="top-end"', () => { + renderMenu({ placement: 'top-end' }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + const menu = screen.getByRole('menu'); + // identity-obj-proxy returns 'menuTop' as the class name + expect(menu.className).toContain('menuTop'); + }); + + it('menu element has "menuBottom" class when placement="bottom-end" (default)', () => { + renderMenu({ placement: 'bottom-end' }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + const menu = screen.getByRole('menu'); + expect(menu.className).toContain('menuBottom'); + }); + }); + + // ─── Scenario 15: data-testid forwarded ────────────────────────────────── + + describe('Scenario 15: data-testid forwarded to trigger', () => { + it('trigger button has the data-testid attribute when provided', () => { + renderMenu({ 'data-testid': 'my-overflow-menu' }); + const trigger = screen.getByTestId('my-overflow-menu'); + expect(trigger).toBeInTheDocument(); + expect(trigger.tagName.toLowerCase()).toBe('button'); + }); + + it('no data-testid on trigger when prop is not provided', () => { + renderMenu(); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + expect(trigger.getAttribute('data-testid')).toBeNull(); + }); + }); + + // ─── ArrowDown opens menu from trigger ─────────────────────────────────── + + describe('ArrowDown on closed trigger opens menu and focuses first item', () => { + it('ArrowDown on trigger opens the menu', async () => { + jest.useFakeTimers(); + renderMenu({ items: buildItems(2) }); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + + act(() => { + fireEvent.keyDown(trigger, { key: 'ArrowDown' }); + }); + + expect(screen.getByRole('menu')).toBeInTheDocument(); + jest.useRealTimers(); + }); + + it('ArrowDown on trigger focuses first menuitem after setTimeout(0)', async () => { + jest.useFakeTimers(); + renderMenu({ items: buildItems(2) }); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + + act(() => { + fireEvent.keyDown(trigger, { key: 'ArrowDown' }); + }); + + // Advance timers to execute the setTimeout(0) callback that focuses first item + await act(async () => { + jest.runAllTimers(); + await Promise.resolve(); + }); + + const items = screen.getAllByRole('menuitem'); + expect(document.activeElement).toBe(items[0]); + + jest.useRealTimers(); + }); + + it('ArrowDown on trigger when menu is already open does not close it', () => { + renderMenu({ items: buildItems(2) }); + const trigger = screen.getByRole('button', { name: 'Open menu' }); + // Open via click + fireEvent.click(trigger); + expect(screen.getByRole('menu')).toBeInTheDocument(); + + // ArrowDown when already open: handleTriggerKeyDown guard `!isOpen` is false, no-op + fireEvent.keyDown(trigger, { key: 'ArrowDown' }); + // Menu should still be open + expect(screen.getByRole('menu')).toBeInTheDocument(); + }); + }); + + // ─── Item without id uses fallback key ──────────────────────────────────── + + describe('item without id uses fallback key (item-{i})', () => { + it('renders items correctly when no id is provided', () => { + const items: OverflowMenuItem[] = [ + { label: 'No ID Item A', onClick: jest.fn<() => void>() }, + { label: 'No ID Item B', onClick: jest.fn<() => void>() }, + ]; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + expect(screen.getByRole('menuitem', { name: 'No ID Item A' })).toBeInTheDocument(); + expect(screen.getByRole('menuitem', { name: 'No ID Item B' })).toBeInTheDocument(); + }); + }); + + // ─── Item with icon ──────────────────────────────────────────────────────── + + describe('item with icon renders icon span', () => { + it('item icon is rendered inside an aria-hidden span', () => { + const items: OverflowMenuItem[] = [ + { + id: 'icon-item', + label: 'With Icon', + onClick: jest.fn<() => void>(), + icon: 🗑, + }, + ]; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + expect(screen.getByTestId('menu-item-icon')).toBeInTheDocument(); + }); + + it('item without icon does not render icon span', () => { + const items: OverflowMenuItem[] = [ + { id: 'no-icon', label: 'No Icon', onClick: jest.fn<() => void>() }, + ]; + renderMenu({ items }); + fireEvent.click(screen.getByRole('button', { name: 'Open menu' })); + const menuItem = screen.getByRole('menuitem', { name: 'No Icon' }); + // No child span with aria-hidden should exist + const iconSpan = menuItem.querySelector('[aria-hidden="true"]'); + expect(iconSpan).toBeNull(); + }); + }); +}); diff --git a/client/src/components/OverflowMenu/OverflowMenu.tsx b/client/src/components/OverflowMenu/OverflowMenu.tsx new file mode 100644 index 00000000..9ae0296b --- /dev/null +++ b/client/src/components/OverflowMenu/OverflowMenu.tsx @@ -0,0 +1,153 @@ +import { useState, useRef, useEffect, type ReactNode } from 'react'; +import styles from './OverflowMenu.module.css'; + +export interface OverflowMenuItem { + id?: string; + label: string; + onClick: () => void; + disabled?: boolean; + variant?: 'default' | 'destructive'; + icon?: ReactNode; +} + +export interface OverflowMenuProps { + items: OverflowMenuItem[]; + triggerAriaLabel: string; + triggerIcon?: ReactNode; + placement?: 'bottom-end' | 'top-end'; + disabled?: boolean; + 'data-testid'?: string; +} + +export function OverflowMenu({ + items, + triggerAriaLabel, + triggerIcon = '⋮', + placement = 'bottom-end', + disabled = false, + 'data-testid': dataTestId, +}: OverflowMenuProps) { + const [isOpen, setIsOpen] = useState(false); + const wrapperRef = useRef(null); + const triggerRef = useRef(null); + const menuRef = useRef(null); + + // Close menu on outside click + useEffect(() => { + if (!isOpen) return; + + const handleMouseDown = (e: MouseEvent) => { + if (wrapperRef.current && !wrapperRef.current.contains(e.target as Node)) { + setIsOpen(false); + } + }; + + document.addEventListener('mousedown', handleMouseDown); + return () => document.removeEventListener('mousedown', handleMouseDown); + }, [isOpen]); + + // Keyboard navigation + const handleTriggerKeyDown = (e: React.KeyboardEvent) => { + if (e.key === 'ArrowDown' && !isOpen) { + e.preventDefault(); + setIsOpen(true); + // Focus first item after state updates + setTimeout(() => { + const firstMenuItem = menuRef.current?.querySelector( + '[role="menuitem"]', + ) as HTMLButtonElement; + firstMenuItem?.focus(); + }, 0); + } + }; + + const handleMenuKeyDown = (e: React.KeyboardEvent) => { + const menuItems = menuRef.current?.querySelectorAll('[role="menuitem"]'); + if (!menuItems || menuItems.length === 0) return; + + const currentIndex = Array.from(menuItems).findIndex( + (item) => item === document.activeElement, + ); + + switch (e.key) { + case 'ArrowDown': { + e.preventDefault(); + const nextIndex = currentIndex === menuItems.length - 1 ? 0 : currentIndex + 1; + (menuItems[nextIndex] as HTMLButtonElement).focus(); + break; + } + case 'ArrowUp': { + e.preventDefault(); + const prevIndex = currentIndex === 0 ? menuItems.length - 1 : currentIndex - 1; + (menuItems[prevIndex] as HTMLButtonElement).focus(); + break; + } + case 'Home': { + e.preventDefault(); + (menuItems[0] as HTMLButtonElement).focus(); + break; + } + case 'End': { + e.preventDefault(); + (menuItems[menuItems.length - 1] as HTMLButtonElement).focus(); + break; + } + case 'Escape': { + e.preventDefault(); + setIsOpen(false); + triggerRef.current?.focus(); + break; + } + } + }; + + const handleItemClick = (item: OverflowMenuItem) => { + setIsOpen(false); + item.onClick(); + }; + + return ( +
+ + {isOpen && ( +
+ {items.map((item, i) => ( + + ))} +
+ )} +
+ ); +} diff --git a/client/src/components/OverflowMenu/index.ts b/client/src/components/OverflowMenu/index.ts new file mode 100644 index 00000000..0684f9a2 --- /dev/null +++ b/client/src/components/OverflowMenu/index.ts @@ -0,0 +1 @@ +export { OverflowMenu, type OverflowMenuProps, type OverflowMenuItem } from './OverflowMenu.js'; diff --git a/client/src/i18n/de/budget.json b/client/src/i18n/de/budget.json index 519efea7..fdf872d0 100644 --- a/client/src/i18n/de/budget.json +++ b/client/src/i18n/de/budget.json @@ -547,7 +547,9 @@ "loadError": "Abschlagszahlungen konnten nicht geladen werden. Bitte versuchen Sie es erneut.", "saveError": "Abschlagszahlung konnte nicht gespeichert werden. Bitte versuchen Sie es erneut.", "deleteError": "Abschlagszahlung konnte nicht gelöscht werden. Bitte versuchen Sie es erneut.", - "revertError": "Status der Abschlagszahlung konnte nicht zurückgesetzt werden. Bitte versuchen Sie es erneut." + "revertError": "Status der Abschlagszahlung konnte nicht zurückgesetzt werden. Bitte versuchen Sie es erneut.", + "revertNetworkError": "Netzwerkfehler – Status der Abschlagszahlung konnte nicht zurückgesetzt werden. Bitte versuchen Sie es erneut.", + "stateConfirmNetworkError": "Netzwerkfehler – Status der Abschlagszahlung konnte nicht aktualisiert werden. Bitte versuchen Sie es erneut." }, "stateConfirm": { "paidDateLabel": "Bezahlt am", diff --git a/client/src/i18n/en/budget.json b/client/src/i18n/en/budget.json index 61ae756d..a138c30f 100644 --- a/client/src/i18n/en/budget.json +++ b/client/src/i18n/en/budget.json @@ -660,7 +660,9 @@ "loadError": "Failed to load deposits. Please try again.", "saveError": "Failed to save deposit. Please try again.", "deleteError": "Failed to delete deposit. Please try again.", - "revertError": "Failed to revert deposit status. Please try again." + "revertError": "Failed to revert deposit status. Please try again.", + "revertNetworkError": "Network error — could not revert deposit status. Please try again.", + "stateConfirmNetworkError": "Network error — could not update deposit status. Please try again." }, "stateConfirm": { "paidDateLabel": "Paid date", diff --git a/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.module.css b/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.module.css index 72ecc5c2..947d424d 100644 --- a/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.module.css +++ b/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.module.css @@ -82,6 +82,17 @@ background-color: var(--color-bg-secondary); } +.tableRowMutating { + opacity: 0.6; + transition: opacity var(--transition-fast); +} + +@media (prefers-reduced-motion: reduce) { + .tableRowMutating { + transition: none; + } +} + .table tbody td { padding: var(--spacing-3); vertical-align: middle; @@ -109,7 +120,7 @@ text-align: center; } -/* ---- Action Cell Menu ---- */ +/* ---- Action Cell ---- */ .actionCell { position: relative; @@ -117,81 +128,6 @@ justify-content: center; } -.menuButton { - background: none; - border: none; - padding: var(--spacing-2); - min-width: 44px; - min-height: 44px; - display: flex; - align-items: center; - justify-content: center; - cursor: pointer; - color: var(--color-text-secondary); - font-size: var(--font-size-lg); - border-radius: var(--radius-md); - transition: var(--transition-button); -} - -.menuButton:hover { - background-color: var(--color-bg-tertiary); - color: var(--color-text-primary); -} - -.menuButton:focus-visible { - outline: none; - box-shadow: var(--shadow-focus); -} - -.menuButton:disabled { - opacity: 0.5; - cursor: not-allowed; -} - -.menu { - position: absolute; - top: 100%; - right: 0; - background-color: var(--color-bg-primary); - border: 1px solid var(--color-border-strong); - border-radius: var(--radius-md); - box-shadow: var(--shadow-md); - z-index: var(--z-dropdown); - min-width: 160px; - overflow: hidden; -} - -.menuItem { - display: block; - width: 100%; - padding: var(--spacing-2-5) var(--spacing-3); - background: none; - border: none; - text-align: left; - cursor: pointer; - color: var(--color-text-primary); - font-size: var(--font-size-sm); - transition: var(--transition-button); -} - -.menuItem:hover { - background-color: var(--color-bg-secondary); -} - -.menuItem:focus-visible { - outline: none; - background-color: var(--color-bg-secondary); - box-shadow: inset 0 0 0 2px var(--color-primary); -} - -.menuItemDanger { - color: var(--color-danger-text-on-light); -} - -.menuItemDanger:hover { - background-color: var(--color-danger-bg); -} - /* ---- Mobile Card List ---- */ .mobileCardList { diff --git a/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.test.tsx b/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.test.tsx index d255bc21..7b33fce9 100644 --- a/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.test.tsx +++ b/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.test.tsx @@ -192,7 +192,6 @@ function renderSection( return render( { expect(chips).toHaveLength(0); }); }); + + // ─── Scenario 17–21: Revert error surfacing (#1413) ─────────────────────── + + describe('revert error surfacing (#1413)', () => { + // Helper: open the first overflow menu and return its menu items + function openMenuForFirstDeposit() { + const menuBtn = screen.getAllByRole('button').find((b) => b.textContent?.includes('⋮'))!; + fireEvent.click(menuBtn); + return screen.getAllByRole('menuitem'); + } + + // ─── Scenario 17: handleRevertToPending — API error ───────────────────── + + it('Scenario 17: handleRevertToPending — ApiClientError shows section-level alert', async () => { + const deposit = makeDeposit('dep-1', { status: 'paid', paidDate: '2026-03-10' }); + mockUpdateDeposit.mockRejectedValueOnce( + new MockApiClientError(400, { code: 'INVALID_DEPOSIT_STATUS_TRANSITION' }), + ); + + renderSection([deposit]); + + const menuItems = openMenuForFirstDeposit(); + // For a paid deposit, "Revert to pending" is a menu item + const revertBtn = menuItems.find((m) => + m.textContent?.toLowerCase().includes('pending'), + )!; + await act(async () => { + fireEvent.click(revertBtn); + }); + + // The section-level FormError (role="alert") should appear + await waitFor(() => { + const alerts = screen.getAllByRole('alert'); + expect(alerts.length).toBeGreaterThan(0); + // The error should contain the translated code from translateApiError mock + const alertText = alerts.map((a) => a.textContent ?? '').join(' '); + expect(alertText).toContain('translated:INVALID_DEPOSIT_STATUS_TRANSITION'); + }); + }); + + // ─── Scenario 18: handleRevertToPending — network error ────────────────── + + it('Scenario 18: handleRevertToPending — plain Error shows revertNetworkError banner', async () => { + const deposit = makeDeposit('dep-1', { status: 'paid', paidDate: '2026-03-10' }); + mockUpdateDeposit.mockRejectedValueOnce(new Error('network')); + + renderSection([deposit]); + + const menuItems = openMenuForFirstDeposit(); + const revertBtn = menuItems.find((m) => + m.textContent?.toLowerCase().includes('pending'), + )!; + await act(async () => { + fireEvent.click(revertBtn); + }); + + // revertNetworkError translation key is used; i18next returns the English string in tests + await waitFor(() => { + const alerts = screen.getAllByRole('alert'); + expect(alerts.length).toBeGreaterThan(0); + const alertText = alerts.map((a) => a.textContent ?? '').join(' '); + expect(alertText).toContain('Network error'); + }); + }); + + // ─── Scenario 19: handleRevertToPaid — API error ───────────────────────── + + it('Scenario 19: handleRevertToPaid — ApiClientError shows section-level alert', async () => { + const deposit = makeDeposit('dep-1', { + status: 'claimed', + paidDate: '2026-03-10', + claimedDate: '2026-03-20', + }); + mockUpdateDeposit.mockRejectedValueOnce( + new MockApiClientError(400, { code: 'INVALID_DEPOSIT_STATUS_TRANSITION' }), + ); + + renderSection([deposit]); + + const menuItems = openMenuForFirstDeposit(); + // For a claimed deposit, "Revert to paid" is the revert action + const revertBtn = menuItems.find( + (m) => + m.textContent?.toLowerCase().includes('revert') && + m.textContent?.toLowerCase().includes('paid'), + )!; + await act(async () => { + fireEvent.click(revertBtn); + }); + + await waitFor(() => { + const alerts = screen.getAllByRole('alert'); + expect(alerts.length).toBeGreaterThan(0); + const alertText = alerts.map((a) => a.textContent ?? '').join(' '); + expect(alertText).toContain('translated:INVALID_DEPOSIT_STATUS_TRANSITION'); + }); + }); + + // ─── Scenario 20: Banner auto-dismiss after 6000ms ──────────────────────── + + it('Scenario 20: revert error banner auto-dismisses after 6000ms', async () => { + jest.useFakeTimers(); + + const deposit = makeDeposit('dep-1', { status: 'paid', paidDate: '2026-03-10' }); + mockUpdateDeposit.mockRejectedValueOnce(new Error('network')); + + renderSection([deposit]); + + const menuItems = openMenuForFirstDeposit(); + const revertBtn = menuItems.find((m) => + m.textContent?.toLowerCase().includes('pending'), + )!; + + await act(async () => { + fireEvent.click(revertBtn); + }); + + // Banner should be present + await waitFor(() => { + expect(screen.getAllByRole('alert').length).toBeGreaterThan(0); + }); + + // Advance past the 6000ms auto-dismiss timer + await act(async () => { + jest.advanceTimersByTime(6001); + await Promise.resolve(); + }); + + await waitFor(() => { + // After dismissal, the revert error alert should be gone + // (The section-level FormError is only rendered when revertError !== '') + const remainingAlerts = screen.queryAllByRole('alert'); + expect(remainingAlerts.length).toBe(0); + }); + + jest.useRealTimers(); + }); + + // ─── Scenario 21: handleStateConfirm — modal error ─────────────────────── + + it('Scenario 21: handleStateConfirm — PATCH rejects with API error, FormError inside dialog', async () => { + const deposit = makeDeposit('dep-1', { status: 'pending' }); + // First call is for the state-confirm (mark-paid); it should reject + mockUpdateDeposit.mockRejectedValueOnce( + new MockApiClientError(400, { code: 'INVALID_DEPOSIT_STATUS_TRANSITION' }), + ); + + renderSection([deposit]); + + // Open menu and click "Mark paid" + const menuItems = openMenuForFirstDeposit(); + const markPaidBtn = menuItems.find((m) => + m.textContent?.toLowerCase().includes('paid'), + )!; + fireEvent.click(markPaidBtn); + + // The state confirm dialog should appear + await waitFor(() => screen.getByRole('dialog')); + + // Click the confirm button + const confirmBtn = screen.getByTestId('modal-footer').querySelector('button:last-child')!; + await act(async () => { + fireEvent.click(confirmBtn); + }); + + // FormError should appear INSIDE the dialog + await waitFor(() => { + const dialog = screen.getByRole('dialog'); + // role="alert" from FormError mock + const alertInsideDialog = dialog.querySelector('[role="alert"]'); + expect(alertInsideDialog).toBeInTheDocument(); + expect(alertInsideDialog!.textContent).toContain('translated:INVALID_DEPOSIT_STATUS_TRANSITION'); + }); + }); + }); }); diff --git a/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.tsx b/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.tsx index 9b6297bd..381aa164 100644 --- a/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.tsx +++ b/client/src/pages/InvoiceDetailPage/InvoiceDepositsSection.tsx @@ -6,6 +6,7 @@ import { ApiClientError } from '../../lib/apiClient.js'; import { useFormatters } from '../../lib/formatters.js'; import { translateApiError } from '../../lib/errorTranslation.js'; import { Badge, type BadgeVariantMap } from '../../components/Badge/Badge.js'; +import { OverflowMenu, type OverflowMenuItem } from '../../components/OverflowMenu/index.js'; import { Modal } from '../../components/Modal/Modal.js'; import { EmptyState } from '../../components/EmptyState/EmptyState.js'; import { FormError } from '../../components/FormError/FormError.js'; @@ -14,7 +15,6 @@ import styles from './InvoiceDepositsSection.module.css'; interface InvoiceDepositsSectionProps { invoiceId: string; - invoiceTotal: number; invoiceStatus: InvoiceStatus; deposits: InvoiceDeposit[]; finalPaymentAmount: number; @@ -38,18 +38,20 @@ interface StateConfirmState { action: StateConfirmAction; } -const emptyForm = (): DepositFormState => ({ - amount: '', - dueDate: '', - status: 'pending', - paidDate: new Date().toISOString().slice(0, 10), - claimedDate: new Date().toISOString().slice(0, 10), - description: '', -}); +const getEmptyForm = (): DepositFormState => { + const today = new Date().toISOString().slice(0, 10); + return { + amount: '', + dueDate: '', + status: 'pending', + paidDate: today, + claimedDate: today, + description: '', + }; +}; export function InvoiceDepositsSection({ invoiceId, - invoiceTotal, invoiceStatus, deposits, finalPaymentAmount, @@ -64,16 +66,34 @@ export function InvoiceDepositsSection({ const [selectedDeposit, setSelectedDeposit] = useState(null); const [isMutating, setIsMutating] = useState(false); const [mutatingDepositId, setMutatingDepositId] = useState(null); - const [menuOpenId, setMenuOpenId] = useState(null); const [stateConfirmDeposit, setStateConfirmDeposit] = useState(null); // Form state - const [depositForm, setDepositForm] = useState(emptyForm()); + const [depositForm, setDepositForm] = useState(() => getEmptyForm()); const [formError, setFormError] = useState(''); + // Revert error handling (transient banner) + const [revertError, setRevertError] = useState(''); + const revertErrorTimerRef = useRef | null>(null); + + // State confirm modal error + const [stateConfirmModalError, setStateConfirmModalError] = useState(''); + // Focus management const addButtonRef = useRef(null); + const showRevertError = (msg: string) => { + if (revertErrorTimerRef.current) clearTimeout(revertErrorTimerRef.current); + setRevertError(msg); + revertErrorTimerRef.current = setTimeout(() => setRevertError(''), 6000); + }; + + useEffect(() => { + return () => { + if (revertErrorTimerRef.current) clearTimeout(revertErrorTimerRef.current); + }; + }, []); + const invoiceStatusVariants: BadgeVariantMap = { pending: { label: t('invoiceDetail.statusLabels.pending')!, @@ -92,7 +112,7 @@ export function InvoiceDepositsSection({ const openAddModal = () => { setSelectedDeposit(null); - setDepositForm(emptyForm()); + setDepositForm(getEmptyForm()); setFormError(''); setModalMode('add'); }; @@ -109,28 +129,27 @@ export function InvoiceDepositsSection({ }); setFormError(''); setModalMode('edit'); - setMenuOpenId(null); }; const openDeleteModal = (deposit: InvoiceDeposit) => { setSelectedDeposit(deposit); setFormError(''); setModalMode('delete'); - setMenuOpenId(null); }; const openStateConfirm = (deposit: InvoiceDeposit, action: StateConfirmAction) => { setStateConfirmDeposit({ deposit, action }); - setMenuOpenId(null); + setStateConfirmModalError(''); }; const closeModal = () => { if (!isMutating) { setModalMode(null); setSelectedDeposit(null); - setDepositForm(emptyForm()); + setDepositForm(getEmptyForm()); setFormError(''); setStateConfirmDeposit(null); + setStateConfirmModalError(''); } }; @@ -254,7 +273,11 @@ export function InvoiceDepositsSection({ await updateDeposit(invoiceId, deposit.id, { status: 'pending' }); onDepositMutated(); } catch (err) { - // Silently fail and just clear the opacity + if (err instanceof ApiClientError) { + showRevertError(translateApiError(err.error.code, tErrors)); + } else { + showRevertError(t('budget:invoiceDetail.deposits.errors.revertNetworkError')); + } } finally { setMutatingDepositId(null); } @@ -266,7 +289,11 @@ export function InvoiceDepositsSection({ await updateDeposit(invoiceId, deposit.id, { status: 'paid' }); onDepositMutated(); } catch (err) { - // Silently fail and just clear the opacity + if (err instanceof ApiClientError) { + showRevertError(translateApiError(err.error.code, tErrors)); + } else { + showRevertError(t('budget:invoiceDetail.deposits.errors.revertNetworkError')); + } } finally { setMutatingDepositId(null); } @@ -294,7 +321,13 @@ export function InvoiceDepositsSection({ setStateConfirmDeposit(null); onDepositMutated(); } catch (err) { - // Could show error, but for now just fail silently + let msg: string; + if (err instanceof ApiClientError) { + msg = translateApiError(err.error.code, tErrors); + } else { + msg = t('budget:invoiceDetail.deposits.errors.stateConfirmNetworkError'); + } + setStateConfirmModalError(msg); } finally { setMutatingDepositId(null); } @@ -342,6 +375,8 @@ export function InvoiceDepositsSection({ {deposits.length > 0 && ( <> + {revertError && } + {/* Desktop/tablet table (hidden on mobile) */}
@@ -367,9 +402,7 @@ export function InvoiceDepositsSection({ openStateConfirm(deposit, 'mark-paid')} @@ -391,9 +424,7 @@ export function InvoiceDepositsSection({ openStateConfirm(deposit, 'mark-paid')} @@ -437,7 +468,6 @@ export function InvoiceDepositsSection({ error={formError} isMutating={isMutating} t={t} - formatCurrency={formatCurrency} /> )} @@ -459,8 +489,12 @@ export function InvoiceDepositsSection({ deposit={stateConfirmDeposit.deposit} action={stateConfirmDeposit.action} onConfirm={handleStateConfirm} - onClose={() => setStateConfirmDeposit(null)} + onClose={() => { + setStateConfirmDeposit(null); + setStateConfirmModalError(''); + }} isMutating={mutatingDepositId === stateConfirmDeposit.deposit.id} + error={stateConfirmModalError} t={t} /> )} @@ -474,9 +508,7 @@ export function InvoiceDepositsSection({ interface DepositRowProps { deposit: InvoiceDeposit; - menuOpenId: string | null; mutatingDepositId: string | null; - onMenuToggle: (id: string | null) => void; onEdit: (deposit: InvoiceDeposit) => void; onDelete: (deposit: InvoiceDeposit) => void; onMarkPaid: () => void; @@ -490,9 +522,7 @@ interface DepositRowProps { function DepositRow({ deposit, - menuOpenId, mutatingDepositId, - onMenuToggle, onEdit, onDelete, onMarkPaid, @@ -503,10 +533,6 @@ function DepositRow({ formatCurrency, formatDate, }: DepositRowProps) { - const isMenuOpen = menuOpenId === deposit.id; - const menuTriggerRef = useRef(null); - const menuRef = useRef(null); - const statusVariants: BadgeVariantMap = { pending: { label: t('invoiceDetail.statusLabels.pending')!, @@ -519,73 +545,66 @@ function DepositRow({ }, }; - const handleMenuKeyDown = (e: React.KeyboardEvent) => { - const menuItems = menuRef.current?.querySelectorAll('[role="menuitem"]'); - if (!menuItems || menuItems.length === 0) return; - - const currentIndex = Array.from(menuItems).findIndex((item) => item === document.activeElement); - - switch (e.key) { - case 'ArrowDown': { - e.preventDefault(); - const nextIndex = currentIndex === menuItems.length - 1 ? 0 : currentIndex + 1; - (menuItems[nextIndex] as HTMLButtonElement).focus(); - break; - } - case 'ArrowUp': { - e.preventDefault(); - const prevIndex = currentIndex === 0 ? menuItems.length - 1 : currentIndex - 1; - (menuItems[prevIndex] as HTMLButtonElement).focus(); - break; - } - case 'Home': { - e.preventDefault(); - (menuItems[0] as HTMLButtonElement).focus(); - break; - } - case 'End': { - e.preventDefault(); - (menuItems[menuItems.length - 1] as HTMLButtonElement).focus(); - break; - } - case 'Escape': { - e.preventDefault(); - onMenuToggle(null); - menuTriggerRef.current?.focus(); - break; - } - } - }; - - const handleTriggerKeyDown = (e: React.KeyboardEvent) => { - if (e.key === 'ArrowDown' && !isMenuOpen) { - e.preventDefault(); - onMenuToggle(deposit.id); - setTimeout(() => { - const firstMenuItem = menuRef.current?.querySelector( - '[role="menuitem"]', - ) as HTMLButtonElement; - firstMenuItem?.focus(); - }, 0); - } - }; - - useEffect(() => { - if (isMenuOpen) { - const firstMenuItem = menuRef.current?.querySelector( - '[role="menuitem"]', - ) as HTMLButtonElement; - firstMenuItem?.focus(); - } - }, [isMenuOpen]); + // Build menu items based on deposit status + const menuItems: OverflowMenuItem[] = []; + + if (deposit.status === 'pending') { + menuItems.push( + { + label: t('budget:invoiceDetail.deposits.menu.markPaid'), + onClick: onMarkPaid, + }, + { + label: t('budget:invoiceDetail.deposits.menu.edit'), + onClick: () => onEdit(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.delete'), + onClick: () => onDelete(deposit), + variant: 'destructive', + }, + ); + } else if (deposit.status === 'paid') { + menuItems.push( + { + label: t('budget:invoiceDetail.deposits.menu.markClaimed'), + onClick: onMarkClaimed, + }, + { + label: t('budget:invoiceDetail.deposits.menu.revertToPending'), + onClick: () => onRevertToPending(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.edit'), + onClick: () => onEdit(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.delete'), + onClick: () => onDelete(deposit), + variant: 'destructive', + }, + ); + } else if (deposit.status === 'claimed') { + menuItems.push( + { + label: t('budget:invoiceDetail.deposits.menu.revertToPaid'), + onClick: () => onRevertToPaid(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.edit'), + onClick: () => onEdit(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.delete'), + onClick: () => onDelete(deposit), + variant: 'destructive', + }, + ); + } return ( @@ -599,146 +618,13 @@ function DepositRow({ @@ -749,20 +635,10 @@ function DepositRow({ // Sub-component: DepositCard (mobile) // ============================================================================ -interface DepositCardProps extends Omit< - DepositRowProps, - 'menuOpenId' | 'mutatingDepositId' | 'onMenuToggle' -> { - menuOpenId: string | null; - mutatingDepositId: string | null; - onMenuToggle: (id: string | null) => void; -} +type DepositCardProps = DepositRowProps; function DepositCard({ deposit, - menuOpenId, - mutatingDepositId, - onMenuToggle, onEdit, onDelete, onMarkPaid, @@ -773,10 +649,6 @@ function DepositCard({ formatCurrency, formatDate, }: DepositCardProps) { - const isMenuOpen = menuOpenId === deposit.id; - const menuTriggerRef = useRef(null); - const menuRef = useRef(null); - const statusVariants: BadgeVariantMap = { pending: { label: t('invoiceDetail.statusLabels.pending')!, @@ -789,65 +661,62 @@ function DepositCard({ }, }; - const handleMenuKeyDown = (e: React.KeyboardEvent) => { - const menuItems = menuRef.current?.querySelectorAll('[role="menuitem"]'); - if (!menuItems || menuItems.length === 0) return; - - const currentIndex = Array.from(menuItems).findIndex((item) => item === document.activeElement); - - switch (e.key) { - case 'ArrowDown': { - e.preventDefault(); - const nextIndex = currentIndex === menuItems.length - 1 ? 0 : currentIndex + 1; - (menuItems[nextIndex] as HTMLButtonElement).focus(); - break; - } - case 'ArrowUp': { - e.preventDefault(); - const prevIndex = currentIndex === 0 ? menuItems.length - 1 : currentIndex - 1; - (menuItems[prevIndex] as HTMLButtonElement).focus(); - break; - } - case 'Home': { - e.preventDefault(); - (menuItems[0] as HTMLButtonElement).focus(); - break; - } - case 'End': { - e.preventDefault(); - (menuItems[menuItems.length - 1] as HTMLButtonElement).focus(); - break; - } - case 'Escape': { - e.preventDefault(); - onMenuToggle(null); - menuTriggerRef.current?.focus(); - break; - } - } - }; - - const handleTriggerKeyDown = (e: React.KeyboardEvent) => { - if (e.key === 'ArrowDown' && !isMenuOpen) { - e.preventDefault(); - onMenuToggle(deposit.id); - setTimeout(() => { - const firstMenuItem = menuRef.current?.querySelector( - '[role="menuitem"]', - ) as HTMLButtonElement; - firstMenuItem?.focus(); - }, 0); - } - }; - - useEffect(() => { - if (isMenuOpen) { - const firstMenuItem = menuRef.current?.querySelector( - '[role="menuitem"]', - ) as HTMLButtonElement; - firstMenuItem?.focus(); - } - }, [isMenuOpen]); + // Build menu items based on deposit status + const menuItems: OverflowMenuItem[] = []; + + if (deposit.status === 'pending') { + menuItems.push( + { + label: t('budget:invoiceDetail.deposits.menu.markPaid'), + onClick: onMarkPaid, + }, + { + label: t('budget:invoiceDetail.deposits.menu.edit'), + onClick: () => onEdit(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.delete'), + onClick: () => onDelete(deposit), + variant: 'destructive', + }, + ); + } else if (deposit.status === 'paid') { + menuItems.push( + { + label: t('budget:invoiceDetail.deposits.menu.markClaimed'), + onClick: onMarkClaimed, + }, + { + label: t('budget:invoiceDetail.deposits.menu.revertToPending'), + onClick: () => onRevertToPending(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.edit'), + onClick: () => onEdit(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.delete'), + onClick: () => onDelete(deposit), + variant: 'destructive', + }, + ); + } else if (deposit.status === 'claimed') { + menuItems.push( + { + label: t('budget:invoiceDetail.deposits.menu.revertToPaid'), + onClick: () => onRevertToPaid(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.edit'), + onClick: () => onEdit(deposit), + }, + { + label: t('budget:invoiceDetail.deposits.menu.delete'), + onClick: () => onDelete(deposit), + variant: 'destructive', + }, + ); + } return (
@@ -882,146 +751,13 @@ function DepositCard({
- - {isMenuOpen && ( -
- {deposit.status === 'pending' && ( - <> - - - - - )} - {deposit.status === 'paid' && ( - <> - - - - - - )} - {deposit.status === 'claimed' && ( - <> - - - - - )} -
- )} + placement="top-end" + />
); @@ -1040,7 +776,6 @@ interface AddEditDepositModalProps { error: string; isMutating: boolean; t: (key: string, opts?: Record) => string; - formatCurrency: (amount: number) => string; } function AddEditDepositModal({ @@ -1052,7 +787,6 @@ function AddEditDepositModal({ error, isMutating, t, - formatCurrency, }: AddEditDepositModalProps) { const isEdit = mode === 'edit'; @@ -1313,18 +1047,19 @@ interface StateConfirmModalProps { onConfirm: (date: string) => void; onClose: () => void; isMutating: boolean; + error?: string; t: (key: string, opts?: Record) => string; } function StateConfirmModal({ - deposit, action, onConfirm, onClose, isMutating, + error, t, }: StateConfirmModalProps) { - const [selectedDate, setSelectedDate] = useState(new Date().toISOString().slice(0, 10)); + const [selectedDate, setSelectedDate] = useState(() => new Date().toISOString().slice(0, 10)); const isMarkPaid = action === 'mark-paid'; const title = isMarkPaid @@ -1362,6 +1097,7 @@ function StateConfirmModal({ } > + {error && }
{formatDate(deposit.dueDate)} {formatCurrency(deposit.amount)}{deposit.description ?? '—'}
- - {isMenuOpen && ( -
- {deposit.status === 'pending' && ( - <> - - - - - )} - {deposit.status === 'paid' && ( - <> - - - - - - )} - {deposit.status === 'claimed' && ( - <> - - - - - )} -
- )} + placement="bottom-end" + />