diff --git a/src/features/agents/Composer.module.css b/src/features/agents/Composer.module.css index 08a7d505f..716a3f609 100644 --- a/src/features/agents/Composer.module.css +++ b/src/features/agents/Composer.module.css @@ -180,15 +180,21 @@ color: var(--fg-muted); } .modelEffortPopup { - width: min(300px, calc(100vw - 24px)); - padding: 4px 12px 12px; + width: min(340px, calc(100vw - 24px)); + padding: 12px 0 6px; +} +.effortBlock { + padding: 0 12px 8px; +} +/* The list takes the height of its models; with many it scrolls inside the popup. */ +.modelEffortPopup .modelList { + max-height: 216px; } .effortHeader { display: flex; align-items: baseline; justify-content: space-between; gap: 12px; - margin-top: 12px; } .effortValue { color: var(--fg-strong); diff --git a/src/features/agents/ConfigPickers.test.tsx b/src/features/agents/ConfigPickers.test.tsx index d2a347924..66745aa5f 100644 --- a/src/features/agents/ConfigPickers.test.tsx +++ b/src/features/agents/ConfigPickers.test.tsx @@ -234,8 +234,24 @@ describe("model and effort button", () => { expect(slider).toHaveAttribute("aria-valuetext", "high"); fireEvent.change(slider, { target: { value: "3" } }); expect(onChange).toHaveBeenLastCalledWith("reasoning_effort", "xhigh"); - fireEvent.click(within(dialog).getByRole("button", { name: "Model" })); - fireEvent.click(screen.getByRole("option", { name: /GPT-6 Sol/ })); + // The model list is in the popup itself, not behind a second dropdown. + expect(within(dialog).queryByRole("button", { name: "Model" })).not.toBeInTheDocument(); + fireEvent.click(within(dialog).getByRole("option", { name: /GPT-6 Sol/ })); + expect(onChange).toHaveBeenLastCalledWith("model", "gpt-6-sol"); + expect(screen.getByRole("dialog", { name: "Model & effort settings" })).toBeInTheDocument(); + }); + + it("filters the model list and picks the highlighted model with the keyboard", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "Model" })); + const dialog = screen.getByRole("dialog", { name: "Model & effort settings" }); + const search = within(dialog).getByPlaceholderText("Search…"); + expect(search).toHaveFocus(); + fireEvent.change(search, { target: { value: "sol" } }); + expect(within(dialog).getAllByRole("option")).toHaveLength(1); + fireEvent.keyDown(search, { key: "ArrowDown" }); + fireEvent.keyDown(search, { key: "Enter" }); expect(onChange).toHaveBeenLastCalledWith("model", "gpt-6-sol"); }); diff --git a/src/features/agents/ConfigPickers.tsx b/src/features/agents/ConfigPickers.tsx index 9118b2202..4223cfe85 100644 --- a/src/features/agents/ConfigPickers.tsx +++ b/src/features/agents/ConfigPickers.tsx @@ -9,7 +9,7 @@ import clsx from "clsx"; import type { AgentConfigOption } from "@/bindings"; import { useTx } from "@/platform/i18n"; import { Icon, IconButton, Popup, Select, T, Toggle } from "@/ui"; -import type { SelectOption } from "@/ui/Select"; +import { SelectListbox, type SelectOption } from "@/ui/Select"; import { mainOptions, modeIcon } from "./composerModel"; import styles from "./Composer.module.css"; @@ -241,15 +241,21 @@ function ModelEffortPicker({ aria-label={tx("Model & effort settings")} className={styles.modelEffortPopup} > -
- {tx(model.name)} - +
+
+ {tx(effort.name)} + {effortName} +
+ onChange(effort.id, v)} />
-
- {tx(effort.name)} - {effortName} -
- onChange(effort.id, v)} /> + onChange(model.id, v)} + /> ); diff --git a/src/features/agents/HomeEditor.test.tsx b/src/features/agents/HomeEditor.test.tsx index 57970f1cd..48a3b5b14 100644 --- a/src/features/agents/HomeEditor.test.tsx +++ b/src/features/agents/HomeEditor.test.tsx @@ -1,5 +1,5 @@ import { beforeEach, expect, it, vi } from "vitest"; -import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; import type { AgentConfigOption, AgentProvider, NewAgentTask } from "@/bindings"; import { HomeEditor } from "./HomeEditor"; import { HOME, savedDraft, updateComposer, useAgents } from "./store"; @@ -64,11 +64,7 @@ beforeEach(() => { }); async function chooseModel(model: string) { - const button = screen.getByRole("button", { name: "Model" }); - if (button.getAttribute("aria-haspopup") === "dialog") { - fireEvent.click(button); - fireEvent.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Model" })); - } else fireEvent.click(button); + fireEvent.click(screen.getByRole("button", { name: "Model" })); fireEvent.click(screen.getByRole("option", { name: model })); fireEvent.pointerDown(document.body); } diff --git a/src/ui/Select.module.css b/src/ui/Select.module.css index c3e155739..91374ff79 100644 --- a/src/ui/Select.module.css +++ b/src/ui/Select.module.css @@ -99,6 +99,7 @@ } /* The label keeps its width (up to most of the row); a long detail (a path) is cut instead. */ .withDetail { + flex-shrink: 0; max-width: 70%; } .detail { diff --git a/src/ui/Select.test.tsx b/src/ui/Select.test.tsx index 2e14d453c..1865b68af 100644 --- a/src/ui/Select.test.tsx +++ b/src/ui/Select.test.tsx @@ -1,6 +1,8 @@ import { describe, expect, it, vi } from "vitest"; import { fireEvent, render, screen, within, waitFor } from "@testing-library/react"; -import { Select, type SelectOption } from "./Select"; +import { StrictMode, useState } from "react"; +import { Select, SelectListbox, type SelectOption } from "./Select"; +import { Dialog } from "./Dialog"; const options: SelectOption[] = [ { value: "9router", label: "9router", section: true }, @@ -106,3 +108,62 @@ describe("Select", () => { expect(changed).toHaveBeenCalledExactlyOnceWith("a"); }); }); + +describe("Select preview lifecycle", () => { + const rows = [ + { value: "a", label: "A" }, + { value: "b", label: "B" }, + ]; + + it("restores using the newly selected value when closing", () => { + const preview = vi.fn(); + function Picker() { + const [value, setValue] = useState("a"); + return ( + + , + ); + const trigger = screen.getByRole("button", { name: "Theme" }); + trigger.focus(); + fireEvent.click(trigger); + expect(within(list()).getByRole("option", { name: "B" })).toHaveAttribute("aria-selected", "true"); + key("Escape"); + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + expect(close).not.toHaveBeenCalled(); + expect(trigger).toHaveFocus(); + }); +}); diff --git a/src/ui/Select.tsx b/src/ui/Select.tsx index 554fe9c33..9019f6faa 100644 --- a/src/ui/Select.tsx +++ b/src/ui/Select.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState, type KeyboardEvent, type ReactNode } from "react"; +import { useCallback, useEffect, useLayoutEffect, useRef, useState, type KeyboardEvent, type ReactNode } from "react"; import clsx from "clsx"; import { Icon, type IconName } from "./icons"; import { Popup } from "./Popup"; @@ -51,25 +51,35 @@ function containsOrFuzzy(haystack: string, needle: string): boolean { return false; } -/** Dropdown select with a listbox popup (↑↓ Enter Esc, type-ahead). */ -export function Select({ +export interface SelectListboxProps { + value: V | null; + options: readonly SelectOption[]; + /** Called with the chosen option; closing, if any, is the caller's. */ + onChoose: (value: V) => void; + searchable?: boolean; + onPreview?: (value: V | null) => void; + /** Extra class for the scrolling list, e.g. to set its height. */ + listClassName?: string; + "aria-label"?: string; +} + +/** The searchable listbox of a {@link Select}, for a popup that shows it without a trigger button. */ +export function SelectListbox({ value, options, - onChange, - placeholder = "Select…", - disabled, - variant = "framed", - className, - renderValue, + onChoose, searchable = false, onPreview, + listClassName, ...aria -}: SelectProps) { +}: SelectListboxProps) { const given = useGivenTx(); - const [open, setOpen] = useState(false); - const triggerRef = useRef(null); - const current = options.find((o) => !o.section && o.value === value); - const [active, setActive] = useState(0); + const [active, setActive] = useState(() => + Math.max( + 0, + options.findIndex((o) => !o.section && !o.disabled && o.value === value), + ), + ); const [query, setQuery] = useState(""); const typeAhead = useRef({ text: "", at: -Infinity }); const isSection = (o: SelectOption) => !!o.section; @@ -117,29 +127,29 @@ export function Select({ const activeIndex = filtered[active] && !unavailable(filtered[active]!) ? active : filtered.findIndex((o) => !unavailable(o)); - const previewed = open ? (filtered[activeIndex]?.value ?? null) : null; + const previewed = filtered[activeIndex]?.value ?? null; const lastPreview = useRef(null); + const previewRef = useRef(onPreview); + useLayoutEffect(() => { + previewRef.current = onPreview; + }); useEffect(() => { if (previewed === lastPreview.current) return; lastPreview.current = previewed; onPreview?.(previewed); }, [previewed, onPreview]); - - const openPopup = () => { - setQuery(""); - setActive( - Math.max( - 0, - options.findIndex((o) => !unavailable(o) && o.value === value), - ), - ); - setOpen(true); - }; + // The list is only mounted while open: closing it ends the preview. + useEffect( + () => () => { + if (lastPreview.current !== null) previewRef.current?.(null); + lastPreview.current = null; + }, + [], + ); const choose = (o: SelectOption | undefined) => { if (!o || unavailable(o)) return; - onChange(o.value); - setOpen(false); + onChoose(o.value); }; const onPopupKey = (e: KeyboardEvent) => { @@ -150,7 +160,12 @@ export function Select({ return; } if (searching && !["ArrowDown", "ArrowUp"].includes(e.key)) return; - const next = listNavigate(e.key, { items: filtered, active: activeIndex, isSection: unavailable, wrap: true }); + const next = listNavigate(e.key, { + items: filtered, + active: activeIndex, + isSection: unavailable, + wrap: true, + }); if (next !== null) { e.preventDefault(); setActive(next); @@ -174,6 +189,101 @@ export function Select({ } }; + return ( +
+ {searchable && ( +
+ { + setQuery(v); + setActive(-1); + }} + size="normal" + variant="framed" + className={styles.search} + /> +
+ )} + {/* Virtualised rows are absolutely positioned; this sizes the popup to the widest label. */} +
+ {options.map((o) => ( +
+ + {o.icon && } + {given(o.label)} + {o.detail && {given(o.detail)}} +
+ ))} +
+ `${o.section ? "section" : "option"}:${o.value}`} + isSection={isSection} + renderSection={(o) => {given(o.label)}} + activeIndex={activeIndex} + onActiveChange={(i) => { + if (!unavailable(filtered[i]!)) setActive(i); + }} + onActivate={(o) => choose(o)} + focusable={false} + hoverSelects + rowHeight={24} + aria-label={given(aria["aria-label"])} + className={clsx(styles.list, listClassName)} + renderItem={(o) => ( + + {o.value === value && } + {o.icon && } + + {given(o.label)} + + {o.detail && ( + + {given(o.detail)} + + )} + + )} + /> +
+ ); +} + +/** Dropdown select with a listbox popup (↑↓ Enter Esc, type-ahead). */ +export function Select({ + value, + options, + onChange, + placeholder = "Select…", + disabled, + variant = "framed", + className, + renderValue, + searchable = false, + onPreview, + ...aria +}: SelectProps) { + const given = useGivenTx(); + const [open, setOpen] = useState(false); + const triggerRef = useRef(null); + const current = options.find((o) => !o.section && o.value === value); + const openPopup = () => setOpen(true); + // Selection can update the callback as the list unmounts; use the committed trigger props. + const previewRef = useRef(onPreview); + useLayoutEffect(() => { + previewRef.current = onPreview; + }); + const preview = useCallback((next: V | null) => previewRef.current?.(next), []); + return ( <>