Skip to content

feat(budget): rework per-source filter to use source rows as toggles with Cost/Payback/Net columns#1357

Merged
steilerDev merged 6 commits intobetafrom
feat/1356-source-row-filter-rework
Apr 26, 2026
Merged

feat(budget): rework per-source filter to use source rows as toggles with Cost/Payback/Net columns#1357
steilerDev merged 6 commits intobetafrom
feat/1356-source-row-filter-rework

Conversation

@steilerDev
Copy link
Copy Markdown
Owner

Summary

Reworks the per-source filter UX shipped in #1354/#1355 per user feedback.

  • Each source row in the expanded Available Funds is now itself the toggle (<tr role="button" aria-pressed>); the chip toolbar above the rows is removed.
  • All sources start selected; URL state stores deselections as ?deselectedSources=<id1>,<id2> (clean default URL).
  • Deselecting a source greys the row, hides its budget lines and any item/area with no surviving lines, drops the Available Funds total to the sum of selected sources, and shows a "(X of Y selected)" caption.
  • New per-source Cost · Payback · Net columns. Payback is computed client-side via entity-level pro-rata; Net = totalAmount + payback − cost.
  • Backend response shape unchanged.

Closes #1356

Test plan

  • Unit / integration tests pass (Quality Gates → Test shards)
  • E2E tests pass (16 shards × 3 viewports) — covers default state, deselection, cascade hide, available funds re-summation, URL round-trip, keyboard, mobile, dark mode
  • Pre-commit auto-fix bot does not report unfixable lint

Frank Steiler and others added 6 commits April 25, 2026 17:41
…with Cost/Payback/Net columns

- Source detail rows under Available Funds become the filter affordance: each <tr role="button" aria-pressed> toggles its source on click, Space, or Enter; Escape selects all
- All sources start selected by default; URL state stores deselections via ?deselectedSources=<id1>,<id2>
- Deselected rows are visually dimmed (text muted, dot opacity 0.4, no left-border accent); aria-pressed conveys state
- Items, areas, and any container with no surviving budget lines render null (cascade beyond just lines)
- Available Funds total recomputes to the sum of currently-selected sources; "(X of Y selected)" caption shown when filter is active
- New per-source columns: Cost (perspective-resolved sum), Payback (entity-level pro-rata, computed client-side), Net = totalAmount + payback - cost
- Remove BudgetSourceChip component, chip filter strip, and obsolete English/German i18n keys
- Live region moved outside table wrapper so announcements survive empty-state toggles

Closes #1356

