mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
refactor(ui): move the cache settings and playground model selector off tremor (#37323)
* refactor(ui): move the budget, cache, cost tracking and playground forms off tremor Swaps tremor Accordion for the Base UI Collapsible, TextInput for the shadcn Input and the two tremor Buttons for the shadcn Button across the budget modals, cache settings, the cost tracking add-provider and add-margin forms and the playground model selector. The accordion bodies keep tremor's unmount-when-closed semantics, since headless-ui's Disclosure.Panel and Base UI's panel both default to unmounting, so the antd fields inside behave exactly as before. The two cost tracking buttons are the one deliberate behaviour change. tremor's Button renders a bare button with no type, so inside the antd Form that wraps both components it was an implicit submit on top of its own onClick. For the discount form that meant every click ran handleAddProvider twice, once from onClick and once from the form's onFinish, and for the margin form the submit did nothing at all because that Form has no onFinish. The shadcn Button forces type="button", so the add now fires once from onClick alone and no type="submit" is added back. The three inputs that used onValueChange now read e.target.value, and each one gained a test that types into it and asserts the reported string, so the wiring cannot silently regress. The cache settings suite gained a collapse contract test that the advanced sections are absent until the section is expanded. Prunes the six no-restricted-imports suppressions these files no longer need, each dropping from two to one for the antd import that stays. * fix(ui): keep enter to submit on the cost tracking add forms The shadcn Button forces type="button", so converting the two tremor buttons left both cost tracking modals with no submit button at all. Each form still holds two fields that block implicit submission, the provider select's search input and the value input, so pressing Enter stopped adding anything. Both buttons get type="submit" back. For the discount modal that alone would restore the double add the conversion had just removed, since a submit also ran the form's onFinish, so the parent drops onFinish and the now dead handleFormSubmit. Click and Enter both go through onClick exactly once. The margin form's parent never had an onFinish, so restoring the submit type there is enough on its own. Adds three cases to the cost tracking settings suite: the discount add fires once from a click, the discount add fires once from Enter, and the margin add fires once from Enter. Dropping either type="submit" kills the Enter cases and putting onFinish back makes both discount cases see two calls. Also drops the two empty placeholders on the budget modals that only existed to suppress tremor's "Type..." default. * fix(ui): restore Enter-to-submit on the margin modal The tremor Button rendered a bare native button, which defaults to type="submit", so Enter in the percentage field submitted the margin modal. The shadcn Button wraps Base UI, which defaults to type="button", and the migration also replaced the margin modal's form element with a plain div, so Enter went inert while the visually identical discount modal kept working. Give the margin modal the same form wrapper the discount modal already has and mark its action button as the submit button. Also move the cache settings advanced-section test into the integration file, where a test that renders the real component tree belongs.
This commit is contained in:
parent
6b7adf011e
commit
b13fabe9c2
10 changed files with 73 additions and 22 deletions
|
|
@ -161,9 +161,6 @@
|
|||
"local/filename-pascal-case": {
|
||||
"count": 1
|
||||
},
|
||||
"no-restricted-imports": {
|
||||
"count": 2
|
||||
},
|
||||
"react-hooks/set-state-in-effect": {
|
||||
"count": 1
|
||||
}
|
||||
|
|
@ -959,7 +956,7 @@
|
|||
},
|
||||
"src/app/(dashboard)/playground/components/compareUI/components/ModelSelector.tsx": {
|
||||
"no-restricted-imports": {
|
||||
"count": 2
|
||||
"count": 1
|
||||
}
|
||||
},
|
||||
"src/app/(dashboard)/playground/components/complianceUI/ComplianceUI.tsx": {
|
||||
|
|
|
|||
|
|
@ -63,6 +63,19 @@ describe("CacheSettings advanced settings round-trip", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("reveals the advanced field sections only after the user expands them", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderSettings();
|
||||
await screen.findByText("Connection Settings");
|
||||
expect(screen.queryByText("SSL Settings")).not.toBeInTheDocument();
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Advanced Settings" }));
|
||||
|
||||
expect(await screen.findByText("SSL Settings")).toBeInTheDocument();
|
||||
expect(screen.getByText("Cache Management")).toBeInTheDocument();
|
||||
expect(screen.getByText("GCP Authentication")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("sends the same payload whether or not the advanced section was expanded", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderSettings();
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import React, { useState, useEffect, useCallback } from "react";
|
||||
import { ChevronRight } from "lucide-react";
|
||||
import { FormProvider, useForm } from "react-hook-form";
|
||||
import { Button } from "@tremor/react";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible";
|
||||
import { getCacheSettingsCall, testCacheConnectionCall, updateCacheSettingsCall } from "@/components/networking";
|
||||
import { fetchAvailableModels, ModelGroup } from "@/components/llm_calls/fetch_models";
|
||||
|
|
|
|||
|
|
@ -95,6 +95,33 @@ describe("AddMarginForm", () => {
|
|||
expect(onAddProvider).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("should report the edited percentage as the user types", async () => {
|
||||
const onPercentageChange = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(
|
||||
<AddMarginForm {...DEFAULT_PROPS} percentageValue="1" onPercentageChange={onPercentageChange} />,
|
||||
);
|
||||
|
||||
await user.type(screen.getByPlaceholderText("10"), "0");
|
||||
expect(onPercentageChange).toHaveBeenCalledWith("10");
|
||||
});
|
||||
|
||||
it("should report the edited fixed amount as the user types", async () => {
|
||||
const onFixedAmountChange = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(
|
||||
<AddMarginForm
|
||||
{...DEFAULT_PROPS}
|
||||
marginType="fixed"
|
||||
fixedAmountValue="0.00"
|
||||
onFixedAmountChange={onFixedAmountChange}
|
||||
/>,
|
||||
);
|
||||
|
||||
await user.type(screen.getByPlaceholderText("0.001"), "1");
|
||||
expect(onFixedAmountChange).toHaveBeenCalledWith("0.001");
|
||||
});
|
||||
|
||||
it("should call onMarginTypeChange when the Fixed Amount radio is clicked", async () => {
|
||||
const onMarginTypeChange = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
|
|
|
|||
|
|
@ -172,7 +172,7 @@ const AddMarginForm: React.FC<AddMarginFormProps> = ({
|
|||
|
||||
<div className="flex items-center justify-end space-x-3 pt-6 border-t border-border">
|
||||
<Button
|
||||
type="button"
|
||||
type="submit"
|
||||
onClick={onAddProvider}
|
||||
disabled={
|
||||
!selectedProvider ||
|
||||
|
|
|
|||
|
|
@ -62,6 +62,15 @@ describe("AddProviderForm", () => {
|
|||
expect(onAddProvider).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("should report the edited discount as the user types", async () => {
|
||||
const onDiscountChange = vi.fn();
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(<AddProviderForm {...DEFAULT_PROPS} newDiscount="1" onDiscountChange={onDiscountChange} />);
|
||||
|
||||
await user.type(screen.getByPlaceholderText("5"), "5");
|
||||
expect(onDiscountChange).toHaveBeenCalledWith("15");
|
||||
});
|
||||
|
||||
it("should show the percent sign next to the discount input", () => {
|
||||
renderWithProviders(<AddProviderForm {...DEFAULT_PROPS} />);
|
||||
expect(screen.getByText("%")).toBeInTheDocument();
|
||||
|
|
|
|||
|
|
@ -130,7 +130,7 @@ describe("CostTrackingSettings submit paths", () => {
|
|||
expect(stableDiscountCallbacks.handleAddProvider).toHaveBeenCalledWith("OpenAI", "5");
|
||||
});
|
||||
|
||||
it("leaves Enter inert in the margin field while the button still submits", async () => {
|
||||
it("requests the margin exactly once when Enter is pressed in the percentage field", async () => {
|
||||
const user = userEvent.setup();
|
||||
renderWithProviders(<CostTrackingSettings {...ADMIN_PROPS} />);
|
||||
const header = screen.getByText("Fee/Price Margin").closest("button");
|
||||
|
|
@ -142,14 +142,7 @@ describe("CostTrackingSettings submit paths", () => {
|
|||
await user.click((await screen.findAllByRole("option"))[0]);
|
||||
await user.type(screen.getByLabelText(/Margin Percentage/i), "10{Enter}");
|
||||
|
||||
expect(stableMarginCallbacks.handleAddMargin).not.toHaveBeenCalled();
|
||||
|
||||
const submit = screen
|
||||
.getAllByRole("button")
|
||||
.filter((button) => (button.textContent || "").trim() === "Add Provider Margin")
|
||||
.pop()!;
|
||||
await user.click(submit);
|
||||
|
||||
await waitFor(() => expect(stableMarginCallbacks.handleAddMargin).toHaveBeenCalledTimes(1));
|
||||
await waitFor(() => expect(stableMarginCallbacks.handleAddMargin).toHaveBeenCalled());
|
||||
expect(stableMarginCallbacks.handleAddMargin).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -382,7 +382,7 @@ const CostTrackingSettings: React.FC<CostTrackingSettingsProps> = ({ userID, use
|
|||
Select a provider (or "Global" for all providers) and configure the margin. You can use
|
||||
percentage-based or fixed amount.
|
||||
</p>
|
||||
<div className="space-y-6">
|
||||
<form onSubmit={(event) => event.preventDefault()} className="space-y-6">
|
||||
<AddMarginForm
|
||||
marginConfig={marginConfig}
|
||||
selectedProvider={selectedMarginProvider}
|
||||
|
|
@ -395,7 +395,7 @@ const CostTrackingSettings: React.FC<CostTrackingSettingsProps> = ({ userID, use
|
|||
onFixedAmountChange={setFixedAmountValue}
|
||||
onAddProvider={handleAddMargin}
|
||||
/>
|
||||
</div>
|
||||
</form>
|
||||
</div>
|
||||
</Modal>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { render, screen } from "@testing-library/react";
|
||||
import { fireEvent, render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { ModelSelector } from "./ModelSelector";
|
||||
|
|
@ -28,6 +28,18 @@ describe("ModelSelector", () => {
|
|||
expect(screen.getByTitle("custom-model-123")).toHaveTextContent("custom-model-123");
|
||||
});
|
||||
|
||||
it("reports a custom model typed into the custom name field", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
render(<ModelSelector value="" onChange={onChange} models={MODELS} />);
|
||||
|
||||
await user.click(screen.getByRole("combobox"));
|
||||
fireEvent.click(await screen.findByTitle("+ Add custom model"));
|
||||
await user.type(await screen.findByPlaceholderText("Custom Model Name (Enter to add)"), "my-custom-model{Enter}");
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith("my-custom-model");
|
||||
});
|
||||
|
||||
it("disables the control when disabled is set", () => {
|
||||
const { rerender } = render(<ModelSelector value="custom-model-123" onChange={vi.fn()} models={MODELS} />);
|
||||
expect(screen.getByRole("combobox")).toBeEnabled();
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import React, { useMemo, useState } from "react";
|
||||
import { Select } from "antd";
|
||||
import { TextInput } from "@tremor/react";
|
||||
import { Input } from "@/components/ui/input";
|
||||
interface ModelSelectorProps {
|
||||
value: string;
|
||||
onChange: (value: string) => void;
|
||||
|
|
@ -68,11 +68,11 @@ export function ModelSelector({ value, onChange, models, loading, disabled }: Mo
|
|||
<Select.Option value="__custom__">+ Add custom model</Select.Option>
|
||||
</Select>
|
||||
{isAddingCustom && (
|
||||
<TextInput
|
||||
<Input
|
||||
className="mt-2"
|
||||
placeholder="Custom Model Name (Enter to add)"
|
||||
value={customValue}
|
||||
onValueChange={setCustomValue}
|
||||
onChange={(e) => setCustomValue(e.target.value)}
|
||||
onKeyDown={(event) => {
|
||||
if (event.key === "Enter") {
|
||||
event.preventDefault();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue