Skip to content

fix(frontend): define the button system and make the default button quiet - #765

Merged
cristim merged 2 commits into
mainfrom
fix/ux-button-system
Oct 9, 2026
Merged

cristim merged 2 commits into
mainfrom
fix/ux-button-system

Conversation

@cristim

@cristim cristim commented Oct 9, 2026

Copy link
Copy Markdown
Member

What

Button system (CSS only, plus one string-match test). Independent of #763/#764 (branched from main e5ac75d).

  • Bare button and .btn were a solid grey fill with an identical hover. They are now a quiet outlined button; only primary, success and destructive actions are filled.
  • Defined the classes that markup used but CSS never did: .btn-danger (outlined red, fills on hover), .btn-warning, .btn-link (text action for table rows), button.secondary (alias of .btn-secondary).
  • Added a button:disabled style (there was none).
  • The generic hover uses :where(button):hover so component classes that set their own background are not overridden.
  • css.test.ts matched the literal button:hover; widened to allow the :where() form.

Not in this PR: collapsing the remaining variants into fewer classes, .btn-small unification, and the #725 table/CSS hygiene checklist (left open).

Refs #695 (VIS-01, VIS-02), #725 (not closed).

Verification

  • tsc clean, full jest passes (css suite 84/84), full Playwright e2e on a private port after a real build: 107/108; the one failure (ladder-config.spec.ts toast "Dismiss notification" strict-mode) passed 3/3 when rerun in isolation, a timing flake under parallel load unrelated to CSS.
  • Before = origin/main e5ac75d, after = this head, mock API, 7 pages x 1280/768/390: /tmp/claude/ux-fixes/shots/{home,opportunities,plans,purchases,inventory,admin-users,admin-general}-{main,pr2}-{1280,768,390}.png. Examples: purchases-*-1280.png (Load History, Approve/Cancel, Sell on Marketplace go from grey slabs to outlined/link actions), admin-users-*-1280.png (Delete is now distinct from Edit).
  • axe color-contrast node counts per page and width identical before and after (no text became unreadable); the contrast tokens are fix(frontend): core color tokens fail WCAG AA contrast #696.
  • Mock-based evidence, not a real backend.

🤖 Generated with Claude Code

…uiet

The bare button was a solid grey fill and .btn-danger, .btn-link,
.btn-warning and .secondary had no CSS. The default and .btn are now an
outlined button; only primary, success and destructive actions are filled.
.btn-danger and .btn-warning are outlined and fill on hover, .btn-link is a
text action for table rows, .secondary aliases .btn-secondary, and disabled
buttons are dimmed. The generic hover uses :where() so component classes
keep their own background.

Refs #695

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cristim cristim added priority/p2 Backlog-worthy triaged Item has been triaged severity/high Significant harm urgency/this-quarter Within the quarter impact/all-users Affects every user effort/m Days type/bug Defect labels Oct 9, 2026
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 45 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 87 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 50d560de-fc2d-4cb4-a94a-89885562793e

📥 Commits

Reviewing files that changed from the base of the PR and between 7e2a3d8 and e6e30f5.


📒 Files selected for processing (2)
  • frontend/src/__tests__/css.test.ts
  • frontend/src/styles/components.css


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Independent review (rv-ux-visual): PR #765 at 8094152e8ffbc3dd4fe354e0e2670d89892b3382

Verdict: approved (no blocking findings). CI 26/26 green at this SHA. This PR's base is e5ac75d; main is now 7e2a3d8. GitHub's merge preview is stale (computed against e5ac75d), so I reviewed what would actually land: this PR's diff applied with git apply onto main 7e2a3d8 (applies cleanly), compared with main 7e2a3d8 itself. Production webpack builds, mock API, Playwright Chromium at 1280/768/390, axe-core 4.11. Mock-based evidence.

