feat(budget-sources): consolidate header actions and mute negative-remaining color (#1335, #1336)#1337
Conversation
…maining color (#1335, #1336) - Move Show lines / Edit / Delete into a single right-aligned action group in each source card's header row so they sit next to the label and badges (#1335) - Unify button padding so all three buttons share a consistent height; stretch edge-to-edge on mobile preserving 44px touch targets - Use the existing --color-danger-text-on-light token for the negative-remaining state so overspend is visible but not dominant (#1336) Fixes #1335 Fixes #1336 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.
[product-architect]
Approve (posted as comment — self-approval blocked). Scope appropriately narrow for two polish items bundled into a single PR.
Verified:
- Architecture compliance — Single-page UI change, no cross-cutting concerns. No new components, abstractions, shared types, schema, API, or i18n keys added. Correct scope for a CSS/layout polish PR.
- Token usage —
.expandTogglepadding (--spacing-2→--spacing-1-5) now matches.editButton/.deleteButtonpadding (var(--spacing-1-5) var(--spacing-3)at CSS lines 317/343), so all three buttons align at the same height.--color-danger-text-on-lightexists intokens.css(light:--color-red-700, dark:--color-red-300), mute applies cleanly in both themes. - Responsive rules — Both tablet (CSS 895-897) and mobile touch-target (CSS 930-932) blocks include
.expandTogglealongside Edit/Delete, so the unified action group stays consistent across viewports. - DOM structure — Button moved cleanly from
.sourceMainto.sourceActions; all handlers, a11y attributes (aria-expanded,aria-controls,aria-label), and translation keys preserved verbatim. - Test coverage — Three new tests are well-targeted: (1) action order guard, (2) discretionary source omits Delete, (3) structural regression guard that
.expandToggleis no longer inside.sourceMain. Appropriate coverage for a structural/layout change without over-testing.
No findings.
steilerDev
left a comment
There was a problem hiding this comment.
[ux-designer]
Reviewed against the specs posted on issues #1335 and #1336. (Self-approval blocked by GitHub — this comment carries the approve verdict.)
Token adherence — .expandToggle padding is now var(--spacing-1-5) var(--spacing-3), matching .editButton and .deleteButton (tokens.css lines 317, 343, 432). .summarySecondaryNegative uses var(--color-danger-text-on-light), resolving to --color-red-700 light / --color-red-300 dark (tokens.css lines 144, 598). No hardcoded values introduced.
Visual consistency — .expandToggle is now first child of .sourceActions with no new CSS class. The button JSX is a verbatim lift; all props (aria-expanded, aria-controls, aria-label, disabled, class composition) preserved exactly.
Action order — Show lines → Edit → Delete confirmed in TSX diff and covered by the new test.
Dark mode — both changed tokens switch automatically via [data-theme="dark"] in tokens.css. No bespoke override block needed or added.
Responsive — both media query blocks (flex: 1 stretch and min-height: 44px) now include .expandToggle. Touch target spec met.
Accessibility — aria-expanded, aria-controls, and dynamic aria-label (expand/collapse i18n variants) preserved verbatim on the moved button. No hover-only disclosure. No keyboard trap.
Test coverage — three new tests cover action order, discretionary-source variant (no Delete), and that .expandToggle is no longer inside .sourceMain.
No findings. Both specs fully implemented. Verdict: APPROVE.
steilerDev
left a comment
There was a problem hiding this comment.
[product-owner] APPROVED (posted as comment — self-approval blocked by GitHub).
#1335 — Header action group
| AC | Result |
|---|---|
Header: label+badges left, right-aligned [Show lines][Edit][Delete] |
PASS — sourceInfo (flex:1) holds name/badges; sourceActions (flex-shrink:0) holds all three buttons in the asserted order |
| Discretionary/system: hide Delete, keep Show lines + Edit | PASS — {!source.isDiscretionary && ...} guard + dedicated test asserts exactly 2 buttons |
| Edit mode: header action group hidden | PASS — sourceActions lives in the non-edit branch of the edit ternary; not rendered while editing |
| Responsive <600px: wrap/stretch, 44px min touch target | PASS — mobile media query sets justify-content: stretch + flex:1 on all three buttons; expandToggle now included in min-height:44px rules at both base and tablet breakpoints |
| Dark mode: no regressions | PASS — no hardcoded colors; existing tokens only |
| Click handlers preserved | PASS — handleToggleLinesWithClearing, startEdit, openDeleteConfirm unchanged |
#1336 — Muted negative-remaining
| AC | Result |
|---|---|
| Use muted danger token | PASS — .summarySecondaryNegative now uses --color-danger-text-on-light (red-700 light / red-300 dark) instead of --color-danger |
| Contrast ≥ 4.5:1 | PASS — red-700 on light surfaces and red-300 on dark surfaces are the established on-surface danger variants used elsewhere in the token system |
| No JS logic change to toggle | PASS — CSS-only change |
Scope & traceability
- Changes confined to the three named files (tsx/css/test); no backend, shared types, or i18n touched
- Both issues referenced via
Fixes #1335/Fixes #1336in commit body and PR description - Test authorship: 3 new tests carry the
qa-integration-testerco-author trailer
Summary
--color-dangerfor the muted--color-danger-text-on-lighttoken in the negative-remaining stateFixes #1335
Fixes #1336
Test plan
Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) noreply@anthropic.com