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.
This commit is contained in:
yuneng-jiang 2026-08-19 23:13:08 -07:00 committed by GitHub
parent 6811f1d37f
commit 5290150a05
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 49 additions and 6 deletions

View file

@ -1,11 +1,12 @@
import React from "react";
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
import { fireEvent, render, screen, waitFor, within } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { useChatHistory } from "@/components/chat/useChatHistory";
import ChatConversationPage from "./page";
const { mockMakeOpenAIResponsesRequest } = vi.hoisted(() => ({
const { mockMakeOpenAIResponsesRequest, shellState } = vi.hoisted(() => ({
mockMakeOpenAIResponsesRequest: vi.fn(),
shellState: { storageUnavailable: false },
}));
vi.mock("next/navigation", () => ({
@ -50,7 +51,7 @@ vi.mock("@/contexts/ChatShellContext", () => ({
conversations: history.conversations,
activeConversation: history.activeConversation,
activeConversationId: history.currentActiveId,
storageUnavailable: false,
storageUnavailable: shellState.storageUnavailable,
staleId: false,
createConversation: history.createConversation,
appendMessage: history.appendMessage,
@ -81,6 +82,7 @@ describe("/ui/chat request metrics", () => {
beforeEach(() => {
localStorage.clear();
mockMakeOpenAIResponsesRequest.mockReset();
shellState.storageUnavailable = false;
});
it("renders latency, TTFT, token counts and cost reported for the assistant turn", async () => {
@ -130,3 +132,20 @@ describe("/ui/chat request metrics", () => {
expect(document.querySelector(".response-metrics")).toBeNull();
});
});
describe("/ui/chat storage banner", () => {
beforeEach(() => {
localStorage.clear();
mockMakeOpenAIResponsesRequest.mockReset();
shellState.storageUnavailable = true;
});
it("keeps the dismiss control amber on hover instead of the ghost variant's foreground", async () => {
render(<ChatConversationPage />);
const banner = await screen.findByText("Chat history won't be saved in this browser session");
const dismiss = within(banner.parentElement!).getByRole("button");
expect(dismiss).toHaveClass("hover:text-warning/80");
expect(dismiss).not.toHaveClass("hover:text-foreground");
});
});

View file

@ -518,7 +518,7 @@ export default function ChatConversationPage() {
variant="ghost"
size="icon-xs"
onClick={() => setStorageBannerDismissed(true)}
className="text-warning hover:bg-warning/15 "
className="text-warning hover:bg-warning/15 hover:text-warning/80"
>
<X className="size-3.5" />
</Button>

View file

@ -156,4 +156,12 @@ describe("SidebarUsageCard", () => {
await user.click(rail);
expect(onExpandRail).toHaveBeenCalledOnce();
});
it("keeps the collapsed rail tinted on hover instead of the outline variant's foreground", async () => {
renderWithClient(<SidebarUsageCard accessToken="token" collapsed onExpandRail={vi.fn()} />);
const rail = await screen.findByTitle("Enterprise usage");
expect(rail).toHaveClass("hover:text-sidebar-primary/80");
expect(rail).not.toHaveClass("hover:text-foreground");
});
});

View file

@ -85,7 +85,7 @@ export default function SidebarUsageCard({ accessToken, collapsed, onExpandRail
variant="outline"
onClick={onExpandRail}
title="Enterprise usage"
className="h-9 w-full rounded-lg border-sidebar-border bg-sidebar text-sidebar-primary shadow-none hover:bg-sidebar-accent"
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"
>
<Award className="size-[18px]" strokeWidth={1.75} />
</Button>

View file

@ -181,6 +181,22 @@ describe("LoggingSettings", () => {
expect(source.match(HARDCODED_PALETTE) ?? []).toHaveLength(0);
});
it("keeps the remove button destructive on hover instead of the ghost variant's foreground", () => {
const initialValue = [
{
callback_name: "langsmith",
callback_type: "success",
callback_vars: {},
},
];
renderWithProviders(<LoggingSettings value={initialValue} onChange={vi.fn()} />);
const remove = screen.getByRole("button", { name: "Remove" });
expect(remove).toHaveClass("hover:text-destructive/80");
expect(remove).not.toHaveClass("hover:text-foreground");
});
it("reports the chosen event type when a different option is picked", async () => {
const user = userEvent.setup({ delay: null });
const mockOnChange = vi.fn();

View file

@ -278,7 +278,7 @@ const LoggingSettings: React.FC<LoggingSettingsProps> = ({
variant="ghost"
onClick={() => removeLoggingConfig(index)}
size="sm"
className="text-destructive hover:bg-destructive/10"
className="text-destructive hover:bg-destructive/10 hover:text-destructive/80"
type="button"
>
<Trash2 />