This supersedes the chip-toolbar UX shipped in #1354/PR #1355 per user feedback.

Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude translator (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude ux-designer (Sonnet 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude product-architect (Opus 4.6) <noreply@anthropic.com>
Co-Authored-By: Claude product-owner (Opus 4.6) <noreply@anthropic.com>
- Rename i18n key overview.costBreakdown.availableFunds (object) to
  availableFundsFilter to avoid colliding with the same-named string
  ("Available funds"). JSON last-write-wins meant the label was being
  overwritten by the caption object, breaking 3 unit tests and the
  rendered Available Funds row label.
- Scope getByText('€200,000.00') to the Available Funds row via within()
  to disambiguate from the source detail row's Net column showing the
  same currency value.
- Relax the "1 of 2" caption regex to "<digit> of <digit>" — the fixture
  includes an unassigned line so the total is N+1 named sources.
- Replace the className-comparison dark-mode smoke check with an
  aria-pressed attribute assertion since deselected rows are styled via
  attribute selectors, not class toggles.

Fixes #1356

Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude translator (Sonnet 4.5) <noreply@anthropic.com>
- Remove the second fireEvent.click in the work-item cascade test —
  the 'No Area' container is also cascade-hidden, so its expand button
  is never rendered. Asserting 'Sourced Work Item' is absent after the
  WI section expands is sufficient.
- Update the "expand shows sub-rows with name and Net value" test to
  set projectedMin/projectedMax to 0 on the source summaries so the Net
  column equals totalAmount; previously the default 5000/8000 values
  produced a non-zero Cost making Net != totalAmount.

Fixes #1356

Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
… row padding

- Replace `textContent()` + `toMatch` with `toHaveText` regex assertion on
  the filter live region. The previous synchronous read could land before
  React re-rendered the announcement after the chip-row click.
- Bump mobile row vertical padding from spacing-3 (12px) to spacing-4
  (16px) so the source detail row's bounding box meets the 44px touch
  target on mobile viewports.

Fixes #1356

Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
…region

Both the "(X of Y selected)" caption and the role="status" live region
computed selected count without considering the Unassigned pseudo-source.
When the user deselects the Unassigned chip, the previous expression
(budgetSources.length - deselectedSourceIds.size) drifted off-by-one
because deselectedSourceIds may contain the literal 'unassigned' key
that isn't in budgetSources.

Both expressions now compute selected as
  named-selected + (hasUnassignedLines && !deselectedSourceIds.has('unassigned') ? 1 : 0)

matching the existing total formula
  budgetSources.length + (hasUnassignedLines ? 1 : 0).

Fixes #1356

Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com>
The off-by-one fix correctly counts the Unassigned pseudo-source as a
virtual source. The fixture (2 named + 1 unassigned = 3 virtual) now
shows "(2 of 3 selected)" after deselecting one source — the previous
"/1\D+\d+/" regex no longer matches. Use toHaveText with a generic
"<digit> of <digit>" regex so the assertion is locale-agnostic and
robust to count semantics.

Fixes #1356

Co-Authored-By: Claude e2e-test-engineer (Sonnet 4.5) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[product-architect] Approved (posted as comment — cannot self-approve).

Verified all five architect decisions from issue #1356 are honored:

  1. Per-source payback formula (computePerSourcePayback, CostBreakdownTable.tsx): entity-level pro-rata, weighted by max-perspective line cost (plannedAmount * (1 + margin) / actualCost / quotation * 1.05 per branch). Equal-distribution fallback (weight = 1/n) correctly triggers when totalCost === 0 and entityPayback > 0. Computed across the full breakdown via walkWi/walkHi regardless of selection (AC-11).
  2. URL param is ?deselectedSources=. Legacy ?sources= is explicitly ignored (verified by unit test ignores legacy ?sources= URL param and E2E assertions). Empty selection = clean URL.
  3. Subsidy Adjustments stays project-wide — payback computation runs over the full breakdown; no filter applied to adjustments section.
  4. Net formula: source rows use source.totalAmount + payback - allocatedCost; unassigned pseudo-row correctly drops totalAmount (no source budget). Sign coloring driven by net >= 0.
  5. Cascade hide applied at both item level (WorkItemRow/HouseholdItemRow early-return when no line in visibleLineIds) and area level (WorkItemAreaSection/HouseholdItemAreaSection via areaHasVisibleLines recursive walk).

No leakage: zero changes under server/, shared/, e2e/containers, or db/migrations. Backend response shape unchanged. BudgetSourceChip directory fully deleted (component, CSS module, test, index) with no remaining references.

Component reuse: source rows use native <tr> with role="button", tabIndex, aria-pressed, and Enter/Space/Escape keydown — no parallel toggle component introduced. Consistent with existing table-row interaction patterns.

Schema, API contract, and ADR documentation require no updates for this change.

Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[product-owner] Review against AC-1 to AC-25 from #1356.

Functional ACs verified — all PASS

  • AC-1 to AC-5 (filter affordance lives on rows): chip toolbar + BudgetSourceChip directory deleted, source rows render as <tr role="button" tabIndex={0} aria-pressed>, default state has all sources selected, deselected rows greyed via [aria-pressed="false"] styling with dot-opacity 0.4 and muted text. Verified.
  • AC-6 to AC-8 (Cost · Payback · Net columns): 4 columns rendered per source row. Cost = resolveProjected(source.projectedMin, source.projectedMax, perspective) formatted with leading minus. Payback resolved via computePerSourcePayback map. Net = source.totalAmount + payback − allocatedCost, sign-coloured. Unassigned pseudo-row drops totalAmount (correctly modelled as having no source budget).
  • AC-9 to AC-11 (per-source payback aggregation): computePerSourcePayback walks WI + HI areas, weights by max-perspective line cost (plannedAmount * (1 + margin) / actualCost / actualCost * 1.05), with the equal-distribution fallback 1/n when totalCost === 0 per architect decision (a). Computed across the full breakdown — independent of selection (AC-11 honoured).
  • AC-12 to AC-15 (cascade hide): visibleLineIds filters lines by deselectedSourceIds; item rows + area sections short-circuit when no surviving lines (areaHasVisibleLines). Sum/Remaining recompute from filtered set. Subsidy Adjustments stays project-wide per architect decision.
  • AC-16 to AC-18 (Available Funds re-summation): filteredAvailableFunds = sum(selected sources' totalAmount) when filtering, else overview.availableFunds. Empty selection → 0. Hero card untouched (out of scope).
  • AC-19 to AC-22 (URL state): ?deselectedSources=<ids> is the canonical param; default produces clean URL; legacy ?sources= is silently ignored (covered by ignores legacy ?sources= URL param unit test + E2E).
  • AC-23 to AC-25 (cleanup): BudgetSourceChip directory fully deleted (component + css + test + index); handleToolbarKeyDown replaced by per-row onKeyDown (Enter/Space toggle, Escape select-all); chip-toolbar tests removed and replaced with tr[role="button"] assertions.

Non-functional gap — MUST FIX before merge

statusAnnouncement live region (line 1638) and activeFilterCaption (line 1414) compute "selected" as budgetSources.length − deselectedSourceIds.size. When the user deselects the Unassigned pseudo-row, deselectedSourceIds contains the literal key 'unassigned' (not a member of budgetSources), so the selected count is decremented against the wrong denominator. Total uses budgetSources.length + (hasUnassignedLines ? 1 : 0) — the selected count needs the symmetrical adjustment. Concretely with 2 sources + unassigned, all 3 selected: caption hidden (correct). Deselect Unassigned: caption shows "(2 of 3 selected)" — wrong, should be "(2 of 3)" only if deselectedSourceIds doesn't contain 'unassigned'; it does, so current code outputs 2 − 1 = 1 → "(1 of 3 selected)". Off-by-one.

Fix: compute selected as (budgetSources.length + (hasUnassignedLines ? 1 : 0)) − deselectedSourceIds.size, or filter deselectedSourceIds against the actual key set before subtracting. Apply to both the visible caption and the SR live region for AC-4 / AC-17 alignment.

Posting as comment (non-blocking on functional ACs); please patch before merge.

Copy link
Copy Markdown
Owner Author

@steilerDev steilerDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[ux-designer] Design review for PR #1357 — per-source filter rework.

Verdict: Comment (non-blocking, fix before merge)

Passing checks

  • <tr role="button" tabIndex={0} aria-pressed> semantics: implemented correctly on both named source rows and the unassigned pseudo-row.
  • Selected state: border-left: 3px solid var(--chip-dot) on .colName — colored accent present via the scoped --chip-dot CSS var, matches spec.
  • Deselected state: color: var(--color-text-muted) on all td cells, opacity: 0.4 on .sourceDot, border-left: 3px solid transparent on .colName — all three non-color signals from the spec are present.
  • Hover: var(--color-bg-hover) on td cells.
  • Focus ring: box-shadow: var(--shadow-focus) on :focus-visible (not outline).
  • Transitions: var(--transition-normal) used throughout; prefers-reduced-motion guard removes transitions.
  • Chip toolbar removed: no role="toolbar", no BudgetSourceChip import/usage.
  • Available Funds total recomputes from filteredAvailableFunds.
  • "(X of Y selected)" caption: rendered in availableFundsFilterCaption with var(--font-size-xs) + var(--color-text-muted) — matches spec.
  • Mobile touch target: padding-top/bottom: var(--spacing-4) inside @media (max-width: 767px) gives ≥44px row height. Confirmed.
  • No new design tokens introduced.
  • Escape key on source row calls onSelectAllSources — correct.

Finding (Medium — fix before merge)

.srOnly class deleted from CSS module but still referenced in JSX.

CostBreakdownTable.module.css removes the .srOnly block entirely, but the role="status" live region div still uses className={styles.srOnly}. In CSS Modules, a missing class resolves silently to undefined, so the live region will render as a visible block in the DOM instead of being visually hidden. This means the screen-reader status announcement is displayed on screen — a functional accessibility regression.

Fix: move .srOnly to shared.module.css (it already belongs there as a utility class) and import it as sharedStyles.srOnly, or restore the definition in the module CSS.

Informational

The selected-state CSS on .rowSourceDetailToggle[aria-pressed="true"] .colName does not declare a transition for the border-left change (the transition is only declared on the deselected td block). This means toggling selected→deselected animates in, but deselected→selected does not animate out. Low impact but worth noting for refinement.

@steilerDev steilerDev merged commit e29b939 into beta Apr 26, 2026
31 checks passed
@steilerDev steilerDev deleted the feat/1356-source-row-filter-rework branch April 26, 2026 07:44
@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.4.0-beta.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.4.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant