Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions site/src/components/Avatar/AvatarData.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ export const AvatarData: FC<AvatarDataProps> = ({
}

return (
<div className="flex items-center w-full gap-3">
<div className="flex items-center w-full min-w-0 gap-3">
{avatar}

<div className="flex flex-col w-full">
<div className="flex flex-col w-full min-w-0">
<span className="text-sm font-semibold text-content-primary">
{title}
</span>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import { useFormik } from "formik";
import {
ChevronDownIcon,
ChevronLeftIcon,
ChevronRightIcon,
InfoIcon,
PencilIcon,
} from "lucide-react";
import { ChevronLeftIcon, InfoIcon, PencilIcon } from "lucide-react";
import { type FC, useState } from "react";
import * as Yup from "yup";
import type * as TypesGen from "#/api/typesGenerated";
Expand Down Expand Up @@ -41,6 +35,13 @@ import {
} from "#/components/Tooltip/Tooltip";
import { cn } from "#/utils/cn";
import { getFormHelpers } from "#/utils/formUtils";
import {
CollapsibleSection,
CollapsibleSectionContent,
CollapsibleSectionDescription,
CollapsibleSectionHeader,
CollapsibleSectionTitle,
} from "../CollapsibleSection";
import type { ProviderState } from "./ChatModelAdminPanel";
import {
GeneralModelConfigFields,
Expand Down Expand Up @@ -116,9 +117,6 @@ export const ModelForm: FC<ModelFormProps> = ({
}) => {
const isEditing = Boolean(editingModel);
const isDefaultModel = isEditing && editingModel?.is_default === true;
const [showAdvanced, setShowAdvanced] = useState(false);
const [showPricing, setShowPricing] = useState(false);
const [showProviderConfig, setShowProviderConfig] = useState(false);
const [confirmingDelete, setConfirmingDelete] = useState(false);

const canManageModels = Boolean(
Expand Down Expand Up @@ -488,97 +486,62 @@ export const ModelForm: FC<ModelFormProps> = ({
</div>

{/* Usage Tracking */}
<div className="border-0 border-t border-solid border-border pt-4">
<button
type="button"
onClick={() => setShowPricing((v) => !v)}
className="flex w-full cursor-pointer items-start justify-between border-0 bg-transparent p-0 text-left transition-colors hover:text-content-primary"
>
<div>
<h3 className="m-0 text-sm font-medium text-content-primary">
Cost Tracking{" "}
</h3>
<p className="m-0 text-xs text-content-secondary">
Set per-token pricing so Coder can track costs and enforce
spending limits.
</p>
</div>
{showPricing ? (
<ChevronDownIcon className="mt-0.5 h-4 w-4 shrink-0 text-content-secondary" />
) : (
<ChevronRightIcon className="mt-0.5 h-4 w-4 shrink-0 text-content-secondary" />
)}
</button>
{showPricing && (
<div className="grid grid-cols-2 gap-3 pt-3 sm:grid-cols-4">
<CollapsibleSection variant="inline" defaultOpen={false}>
<CollapsibleSectionHeader>
<CollapsibleSectionTitle as="h3">
Cost Tracking
</CollapsibleSectionTitle>
<CollapsibleSectionDescription>
Set per-token pricing so Coder can track costs and enforce
spending limits.
</CollapsibleSectionDescription>
</CollapsibleSectionHeader>
<CollapsibleSectionContent>
<div className="grid grid-cols-2 gap-3 sm:grid-cols-4">
<PricingModelConfigFields
provider={selectedProviderState.provider}
form={form}
fieldErrors={modelConfigFormBuildResult.fieldErrors}
disabled={isSaving}
/>
</div>
)}
</div>

</CollapsibleSectionContent>
</CollapsibleSection>
{/* Provider Configuration */}
<div className="border-0 border-t border-solid border-border pt-4">
<button
type="button"
onClick={() => setShowProviderConfig((v) => !v)}
className="flex w-full cursor-pointer items-start justify-between border-0 bg-transparent p-0 text-left transition-colors hover:text-content-primary"
>
<CollapsibleSection variant="inline" defaultOpen={false}>
<CollapsibleSectionHeader>
<CollapsibleSectionTitle as="h3">
Provider Configuration
</CollapsibleSectionTitle>
<CollapsibleSectionDescription>
Tune provider-specific behavior like reasoning, tool calling,
and web search.
</CollapsibleSectionDescription>
</CollapsibleSectionHeader>
<CollapsibleSectionContent>
<div>
<h3 className="m-0 text-sm font-medium text-content-primary">
Provider Configuration
</h3>
<p className="m-0 text-xs text-content-secondary">
Tune provider-specific behavior like reasoning, tool calling,
and web search.
</p>
</div>
{showProviderConfig ? (
<ChevronDownIcon className="mt-0.5 h-4 w-4 shrink-0 text-content-secondary" />
) : (
<ChevronRightIcon className="mt-0.5 h-4 w-4 shrink-0 text-content-secondary" />
)}
</button>
{showProviderConfig && (
<div className="pt-3">
<ModelConfigFields
provider={selectedProviderState.provider}
form={form}
fieldErrors={modelConfigFormBuildResult.fieldErrors}
disabled={isSaving}
/>
</div>
)}
</div>

</CollapsibleSectionContent>
</CollapsibleSection>
{/* Advanced */}
<div className="border-0 border-t border-solid border-border pt-4">
<button
type="button"
onClick={() => setShowAdvanced((v) => !v)}
className="flex w-full cursor-pointer items-start justify-between border-0 bg-transparent p-0 text-left transition-colors hover:text-content-primary"
>
<div>
<h3 className="m-0 text-sm font-medium text-content-primary">
Advanced
</h3>
<p className="m-0 text-xs text-content-secondary">
Low-level parameters like temperature and penalties. Rarely
need changing.
</p>
</div>
{showAdvanced ? (
<ChevronDownIcon className="mt-0.5 h-4 w-4 shrink-0 text-content-secondary" />
) : (
<ChevronRightIcon className="mt-0.5 h-4 w-4 shrink-0 text-content-secondary" />
)}
</button>
{showAdvanced && (
<div className="grid grid-cols-2 gap-3 pt-3 sm:grid-cols-3">
<CollapsibleSection variant="inline" defaultOpen={false}>
<CollapsibleSectionHeader>
<CollapsibleSectionTitle as="h3">
Advanced
</CollapsibleSectionTitle>
<CollapsibleSectionDescription>
Low-level parameters like temperature and penalties. Rarely need
changing.
</CollapsibleSectionDescription>
</CollapsibleSectionHeader>
<CollapsibleSectionContent>
<div className="grid grid-cols-2 gap-3 sm:grid-cols-3">
<GeneralModelConfigFields
provider={selectedProviderState.provider}
form={form}
Expand Down Expand Up @@ -629,10 +592,11 @@ export const ModelForm: FC<ModelFormProps> = ({
)}
</div>
</div>
)}
</div>
</CollapsibleSectionContent>
</CollapsibleSection>
</div>
<div className="mt-auto py-6">
{" "}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 New stray {" "} text node before <hr>. Same class of bug as resolved #comment-3045753889. The base code at this position had no whitespace node. The four original instances were removed, but this one was introduced. (Bisky P3, Gon P3, Nami P3, Mafuuu P3, Meruem P3, Razor P3, Ging P3)

πŸ€–

<hr className="mb-4 border-0 border-t border-solid border-border" />
<div className="flex items-center justify-between">
{isEditing && editingModel && onDeleteModel ? (
Expand Down
193 changes: 193 additions & 0 deletions site/src/pages/AgentsPage/components/CollapsibleSection.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
import type { Meta, StoryObj } from "@storybook/react-vite";
import { expect, userEvent, within } from "storybook/test";
import { AdminBadge } from "./AdminBadge";
import {
CollapsibleSection,
CollapsibleSectionContent,
CollapsibleSectionDescription,
CollapsibleSectionHeader,
CollapsibleSectionTitle,
} from "./CollapsibleSection";

const meta: Meta<typeof CollapsibleSection> = {
title: "pages/AgentsPage/CollapsibleSection",
component: CollapsibleSection,
decorators: [
(Story) => (
<div style={{ maxWidth: 600 }}>
<Story />
</div>
),
],
};
export default meta;
type Story = StoryObj<typeof CollapsibleSection>;

const Placeholder = () => (
<p className="text-sm text-content-secondary">Placeholder content</p>
);

export const DefaultOpen: Story = {
render: () => (
<CollapsibleSection>
<CollapsibleSectionHeader>
<div className="flex items-center gap-2">
<CollapsibleSectionTitle>Default spend limit</CollapsibleSectionTitle>
<AdminBadge />
</div>
<CollapsibleSectionDescription>
The deployment-wide spending cap.
</CollapsibleSectionDescription>
</CollapsibleSectionHeader>
<CollapsibleSectionContent>
<Placeholder />
</CollapsibleSectionContent>
</CollapsibleSection>
),
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

expect(canvas.getByText("Placeholder content")).toBeInTheDocument();

const header = canvas.getByRole("button", {
name: /Default spend limit/i,
});
expect(header).toHaveAttribute("aria-expanded", "true");

await userEvent.click(header);
expect(header).toHaveAttribute("aria-expanded", "false");
expect(canvas.queryByText("Placeholder content")).not.toBeInTheDocument();

await userEvent.click(header);
expect(header).toHaveAttribute("aria-expanded", "true");
expect(canvas.getByText("Placeholder content")).toBeInTheDocument();
},
};

export const Collapsed: Story = {
render: () => (
<CollapsibleSection defaultOpen={false}>
<CollapsibleSectionHeader>
<div className="flex items-center gap-2">
<CollapsibleSectionTitle>Default spend limit</CollapsibleSectionTitle>
<AdminBadge />
</div>
<CollapsibleSectionDescription>
The deployment-wide spending cap.
</CollapsibleSectionDescription>
</CollapsibleSectionHeader>
<CollapsibleSectionContent>
<Placeholder />
</CollapsibleSectionContent>
</CollapsibleSection>
),
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

expect(canvas.queryByText("Placeholder content")).not.toBeInTheDocument();

const header = canvas.getByRole("button", {
name: /Default spend limit/i,
});
expect(header).toHaveAttribute("aria-expanded", "false");

await userEvent.click(header);
expect(header).toHaveAttribute("aria-expanded", "true");
expect(canvas.getByText("Placeholder content")).toBeInTheDocument();

await userEvent.click(header);
expect(header).toHaveAttribute("aria-expanded", "false");
expect(canvas.queryByText("Placeholder content")).not.toBeInTheDocument();
},
};

export const NoBadge: Story = {
render: () => (
<CollapsibleSection>
<CollapsibleSectionHeader>
<CollapsibleSectionTitle>Group limits</CollapsibleSectionTitle>
<CollapsibleSectionDescription>
Override defaults for groups.
</CollapsibleSectionDescription>
</CollapsibleSectionHeader>
<CollapsibleSectionContent>
<Placeholder />
</CollapsibleSectionContent>
</CollapsibleSection>
),
};

export const InlineVariant: Story = {
render: () => (
<CollapsibleSection variant="inline" defaultOpen={false}>
<CollapsibleSectionHeader>
<CollapsibleSectionTitle as="h3">Cost Tracking</CollapsibleSectionTitle>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No story tests controlled mode (open/onOpenChange), the API surface added to resolve round 1 finding #comment-3045753898.

Every story uses uncontrolled mode (either defaultOpen or the implicit default). The controlled code path at CollapsibleSection.tsx lines 109-119 is untested. You could delete those lines and all five stories pass green.

PRs 2-4 in the stack will consume controlled mode (force-open on validation error, "expand all" toggle). A consumer passing open without onOpenChange gets a dead trigger that silently ignores clicks, and there's no story documenting or testing that edge.

Fix: add a Controlled story that manages open state externally, toggles via a parent button, and asserts the section responds. Also test the open-without-onOpenChange frozen state.

PS. The as="h3" on this story is exercised but never asserted. expect(canvas.getByRole("heading", { level: 3 })).toBeInTheDocument() would catch a regression of the round 1 heading-level fix. (Bisky P2, Nami P3, Mafuuu P3, Meruem P3, Razor P3, Ging P3)

πŸ€–

<CollapsibleSectionDescription>
Set per-token pricing so Coder can track costs and enforce spending
limits.
</CollapsibleSectionDescription>
</CollapsibleSectionHeader>
<CollapsibleSectionContent>
<Placeholder />
</CollapsibleSectionContent>
</CollapsibleSection>
),
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

expect(canvas.queryByText("Placeholder content")).not.toBeInTheDocument();

const header = canvas.getByRole("button", {
name: /Cost Tracking/i,
});
expect(header).toHaveAttribute("aria-expanded", "false");

await userEvent.click(header);
expect(header).toHaveAttribute("aria-expanded", "true");
expect(canvas.getByText("Placeholder content")).toBeInTheDocument();

await userEvent.click(header);
expect(header).toHaveAttribute("aria-expanded", "false");
expect(canvas.queryByText("Placeholder content")).not.toBeInTheDocument();
},
};

export const KeyboardToggle: Story = {
render: () => (
<CollapsibleSection>
<CollapsibleSectionHeader>
<CollapsibleSectionTitle>Keyboard section</CollapsibleSectionTitle>
</CollapsibleSectionHeader>
<CollapsibleSectionContent>
<Placeholder />
</CollapsibleSectionContent>
</CollapsibleSection>
),
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

const header = canvas.getByRole("button", {
name: /Keyboard section/i,
});
expect(header).toHaveAttribute("aria-expanded", "true");
expect(canvas.getByText("Placeholder content")).toBeInTheDocument();

header.focus();

await userEvent.keyboard("{Enter}");
expect(header).toHaveAttribute("aria-expanded", "false");
expect(canvas.queryByText("Placeholder content")).not.toBeInTheDocument();

await userEvent.keyboard("{Enter}");
expect(header).toHaveAttribute("aria-expanded", "true");
expect(canvas.getByText("Placeholder content")).toBeInTheDocument();

await userEvent.keyboard(" ");
expect(header).toHaveAttribute("aria-expanded", "false");
expect(canvas.queryByText("Placeholder content")).not.toBeInTheDocument();

await userEvent.keyboard(" ");
expect(header).toHaveAttribute("aria-expanded", "true");
expect(canvas.getByText("Placeholder content")).toBeInTheDocument();
},
};
Loading
Loading