fix(ui): keep semantic button colours on hover after the no-op hover cleanup - #37580
Conversation
…cleanup PR #37579 read `text-X hover:text-X` on a shadcn Button as dead weight and removed the hover half. On the ghost and outline variants it was not dead: both carry their own `hover:text-foreground`, and the duplicate in the className was the thing displacing it through tailwind-merge. Dropping it handed the hover back to the variant, so the Remove button in a team's logging settings, the chat storage banner's dismiss control, and the collapsed enterprise-usage rail all lose their colour the moment you point at them. Each of the three now carries a distinct hover value, following the alpha-step idiom the rest of that migration used, which restores the colour and keeps `local/no-noop-hover-variant` satisfied. Every other hover utility that PR dropped sits on a plain element or a variant with no competing `hover:text-`, so those stay as they are.
Greptile SummaryThe PR restores semantic hover colors for three dashboard controls and adds focused regression coverage.
Confidence Score: 5/5The PR appears safe to merge; the semantic hover utilities are valid and the focused tests exercise the same class-merging path used in production. The three caller-supplied hover classes correctly replace the ghost or outline variant’s neutral hover foreground, while preserving valid theme-token colors in light and dark themes.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/app/chat/page.tsx | Adds a valid warning hover text utility that overrides the ghost variant’s neutral foreground. |
| ui/litellm-dashboard/src/app/chat/page.integration.test.tsx | Adds regression coverage using the existing chat shell mock and verifies the final merged dismiss-button classes. |
| ui/litellm-dashboard/src/components/SidebarUsageCard.tsx | Preserves the sidebar-primary tint when the collapsed outline button is hovered. |
| ui/litellm-dashboard/src/components/SidebarUsageCard.test.tsx | Verifies that the collapsed rail retains its semantic hover class and drops the outline foreground override. |
| ui/litellm-dashboard/src/components/team/LoggingSettings.tsx | Preserves destructive styling on hover for logging integration removal buttons. |
| ui/litellm-dashboard/src/components/team/LoggingSettings.test.tsx | Adds focused coverage for the remove button’s final merged hover classes. |
Reviews (1): Last reviewed commit: "fix(ui): keep semantic button colours on..." | Re-trigger Greptile
tin-berri
left a comment
There was a problem hiding this comment.
done — #37580 approved. Tiny, well-scoped follow-up to #37579: three buttons (team logging "Remove", collapsed enterprise-usage rail, chat storage-banner dismiss) had a semantic color class overridden on hover by the shadcn ghost/outline variant's default hover:text-foreground, per the PR's caveat that tailwind-merge resolves class collisions by order regardless of specificity — so simply reordering wasn't the fix, each button needed its own explicit hover:<color>/80. Each of the three fixes has a matching regression test that asserts the pinned hover class is present and the variant's foreground class is absent, which is exactly the right assertion to catch a future re-override. No auth/billing/security surface, CI green.
TLDR
Problem this solves:
How it solves it:
User Flow
Before: an admin pointing at a red, amber, or tinted button watches it turn plain grey, so the colour that told them what the button does is gone exactly when they are about to click it
After: the same three buttons keep their colour and just deepen slightly, so the meaning survives the hover
Relevant issues
Follow-up to #37579
Linear ticket
Pre-Submission checklist
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Setup: run the dashboard dev server against a local proxy so the screens below render live
Dark mode has no in-app toggle yet, so switch it on from the browser console where a case calls for it:
Before (93c1461)
Remove button in a team's logging settings
Collapsed enterprise usage rail
Chat storage banner dismiss
After (3ca5c70e2eb0b3b7a37e1b7cf3fbb6ef6da1efb4)
Remove button in a team's logging settings
Collapsed enterprise usage rail
Chat storage banner dismiss
Type
🐛 Bug Fix
Caveats (if any)
Final Attestation