fix(ui): lay the savings header out with the card's own slots

The header was hand-rolled rows, so the title, the subtitle, the legend and the
tab control all competed for one line. The subtitle is longer on Cumulative than
on Per day, so it wrapped on one tab and not the other and the chart moved with
it; pinning the controls against shrinking then pushed them past the card edge
once the viewport narrowed.

CardHeader already solves this. It is a grid that switches to
`grid-cols-[1fr_auto]` when a card-action slot is present, sizing the controls to
their content and giving the rest to the title, with the description on its own
row. Using CardTitle, CardDescription and CardAction removes the bespoke layout
rather than tuning it, and the controls wrap inside their own column instead of
overflowing.

The guard test now anchors on those slots. Its previous selector matched a
summary card's hint rather than this subtitle, so it passed with the subtitle
moved back into the controls.
This commit is contained in:
Tin Chi Lo 2026-07-31 23:07:39 -07:00
parent 669fd84aee
commit f66687b96d
2 changed files with 40 additions and 44 deletions

View file

@ -249,41 +249,38 @@ describe("UsageTab", () => {
expect(readSeries(bars)[0]).toMatchObject({ "Auto-router": -0.05 });
});
it("keeps the header the same shape on both tabs so the chart cannot shift", async () => {
it("lays the savings header out with the card's own slots so nothing shifts between tabs", async () => {
// The subtitle differs in length between the tabs ("Running total saved" vs "Saved
// per day"). Sharing a row with the title and controls made the header grow a line
// when it wrapped, moving the legend, the toggle and the chart below it.
// per day"). Hand-rolled rows made it compete with the legend and the toggle for
// width, so the header grew a line on one tab and the chart moved with it. CardHeader
// sizes the action column to its content and gives the rest to the title column.
const { getByRole, getByTestId, container } = renderWith(twoDays());
const layout = () => {
const header = () => {
const legend = getByTestId("chart-legend");
const controls = legend.parentElement as HTMLElement;
// the toggle travels with the legend, so neither can move independently
expect(controls.contains(getByRole("tablist"))).toBe(true);
const titleRow = controls.parentElement as HTMLElement;
// by text, not by class: several cards render a muted <p>, and grabbing the first
// one silently asserts against a summary-card hint instead of this subtitle
const subtitle = Array.from(container.querySelectorAll("p")).find((el) =>
/Running total saved|Saved per day/.test(el.textContent ?? ""),
);
expect(subtitle, "savings subtitle should be rendered").toBeTruthy();
return { controls, titleRow, subtitle: subtitle as HTMLElement };
const action = legend.closest('[data-slot="card-action"]') as HTMLElement;
const cardHeader = action.parentElement as HTMLElement;
const description = cardHeader.querySelector('[data-slot="card-description"]') as HTMLElement;
return { action, cardHeader, description };
};
const before = layout();
// the subtitle is a sibling BELOW the title row, never inside it, so its length
// cannot change that row's height
expect(before.titleRow.contains(before.subtitle)).toBe(false);
expect(before.titleRow.className).not.toContain("flex-wrap");
expect(before.controls.className).toContain("shrink-0");
const before = header();
expect(before.action).toBeTruthy();
expect(before.description).toBeTruthy();
// the toggle rides in the same action slot as the legend, so neither moves alone
expect(before.action.contains(getByRole("tablist"))).toBe(true);
// the subtitle lives outside that slot, so its length cannot reposition the controls
expect(before.action.contains(before.description)).toBe(false);
expect(before.description.textContent).toContain("Running total saved");
await userEvent.click(getByRole("tab", { name: "Per day" }));
const after = layout();
expect(after.controls).toBe(before.controls);
expect(after.titleRow).toBe(before.titleRow);
expect(after.titleRow.contains(after.subtitle)).toBe(false);
expect(container.textContent).toContain("Saved per day");
const after = header();
expect(after.action).toBe(before.action);
expect(after.cardHeader).toBe(before.cardHeader);
expect(after.action.contains(after.description)).toBe(false);
expect(after.description.textContent).toContain("Saved per day");
expect(container.textContent).toContain("Savings");
});
it("subtracts a losing auto-router route from the total and keeps it out of the donut", () => {

View file

@ -5,7 +5,7 @@ import { Info } from "lucide-react";
import { AreaChart, BarChart, CustomLegend, DonutChart, SEQUENTIAL_COLOR_RAMP } from "@/components/shared/charts";
import AdvancedDatePicker from "@/components/shared/advanced_date_picker";
import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
import { Card, CardAction, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card";
import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover";
import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs";
import { getToolSpend, ToolSpendResponse } from "@/components/networking";
@ -210,23 +210,22 @@ const UsageTab: React.FC<UsageTabProps> = ({ accessToken, activity }) => {
<div className="grid grid-cols-1 gap-6 lg:grid-cols-3">
<Card className="lg:col-span-2">
{/* Title and controls share a fixed row; the subtitle gets its own line below.
Competing for one row made the header taller whenever the subtitle wrapped,
which differs between the two tabs, so the chart shifted down on one of them */}
<CardHeader className="space-y-1.5">
<div className="flex flex-row items-center justify-between gap-4">
<CardTitle>Savings</CardTitle>
<div className="flex shrink-0 items-center gap-4">
<CustomLegend categories={SAVINGS_SERIES} colors={SAVINGS_COLORS} />
<Tabs value={accumulation} onValueChange={(value) => setAccumulation(value as SavingsAccumulation)}>
<TabsList>
<TabsTrigger value="cumulative">Cumulative</TabsTrigger>
<TabsTrigger value="per-interval">{intervalLabel}</TabsTrigger>
</TabsList>
</Tabs>
</div>
</div>
<p className="text-sm text-muted-foreground">{savingsSubtitle}</p>
{/* CardHeader's own slots rather than hand-rolled rows: the action column is
sized to its content and the title column takes the rest, so the subtitle
never competes with the controls for width and neither moves when it grows.
The controls wrap within their column instead of pushing past the card */}
<CardHeader>
<CardTitle>Savings</CardTitle>
<CardDescription>{savingsSubtitle}</CardDescription>
<CardAction className="flex flex-wrap items-center justify-end gap-x-4 gap-y-2">
<CustomLegend categories={SAVINGS_SERIES} colors={SAVINGS_COLORS} />
<Tabs value={accumulation} onValueChange={(value) => setAccumulation(value as SavingsAccumulation)}>
<TabsList>
<TabsTrigger value="cumulative">Cumulative</TabsTrigger>
<TabsTrigger value="per-interval">{intervalLabel}</TabsTrigger>
</TabsList>
</Tabs>
</CardAction>
</CardHeader>
<CardContent>
{accumulation === "cumulative" ? (