Verified (main vs main + #765)

  • H1 / fix(frontend): undefined button classes and grey default button break the button system #695 fixed: the undefined classes now render.
    • Purchases row actions (Approve, Cancel, Sell on Marketplace, ↻ Retry, .btn-link) go from grey slabs to blue text links: transparent, #2563eb, #eff6ff on hover.
    • Admin Users and Groups Delete, registrations Reject (.btn-danger) and the Plans ✕ are now a red outline on #fce8e6 that fills red on hover, so they are clearly distinct from Edit.
  • H2 fixed: the bare and .btn buttons are a quiet white outline (#fff, 1px #d1d5db, #111827 text) with a real hover. Home "View Details", "Load History" and the column-filter triggers are no longer heavy grey blocks.
  • Hover rule audit: I compared the hover background and border of 110 visible buttons on 8 pages between the builds; 42 changed. All of them are intended or improvements. Notably, the top-bar Provider/Account chips and "Create Plan" used to hover to solid grey #6b7280, because the old generic button:hover leaked onto them; they now stay light. No component class lost its own hover.
  • No layout regressions: page overflow is 0 at every page and width, as on main. axe is identical to main on all 33 page/width combinations, with no console errors. Pixel diffs are button restyling only. Plans at 390 is 20px taller because the header now wraps; on main the "Purchase Plans" title ran into the "New Plan" button.

Findings (non-blocking follow-ups)

  1. Approve and Cancel are now equally weighted links in the approval queue and Purchase History (both .btn-link, same colour and hover). Before they were equally weighted grey slabs. The destructive "Cancel" still does not read as destructive. This is markup (history.ts), outside this CSS PR: e.g. Approve as .btn-small.primary and Cancel as a danger-coloured link.
  2. Home "Cancel" on upcoming purchases is still a solid red fill (button.danger), the loudest element in each row next to the now-quiet "View Details". The PR's own rule ("only primary and destructive actions are filled") makes this intentional, but in a list of rows an outlined danger (.btn-danger) would match Admin.
  3. Hover contrast until fix(frontend): meet WCAG AA contrast and show keyboard focus #769 lands: .btn-danger:hover and button.danger put white on --cudly-error #ea4335 (3.92:1). fix(frontend): meet WCAG AA contrast and show keyboard focus #769 changes the token to #d93025 (4.77:1), so this resolves with both merged.
  4. Merge conflict with fix(frontend): meet WCAG AA contrast and show keyboard focus #769 (verified: fix(frontend): meet WCAG AA contrast and show keyboard focus #769's diff does not apply on top of this one). Both PRs edit button.success:hover / button.danger:hover in components.css. This PR adds border-color: #2d9048 / #d33426 in the same rules where fix(frontend): meet WCAG AA contrast and show keyboard focus #769 changes the background to #166534 / var(--cudly-error-fg). Whichever merges second must carry fix(frontend): meet WCAG AA contrast and show keyboard focus #769's colours into these new border-color lines too, or the hover border keeps the old colour. The same applies to .btn-small.success:hover.
  5. Nit: at 390 "New Plan" wraps to two lines; white-space: nowrap on .btn/button would avoid that.

Out of scope, seen while testing: since #767, the Purchases tables' action columns (Approve / Cancel / Sell) sit off-screen to the right at 1280 inside the table scroller, so the row actions are hidden until the user scrolls (scroll affordance tracked in #185).

Evidence (local, review host): /tmp/claude/ux-pr/shots-765-769/ (<page>-{main,p765}-{1280,768,390}.png, *-diff-*-p765.png, btn-*-{main,p765}.png, buttons-{main,p765}.json, crop-home-*.png, crop-plans390-*.png). Scripts: /tmp/claude/ux-run/{build-merged.sh,review-shoot.js,pr765-buttons.js,hover-diff.js}.

@cristim

cristim commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

UX merge gate at head e6e30f5 (main 7e2a3d8 merged into the PR branch via update-branch). Verdict: independent rv-ux-visual APPROVED at 8094152; PR-owned diff (components.css, css.test.ts) is line-identical against the old base and against main. CI 27/27 green, mergeStateStatus CLEAN. Local from git archive (macOS): npm ci, tsc --noEmit, jest (100 suites, 3168 passed, 1 skipped), eslint all exit 0. Merging squash with --match-head-commit.

@cristim
cristim merged commit 6125d2c into main Oct 9, 2026
27 checks passed
cristim added a commit that referenced this pull request Oct 9, 2026
Resolve components.css hover conflicts: keep #765 border-color lines with
#769 AA colours (#166534, var(--cudly-error-fg)) as background and border.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@cristim

cristim commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

Post-merge check (rv-ux-visual)

Merged at head e6e30f5897899355b10fcbdf70798b87323618e8: its patch against main 7e2a3d8 is byte-identical to the one I approved at 8094152e (verified live as main+PR). CI 26/26 green at the merged head. Approval stands. The #769 conflict was resolved correctly when #769 merged (see #769).

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

Labels

effort/m Days impact/all-users Affects every user priority/p2 Backlog-worthy severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant