fix(budget-sources): make card layout a single full-width column (#1346)#1348
Conversation
- Restructure source card so the bar chart and summary rows span the full card width; header line contains [name + badges] on the left and [Show lines] [Edit] [Delete] right-aligned on the right - Remove the now-unused .sourceInfo wrapper; .sourceRowHeader moves inside the view-mode fragment only - Restyle .expandToggle at rest to match the Edit button so the three header buttons read as one cohesive action group - Keep the primary-based active indicator on the expanded state - Add three DOM-structure regression tests Fixes #1346 Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude frontend-developer (Haiku) <noreply@anthropic.com> Co-Authored-By: Claude qa-integration-tester <noreply@anthropic.com>
|
Thank you for your submission! We require all contributors to sign our Contributor License Agreement before we can accept your contribution. I have read the CLA Document and I hereby sign the CLA Frank Steiler seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
steilerDev
left a comment
There was a problem hiding this comment.
[ux-designer]
Design review — Budget Sources card layout restructure.
Checklist
Token adherence — All changed CSS values use design tokens correctly. The restated .expandToggle at rest uses var(--color-bg-tertiary), var(--color-border-strong), var(--font-size-sm), and var(--color-text-secondary), which are the exact same properties used by .editButton (lines 317–322). The expanded state .expandToggleActive retains its var(--color-primary-bg) / var(--color-primary-badge-text) / var(--color-primary) differentiation. No hardcoded values introduced.
Visual cohesion — The three header buttons (Edit, Delete, Show lines) now share a consistent resting style: bg-tertiary fill, border-strong outline, text-secondary label, font-size-sm. .expandToggleActive remains clearly distinct through the primary-bg fill and primary-coloured border. The chevron icon and reduced-motion guard on it are untouched.
Dark mode — All tokens used (--color-bg-tertiary, --color-border-strong, --color-text-secondary, --color-primary-bg, --color-primary-badge-text, --color-primary) have Layer 3 dark-mode overrides in tokens.css. No new tokens were introduced, so no new dark-mode rules are required.
Responsive — The .sourceRowHeader now carries both align-items: center (desktop) and flex-wrap: wrap. The mobile media query at < 640px overrides this to flex-direction: column; align-items: stretch, which stretches all three buttons to full width — correct. Interest rate, bar chart, and terms rows are direct children of .sourceRow (a flex-direction: column container) and span full width at all breakpoints, which is the intended fix. The tablet rule adds min-height: 44px touch targets to all three buttons.
Accessibility — align-items change from flex-start to center on the header row is safe; the badge cluster and the action group will vertically centre-align relative to each other, which reads naturally. No ARIA props were removed or changed. The expand toggle still uses aria-expanded (carried from the pre-existing implementation, not touched in this diff).
Hover state note — .expandToggle:hover now keeps background-color: var(--color-bg-tertiary) (no change on hover), which matches .editButton:hover exactly. This is intentional for visual parity but means there is no hover feedback on the expand toggle at rest. This is informational only — not a blocker, as .editButton has the same pattern and was not flagged previously.
Tests — Three structural regression tests added: absence of .sourceInfo, bar chart not inside header, sourceMain as direct child of header. These guard the layout contract adequately.
No findings. Approving.
steilerDev
left a comment
There was a problem hiding this comment.
[product-architect]
Review verdict: approve (posted as comment since GitHub blocks self-approval). Verified against the Architecture, API Contract, and Schema wiki pages — no cross-cutting concerns touched.
Architecture compliance
- Frontend-only change; no shared types, API, schema, ADR, or new design tokens introduced
.sourceRowalready usesdisplay: flex; flex-direction: column; gap: var(--spacing-4), so promoting the bar chart / interest-rate / terms to direct children yields the desired full-width single-column flow without any new layout rules — minimal, idiomatic restructure- Conditional rendering of
.sourceActions(hidden in edit mode,[Show lines] [Edit]only for discretionary) is preserved unchanged; ARIA attributes (aria-expanded, aria-controls, aria-label) untouched - Mobile media query at
.sourceRowHeaderstill adapts to the wrap; the newflex-wrap: wrapon the default header handles the ~600px graceful-wrap acceptance criterion
Code quality
- Large TSX line count is cascade-only: removing two wrapper divs (
sourceRowHeaderaround the whole conditional,sourceInfoinside the view fragment) — no refactoring beyond scope - Edit-mode form now renders directly under
.sourceRow..editFormisflex: 1; flex-direction: column; gap: var(--spacing-3), which composes correctly inside the column-flex.sourceRowparent .expandToggleat-rest now matches.editButtonexactly (bg-tertiary / border-strong / text-secondary / font-size-sm), satisfying the cohesive-action-group acceptance criterion; expanded/active state (primary-based) is untouched- All new CSS values reference existing semantic tokens (no raw colors, spacing, or sizes)
Test coverage
- 3 new DOM-structure regression tests lock in the invariants that motivated the fix:
.sourceInfoabsent,.sourceBarSectionnot inside.sourceRowHeader,.sourceMainis a direct child of.sourceRowHeader. These are the right shape of test for this kind of visual-structural regression
No findings.
Summary
.sourceRowHeadernow holds only[name + badges]and the action group; the bar chart, interest rate, and terms are direct children of.sourceRow, giving the bar chart and summary rows the full card width.expandToggle(Show lines) at-rest style now matches.editButtonso the three header buttons read as one cohesive group; the primary-based expanded/active state is unchangedFixes #1346
Test plan