feat(KB-183): replace native select with CustomModelDropdown in SettingsModal
- Replace native <select> with CustomModelDropdown for model selection - Remove legacy option filtering and selection logic from SettingsModal - Update SettingsModal tests to work with new dropdown component - Add comprehensive test coverage for CustomModelDropdown interactions
This commit is contained in:
@@ -5,7 +5,7 @@ import { fetchSettings, updateSettings, fetchAuthStatus, loginProvider, logoutPr
|
||||
import type { AuthProvider, ModelInfo } from "../api";
|
||||
import type { ToastType } from "../hooks/useToast";
|
||||
import { ThemeSelector } from "./ThemeSelector";
|
||||
import { filterModels } from "../utils/modelFilter";
|
||||
import { CustomModelDropdown } from "./CustomModelDropdown";
|
||||
|
||||
/**
|
||||
* Settings sections configuration.
|
||||
@@ -78,7 +78,6 @@ export function SettingsModal({
|
||||
// Model state
|
||||
const [availableModels, setAvailableModels] = useState<ModelInfo[]>([]);
|
||||
const [modelsLoading, setModelsLoading] = useState(false);
|
||||
const [modelFilter, setModelFilter] = useState("");
|
||||
|
||||
useEffect(() => {
|
||||
fetchSettings()
|
||||
@@ -243,12 +242,6 @@ export function SettingsModal({
|
||||
</>
|
||||
);
|
||||
case "model": {
|
||||
// Filter and group models by provider
|
||||
const filteredModels = filterModels(availableModels, modelFilter);
|
||||
const modelsByProvider = filteredModels.reduce<Record<string, ModelInfo[]>>((acc, m) => {
|
||||
(acc[m.provider] ??= []).push(m);
|
||||
return acc;
|
||||
}, {});
|
||||
const selectedValue = form.defaultProvider && form.defaultModelId
|
||||
? `${form.defaultProvider}/${form.defaultModelId}`
|
||||
: "";
|
||||
@@ -264,34 +257,12 @@ export function SettingsModal({
|
||||
) : (
|
||||
<div className="form-group">
|
||||
<label htmlFor="defaultModel">Default Model</label>
|
||||
{/* Filter input */}
|
||||
<div className="model-selector-filter">
|
||||
<input
|
||||
type="text"
|
||||
className="model-selector-filter-input"
|
||||
placeholder="Filter models…"
|
||||
value={modelFilter}
|
||||
onChange={(e) => setModelFilter(e.target.value)}
|
||||
/>
|
||||
{modelFilter && (
|
||||
<button
|
||||
type="button"
|
||||
className="model-selector-filter-clear"
|
||||
onClick={() => setModelFilter("")}
|
||||
aria-label="Clear filter"
|
||||
>
|
||||
×
|
||||
</button>
|
||||
)}
|
||||
<span className="model-selector-results-count">
|
||||
{filteredModels.length} model{filteredModels.length !== 1 ? "s" : ""}
|
||||
</span>
|
||||
</div>
|
||||
<select
|
||||
<CustomModelDropdown
|
||||
id="defaultModel"
|
||||
label="Default Model"
|
||||
models={availableModels}
|
||||
value={selectedValue}
|
||||
onChange={(e) => {
|
||||
const val = e.target.value;
|
||||
onChange={(val) => {
|
||||
if (!val) {
|
||||
setForm((f) => ({ ...f, defaultProvider: undefined, defaultModelId: undefined }));
|
||||
} else {
|
||||
@@ -303,23 +274,8 @@ export function SettingsModal({
|
||||
}));
|
||||
}
|
||||
}}
|
||||
>
|
||||
<option value="">Use default</option>
|
||||
{Object.entries(modelsByProvider).map(([provider, models]) => (
|
||||
<optgroup key={provider} label={provider}>
|
||||
{models.map((m) => (
|
||||
<option key={`${m.provider}/${m.id}`} value={`${m.provider}/${m.id}`}>
|
||||
{m.name}
|
||||
</option>
|
||||
))}
|
||||
</optgroup>
|
||||
))}
|
||||
</select>
|
||||
{filteredModels.length === 0 && modelFilter && (
|
||||
<div className="model-selector-no-results">
|
||||
No models match '{modelFilter}'
|
||||
</div>
|
||||
)}
|
||||
placeholder="Use default"
|
||||
/>
|
||||
<small>Select the AI model used for agent sessions. "Use default" lets the engine choose automatically.</small>
|
||||
</div>
|
||||
)}
|
||||
|
||||
@@ -372,27 +372,42 @@ describe("SettingsModal", () => {
|
||||
});
|
||||
|
||||
it("shows model selector with available models grouped by provider", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<SettingsModal onClose={onClose} addToast={addToast} />);
|
||||
await waitFor(() => expect(fetchSettings).toHaveBeenCalled());
|
||||
|
||||
fireEvent.click(screen.getByText("Model"));
|
||||
await waitFor(() => expect(fetchModels).toHaveBeenCalled());
|
||||
|
||||
expect(screen.getByLabelText("Default Model")).toBeTruthy();
|
||||
// Dropdown trigger should be present
|
||||
const trigger = screen.getByLabelText("Default Model");
|
||||
expect(trigger).toBeTruthy();
|
||||
expect(trigger.tagName).toBe("BUTTON");
|
||||
|
||||
// Open dropdown to see models
|
||||
await user.click(trigger);
|
||||
|
||||
// Models should be visible in dropdown
|
||||
expect(screen.getByText("Claude Sonnet 4.5")).toBeTruthy();
|
||||
expect(screen.getByText("GPT-4o")).toBeTruthy();
|
||||
expect(screen.getByText("Use default")).toBeTruthy();
|
||||
|
||||
// "Use default" appears twice (trigger text + dropdown option) - use getAllByText
|
||||
const useDefaultElements = screen.getAllByText("Use default");
|
||||
expect(useDefaultElements.length).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
it("selecting a model updates form with provider and model ID", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<SettingsModal onClose={onClose} addToast={addToast} />);
|
||||
await waitFor(() => expect(fetchSettings).toHaveBeenCalled());
|
||||
|
||||
fireEvent.click(screen.getByText("Model"));
|
||||
await waitFor(() => expect(fetchModels).toHaveBeenCalled());
|
||||
|
||||
const select = screen.getByLabelText("Default Model") as HTMLSelectElement;
|
||||
fireEvent.change(select, { target: { value: "anthropic/claude-sonnet-4-5" } });
|
||||
// Open dropdown and select a model
|
||||
const trigger = screen.getByLabelText("Default Model");
|
||||
await user.click(trigger);
|
||||
await user.click(screen.getByText("Claude Sonnet 4.5"));
|
||||
|
||||
fireEvent.click(screen.getByText("Save"));
|
||||
await waitFor(() => expect(updateSettings).toHaveBeenCalledTimes(1));
|
||||
@@ -403,6 +418,7 @@ describe("SettingsModal", () => {
|
||||
});
|
||||
|
||||
it("Use default option clears model selection", async () => {
|
||||
const user = userEvent.setup();
|
||||
(fetchSettings as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
|
||||
...defaultSettings,
|
||||
defaultProvider: "anthropic",
|
||||
@@ -415,8 +431,18 @@ describe("SettingsModal", () => {
|
||||
fireEvent.click(screen.getByText("Model"));
|
||||
await waitFor(() => expect(fetchModels).toHaveBeenCalled());
|
||||
|
||||
const select = screen.getByLabelText("Default Model") as HTMLSelectElement;
|
||||
fireEvent.change(select, { target: { value: "" } });
|
||||
// Open dropdown and select "Use default"
|
||||
const trigger = screen.getByLabelText("Default Model");
|
||||
await user.click(trigger);
|
||||
|
||||
// Find and click the "Use default" option in the dropdown
|
||||
const defaultOptions = screen.getAllByText("Use default");
|
||||
const dropdownDefault = defaultOptions.find((el) =>
|
||||
el.classList.contains("model-combobox-option-text--default")
|
||||
);
|
||||
if (dropdownDefault) {
|
||||
await user.click(dropdownDefault);
|
||||
}
|
||||
|
||||
fireEvent.click(screen.getByText("Save"));
|
||||
await waitFor(() => expect(updateSettings).toHaveBeenCalledTimes(1));
|
||||
@@ -542,15 +568,23 @@ describe("SettingsModal", () => {
|
||||
expect(providerRow).toBeTruthy();
|
||||
});
|
||||
|
||||
it("model section renders select element", async () => {
|
||||
it("model section renders CustomModelDropdown button", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<SettingsModal onClose={onClose} addToast={addToast} />);
|
||||
await waitFor(() => expect(fetchSettings).toHaveBeenCalled());
|
||||
|
||||
fireEvent.click(screen.getByText("Model"));
|
||||
await waitFor(() => expect(fetchModels).toHaveBeenCalled());
|
||||
|
||||
const select = screen.getByLabelText("Default Model") as HTMLSelectElement;
|
||||
expect(select.tagName).toBe("SELECT");
|
||||
// CustomModelDropdown renders as a button trigger, not a select element
|
||||
const trigger = screen.getByLabelText("Default Model");
|
||||
expect(trigger.tagName).toBe("BUTTON");
|
||||
expect(trigger).toHaveAttribute("aria-haspopup", "listbox");
|
||||
|
||||
// Open dropdown to verify it works
|
||||
await user.click(trigger);
|
||||
expect(trigger).toHaveAttribute("aria-expanded", "true");
|
||||
expect(screen.getByPlaceholderText("Filter models…")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("checkbox labels use checkbox-label class", async () => {
|
||||
@@ -907,15 +941,20 @@ describe("SettingsModal", () => {
|
||||
expect(screen.getByLabelText("ntfy Topic")).toBeTruthy();
|
||||
});
|
||||
|
||||
// Model filter tests
|
||||
it("renders filter input in Model section", async () => {
|
||||
// Model filter tests with CustomModelDropdown
|
||||
it("renders filter input in Model section dropdown", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<SettingsModal onClose={onClose} addToast={addToast} />);
|
||||
await waitFor(() => expect(fetchSettings).toHaveBeenCalled());
|
||||
|
||||
fireEvent.click(screen.getByText("Model"));
|
||||
await waitFor(() => expect(fetchModels).toHaveBeenCalled());
|
||||
|
||||
// Filter input should be present
|
||||
// Open dropdown to access filter input
|
||||
const trigger = screen.getByLabelText("Default Model");
|
||||
await user.click(trigger);
|
||||
|
||||
// Filter input should be present in dropdown
|
||||
expect(screen.getByPlaceholderText("Filter models…")).toBeTruthy();
|
||||
});
|
||||
|
||||
@@ -927,6 +966,10 @@ describe("SettingsModal", () => {
|
||||
fireEvent.click(screen.getByText("Model"));
|
||||
await waitFor(() => expect(fetchModels).toHaveBeenCalled());
|
||||
|
||||
// Open dropdown
|
||||
const trigger = screen.getByLabelText("Default Model");
|
||||
await user.click(trigger);
|
||||
|
||||
// Type a filter - only claude model should match
|
||||
const filterInput = screen.getByPlaceholderText("Filter models…");
|
||||
await user.type(filterInput, "claude");
|
||||
@@ -934,12 +977,9 @@ describe("SettingsModal", () => {
|
||||
// Should show result count (1 model matches "claude")
|
||||
expect(screen.getByText("1 model")).toBeTruthy();
|
||||
|
||||
// The select should be updated (Use default + 1 filtered model)
|
||||
const select = screen.getByLabelText("Default Model") as HTMLSelectElement;
|
||||
const options = Array.from(select.options).map((o) => o.textContent);
|
||||
expect(options).toContain("Use default");
|
||||
expect(options).toContain("Claude Sonnet 4.5");
|
||||
expect(options).not.toContain("GPT-4o");
|
||||
// Only Claude should be visible, GPT-4o should be filtered out
|
||||
expect(screen.getByText("Claude Sonnet 4.5")).toBeTruthy();
|
||||
expect(screen.queryByText("GPT-4o")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("clear button resets filter in Model section", async () => {
|
||||
@@ -950,6 +990,10 @@ describe("SettingsModal", () => {
|
||||
fireEvent.click(screen.getByText("Model"));
|
||||
await waitFor(() => expect(fetchModels).toHaveBeenCalled());
|
||||
|
||||
// Open dropdown
|
||||
const trigger = screen.getByLabelText("Default Model");
|
||||
await user.click(trigger);
|
||||
|
||||
// Type a filter
|
||||
const filterInput = screen.getByPlaceholderText("Filter models…");
|
||||
await user.type(filterInput, "openai");
|
||||
@@ -966,10 +1010,8 @@ describe("SettingsModal", () => {
|
||||
expect(screen.getByText("2 models")).toBeTruthy();
|
||||
|
||||
// All models should be visible again
|
||||
const select = screen.getByLabelText("Default Model") as HTMLSelectElement;
|
||||
const options = Array.from(select.options).map((o) => o.textContent);
|
||||
expect(options).toContain("GPT-4o");
|
||||
expect(options).toContain("Claude Sonnet 4.5");
|
||||
expect(screen.getByText("GPT-4o")).toBeTruthy();
|
||||
expect(screen.getByText("Claude Sonnet 4.5")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("shows empty state in Model section when filter matches nothing", async () => {
|
||||
@@ -980,12 +1022,16 @@ describe("SettingsModal", () => {
|
||||
fireEvent.click(screen.getByText("Model"));
|
||||
await waitFor(() => expect(fetchModels).toHaveBeenCalled());
|
||||
|
||||
// Open dropdown
|
||||
const trigger = screen.getByLabelText("Default Model");
|
||||
await user.click(trigger);
|
||||
|
||||
// Type a filter that matches nothing
|
||||
const filterInput = screen.getByPlaceholderText("Filter models…");
|
||||
await user.type(filterInput, "nonexistent");
|
||||
|
||||
// Should show no results message
|
||||
expect(screen.getByText("No models match 'nonexistent'")).toBeTruthy();
|
||||
expect(screen.getByText(/No models match/)).toBeTruthy();
|
||||
expect(screen.getByText("0 models")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user