Skip to content

Commit 5290150

Browse files
authored
fix(ui): keep semantic button colours on hover after the no-op hover cleanup (#37580)
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.
1 parent 6811f1d commit 5290150

6 files changed

Lines changed: 49 additions & 6 deletions

File tree

ui/litellm-dashboard/src/app/chat/page.integration.test.tsx

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import React from "react";
2-
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
2+
import { fireEvent, render, screen, waitFor, within } from "@testing-library/react";
33
import { beforeEach, describe, expect, it, vi } from "vitest";
44
import { useChatHistory } from "@/components/chat/useChatHistory";
55
import ChatConversationPage from "./page";
66

7-
const { mockMakeOpenAIResponsesRequest } = vi.hoisted(() => ({
7+
const { mockMakeOpenAIResponsesRequest, shellState } = vi.hoisted(() => ({
88
mockMakeOpenAIResponsesRequest: vi.fn(),
9+
shellState: { storageUnavailable: false },
910
}));
1011

1112
vi.mock("next/navigation", () => ({
@@ -50,7 +51,7 @@ vi.mock("@/contexts/ChatShellContext", () => ({
5051
conversations: history.conversations,
5152
activeConversation: history.activeConversation,
5253
activeConversationId: history.currentActiveId,
53-
storageUnavailable: false,
54+
storageUnavailable: shellState.storageUnavailable,
5455
staleId: false,
5556
createConversation: history.createConversation,
5657
appendMessage: history.appendMessage,
@@ -81,6 +82,7 @@ describe("/ui/chat request metrics", () => {
8182
beforeEach(() => {
8283
localStorage.clear();
8384
mockMakeOpenAIResponsesRequest.mockReset();
85+
shellState.storageUnavailable = false;
8486
});
8587

8688
it("renders latency, TTFT, token counts and cost reported for the assistant turn", async () => {
@@ -130,3 +132,20 @@ describe("/ui/chat request metrics", () => {
130132
expect(document.querySelector(".response-metrics")).toBeNull();
131133
});
132134
});
135+
136+
describe("/ui/chat storage banner", () => {
137+
beforeEach(() => {
138+
localStorage.clear();
139+
mockMakeOpenAIResponsesRequest.mockReset();
140+
shellState.storageUnavailable = true;
141+
});
142+
143+
it("keeps the dismiss control amber on hover instead of the ghost variant's foreground", async () => {
144+
render(<ChatConversationPage />);
145+
146+
const banner = await screen.findByText("Chat history won't be saved in this browser session");
147+
const dismiss = within(banner.parentElement!).getByRole("button");
148+
expect(dismiss).toHaveClass("hover:text-warning/80");
149+
expect(dismiss).not.toHaveClass("hover:text-foreground");
150+
});
151+
});

ui/litellm-dashboard/src/app/chat/page.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ export default function ChatConversationPage() {
518518
variant="ghost"
519519
size="icon-xs"
520520
onClick={() => setStorageBannerDismissed(true)}
521-
className="text-warning hover:bg-warning/15 "
521+
className="text-warning hover:bg-warning/15 hover:text-warning/80"
522522
>
523523
<X className="size-3.5" />
524524
</Button>

ui/litellm-dashboard/src/components/SidebarUsageCard.test.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,4 +156,12 @@ describe("SidebarUsageCard", () => {
156156
await user.click(rail);
157157
expect(onExpandRail).toHaveBeenCalledOnce();
158158
});
159+
160+
it("keeps the collapsed rail tinted on hover instead of the outline variant's foreground", async () => {
161+
renderWithClient(<SidebarUsageCard accessToken="token" collapsed onExpandRail={vi.fn()} />);
162+
163+
const rail = await screen.findByTitle("Enterprise usage");
164+
expect(rail).toHaveClass("hover:text-sidebar-primary/80");
165+
expect(rail).not.toHaveClass("hover:text-foreground");
166+
});
159167
});

ui/litellm-dashboard/src/components/SidebarUsageCard.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export default function SidebarUsageCard({ accessToken, collapsed, onExpandRail
8585
variant="outline"
8686
onClick={onExpandRail}
8787
title="Enterprise usage"
88-
className="h-9 w-full rounded-lg border-sidebar-border bg-sidebar text-sidebar-primary shadow-none hover:bg-sidebar-accent"
88+
className="h-9 w-full rounded-lg border-sidebar-border bg-sidebar text-sidebar-primary shadow-none hover:bg-sidebar-accent hover:text-sidebar-primary/80"
8989
>
9090
<Award className="size-[18px]" strokeWidth={1.75} />
9191
</Button>

ui/litellm-dashboard/src/components/team/LoggingSettings.test.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,22 @@ describe("LoggingSettings", () => {
181181
expect(source.match(HARDCODED_PALETTE) ?? []).toHaveLength(0);
182182
});
183183

184+
it("keeps the remove button destructive on hover instead of the ghost variant's foreground", () => {
185+
const initialValue = [
186+
{
187+
callback_name: "langsmith",
188+
callback_type: "success",
189+
callback_vars: {},
190+
},
191+
];
192+
193+
renderWithProviders(<LoggingSettings value={initialValue} onChange={vi.fn()} />);
194+
195+
const remove = screen.getByRole("button", { name: "Remove" });
196+
expect(remove).toHaveClass("hover:text-destructive/80");
197+
expect(remove).not.toHaveClass("hover:text-foreground");
198+
});
199+
184200
it("reports the chosen event type when a different option is picked", async () => {
185201
const user = userEvent.setup({ delay: null });
186202
const mockOnChange = vi.fn();

ui/litellm-dashboard/src/components/team/LoggingSettings.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ const LoggingSettings: React.FC<LoggingSettingsProps> = ({
278278
variant="ghost"
279279
onClick={() => removeLoggingConfig(index)}
280280
size="sm"
281-
className="text-destructive hover:bg-destructive/10"
281+
className="text-destructive hover:bg-destructive/10 hover:text-destructive/80"
282282
type="button"
283283
>
284284
<Trash2 />

0 commit comments

Comments
 (0)