From 8a69803afd642e1b3c98af6ebe1e3119dc0f2665 Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:00:38 +0100 Subject: [PATCH 1/3] fix(web): append new model favourites instead of prepending Starring a model no longer shifts every existing favourite down one row, so a favourite keeps its position and its Ctrl/Cmd+N number. At the 40-entry cap the oldest favourite drops. Persisted order loads as is. --- .../__tests__/modelFavoritesStore.test.ts | 48 +++++++++++++++++++ apps/web/src/stores/modelFavoritesStore.ts | 4 +- 2 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 apps/web/src/stores/__tests__/modelFavoritesStore.test.ts diff --git a/apps/web/src/stores/__tests__/modelFavoritesStore.test.ts b/apps/web/src/stores/__tests__/modelFavoritesStore.test.ts new file mode 100644 index 000000000..c8ba17e2a --- /dev/null +++ b/apps/web/src/stores/__tests__/modelFavoritesStore.test.ts @@ -0,0 +1,48 @@ +// @vitest-environment jsdom +import { describe, it, expect, beforeEach } from "vitest"; +import { useModelFavoritesStore, type ModelFavoriteEntry } from "../modelFavoritesStore"; + +function fav(modelId: string): ModelFavoriteEntry { + return { providerId: "claude", modelId, label: modelId }; +} + +const ids = () => useModelFavoritesStore.getState().entries.map((e) => e.modelId); +const toggle = (modelId: string) => useModelFavoritesStore.getState().toggleFavorite(fav(modelId)); + +describe("modelFavoritesStore", () => { + beforeEach(() => { + localStorage.clear(); + useModelFavoritesStore.setState({ entries: [] }); + }); + + it("appends a new favourite so existing ones keep their positions", () => { + toggle("a"); + toggle("b"); + toggle("c"); + expect(ids()).toEqual(["a", "b", "c"]); + }); + + it("unstarring removes only that entry", () => { + ["a", "b", "c"].forEach(toggle); + toggle("b"); + expect(ids()).toEqual(["a", "c"]); + }); + + it("keeps a persisted order as loaded", async () => { + localStorage.setItem( + "mcode-model-favorites", + JSON.stringify({ state: { entries: [fav("z"), fav("a"), fav("m")] }, version: 0 }), + ); + await useModelFavoritesStore.persist.rehydrate(); + expect(ids()).toEqual(["z", "a", "m"]); + toggle("n"); + expect(ids()).toEqual(["z", "a", "m", "n"]); + }); + + it("evicts the oldest favourite past the 40 cap", () => { + const forty = Array.from({ length: 40 }, (_, i) => `m${i}`); + useModelFavoritesStore.setState({ entries: forty.map(fav) }); + toggle("new"); + expect(ids()).toEqual([...forty.slice(1), "new"]); + }); +}); diff --git a/apps/web/src/stores/modelFavoritesStore.ts b/apps/web/src/stores/modelFavoritesStore.ts index 22a997489..1e1572d81 100644 --- a/apps/web/src/stores/modelFavoritesStore.ts +++ b/apps/web/src/stores/modelFavoritesStore.ts @@ -14,7 +14,7 @@ export interface ModelFavoriteEntry { /** Persisted favorite models for {@link ModelSelector}. */ interface ModelFavoritesState { entries: ModelFavoriteEntry[]; - /** Adds or removes a favorite keyed by provider and model id. */ + /** Adds or removes a favorite keyed by provider and model id. New favorites append; the oldest drops at the cap. */ toggleFavorite: (entry: ModelFavoriteEntry) => void; isFavorite: (providerId: string, modelId: string) => boolean; } @@ -40,7 +40,7 @@ export const useModelFavoritesStore = create()( modelId: entry.modelId, label, }; - return { entries: [row, ...s.entries].slice(0, MAX_FAVORITES) }; + return { entries: [...s.entries, row].slice(-MAX_FAVORITES) }; }); }, isFavorite: (providerId, modelId) => From 3291e3a156cf57161331764a78af548d13dda77f Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:00:38 +0100 Subject: [PATCH 2/3] feat(web): pick models with Ctrl/Cmd+1..9 While the model picker is open, Ctrl/Cmd+1..9 select the Nth selectable row of the active tab and close the picker. Each numbered row shows its keycap. Disabled rows get no number, and search renumbers the rows. Picker gains an opt-in numberedShortcuts prop; only ModelSelector uses it. Closes #2226 --- .../web/src/components/chat/ModelSelector.tsx | 1 + .../chat/__tests__/ModelSelector.test.tsx | 46 ++++++++++++- .../components/ui/__tests__/picker.test.tsx | 56 ++++++++++++++++ apps/web/src/components/ui/picker.tsx | 65 ++++++++++++++++--- 4 files changed, 158 insertions(+), 10 deletions(-) diff --git a/apps/web/src/components/chat/ModelSelector.tsx b/apps/web/src/components/chat/ModelSelector.tsx index 624fbf12c..b6cce16dc 100644 --- a/apps/web/src/components/chat/ModelSelector.tsx +++ b/apps/web/src/components/chat/ModelSelector.tsx @@ -613,6 +613,7 @@ export function ModelSelector({ className={cn(PICKER_PANEL_CLASS, "max-h-(--available-height) overflow-y-auto")} > ({ @@ -63,10 +65,10 @@ async function openPicker() { } function optionNames(): string[] { - // Hidden description spans are part of textContent, so drop them to read only what the row shows. + // Hidden description spans and shortcut keycaps are part of textContent, so drop them to read only the row name. return screen.getAllByRole("option").map((option) => { const visible = option.cloneNode(true) as HTMLElement; - visible.querySelectorAll("[hidden]").forEach((node) => node.remove()); + visible.querySelectorAll("[hidden], [data-slot=kbd]").forEach((node) => node.remove()); return visible.textContent ?? ""; }); } @@ -310,4 +312,44 @@ describe("ModelSelector", () => { expect(within(dialog).queryByRole("option")).toBeNull(); expect(screen.getByLabelText("Model locked while the agent runs")).toBeInTheDocument(); }); + + async function openCodexTab(onSelect: () => void) { + setProviders({ claude: true, codex: true }); + listProviderModels.mockResolvedValue([ + { id: "gpt-9-zeta", name: "GPT-9 Zeta", group: "OpenAI" }, + { id: "gpt-9-alpha", name: "GPT-9 Alpha", group: "OpenAI" }, + ]); + render(); + await openPicker(); + await userEvent.click(screen.getByRole("radio", { name: "Codex" })); + await waitFor(() => expect(optionNames()).toEqual(["GPT-9 Zeta", "GPT-9 Alpha"])); + screen.getByRole("combobox").focus(); + } + + it("picks the second model with Ctrl+2 and closes", async () => { + const onSelect = vi.fn(); + await openCodexTab(onSelect); + + await userEvent.keyboard("{Control>}2{/Control}"); + + expect(onSelect).toHaveBeenCalledWith("gpt-9-alpha", "codex"); + await waitFor(() => expect(screen.queryByRole("dialog")).toBeNull()); + }); + + it("picks a model with Ctrl+1 instead of switching threads", async () => { + onTestFinished(initShortcuts()); + const goTo1 = vi.fn(); + onTestFinished(registerCommand({ id: "thread.goTo1", title: "Go to thread 1", category: "Navigation", handler: goTo1 })); + await userEvent.keyboard("{Control>}1{/Control}"); + expect(goTo1).toHaveBeenCalledTimes(1); + goTo1.mockClear(); + + const onSelect = vi.fn(); + await openCodexTab(onSelect); + await userEvent.keyboard("{Control>}1{/Control}"); + + expect(goTo1).not.toHaveBeenCalled(); + expect(onSelect).toHaveBeenCalledWith("gpt-9-zeta", "codex"); + }); }); diff --git a/apps/web/src/components/ui/__tests__/picker.test.tsx b/apps/web/src/components/ui/__tests__/picker.test.tsx index c6d40d0d2..2ecbb9365 100644 --- a/apps/web/src/components/ui/__tests__/picker.test.tsx +++ b/apps/web/src/components/ui/__tests__/picker.test.tsx @@ -455,3 +455,59 @@ describe("Picker", () => { expect(screen.getByRole("option", { name: "branch-008" })).not.toHaveAttribute("data-selected"); }); }); + +function NumberedPicker({ onSelect, numbered = true }: { readonly onSelect: (name: string) => void; readonly numbered?: boolean }) { + const [query, setQuery] = useState(""); + const names = ["alpha", "bravo", "charlie", "delta"].filter((name) => name.includes(query)); + return ( + + numberedShortcuts={numbered} + query={query} + onQueryChange={setQuery} + items={names} + total={names.length} + status="ready" + renderItem={(name) => ({ key: name, name, disabled: name === "bravo" })} + onSelect={onSelect} + /> + ); +} + +function chipOf(name: string): string | null { + return screen.getByRole("option", { name: new RegExp(name) }).querySelector("[data-slot=kbd]")?.textContent ?? null; +} + +describe("Picker numbered shortcuts", () => { + it("numbers only the selectable rows", () => { + render( {}} />); + expect(["alpha", "bravo", "charlie", "delta"].map(chipOf)).toEqual(["Ctrl+1", null, "Ctrl+2", "Ctrl+3"]); + expect(screen.getByRole("option", { name: /charlie/ })).toHaveAttribute("aria-keyshortcuts", "Control+2"); + }); + + it("picks the Nth selectable row with Ctrl or Meta, skipping disabled rows", () => { + const onSelect = vi.fn(); + render(); + const search = screen.getByRole("combobox"); + fireEvent.keyDown(search, { key: "2", ctrlKey: true }); + fireEvent.keyDown(search, { key: "2", metaKey: true }); + expect(onSelect.mock.calls).toEqual([["charlie"], ["charlie"]]); + }); + + it("renumbers the rows a search leaves", async () => { + const onSelect = vi.fn(); + render(); + const search = screen.getByRole("combobox"); + await userEvent.type(search, "del"); + expect(chipOf("delta")).toBe("Ctrl+1"); + fireEvent.keyDown(search, { key: "1", ctrlKey: true }); + expect(onSelect).toHaveBeenCalledWith("delta"); + }); + + it("does nothing without the prop", () => { + const onSelect = vi.fn(); + render(); + fireEvent.keyDown(screen.getByRole("combobox"), { key: "1", ctrlKey: true }); + expect(onSelect).not.toHaveBeenCalled(); + expect(chipOf("alpha")).toBeNull(); + }); +}); diff --git a/apps/web/src/components/ui/picker.tsx b/apps/web/src/components/ui/picker.tsx index 635a862e2..df64caf90 100644 --- a/apps/web/src/components/ui/picker.tsx +++ b/apps/web/src/components/ui/picker.tsx @@ -13,7 +13,10 @@ import { } from "react"; import { CheckIcon, SearchIcon } from "lucide-react"; +import { formatKeybinding } from "@/lib/keybinding-manager"; +import { isMac } from "@/lib/platform"; import { cn } from "@/lib/utils"; +import { Kbd } from "./kbd"; import { Button } from "./button"; import { SegmentedControl } from "./segmented-control"; import { Spinner } from "./spinner"; @@ -109,6 +112,8 @@ export interface PickerProps { readonly hairlines?: boolean; /** Set false to mark the selected row by its fill alone, without the trailing check. Defaults to true. */ readonly selectedCheck?: boolean; + /** Ctrl/Cmd+1..9 pick the Nth selectable row, and each such row shows its keycap. Defaults to false. */ + readonly numberedShortcuts?: boolean; readonly onSelect: (item: T) => void; } @@ -128,12 +133,14 @@ interface ListRow { * scroll fades, and a count footer. It renders the panel body only, so callers choose the surface * (popover, dialog or inline); put the search field first so the host's initial focus lands on it. * Focus stays in the search field; the arrow keys move a highlight through the listbox via - * `aria-activedescendant`, Home and End reach its ends, and Enter picks the highlighted row. + * `aria-activedescendant`, Home and End reach its ends, and Enter picks the highlighted row. With + * `numberedShortcuts`, Ctrl/Cmd+1..9 pick the first nine selectable rows directly. */ export function Picker(props: PickerProps) { const { query, onQueryChange, searchPlaceholder = "Search", items, renderItem, selectedKey, status, hairlines = true, selectedCheck = true } = props; const listId = useId(); const rows = useMemo[]>(() => items.map((item) => ({ item, row: renderItem(item) })), [items, renderItem]); + const shortcutRows = useShortcutRows(rows, props.numberedShortcuts === true); const listKey = `${props.activeTab ?? ""}\u0000${query}`; const { activeIndex, setHighlight } = useActiveRow(rows, listKey, selectedKey); const loadMore = useLoadMoreOnce(props, listKey); @@ -145,6 +152,13 @@ export function Picker(props: PickerProps) { const handleKeyDown = (event: KeyboardEvent) => { // Enter that ends an IME composition commits text, not a row. if (event.nativeEvent.isComposing) return; + const digit = props.numberedShortcuts ? numberShortcutDigit(event) : null; + if (digit !== null) { + event.preventDefault(); + const index = shortcutRows[digit - 1]; + if (index !== undefined) select(rows[index]); + return; + } if (event.key === "Enter") { event.preventDefault(); select(rows[activeIndex]); @@ -208,6 +222,7 @@ export function Picker(props: PickerProps) { listId={listId} listKey={listKey} rows={rows} + shortcutRows={shortcutRows} activeIndex={activeIndex} selectedKey={selectedKey} onHighlight={highlightRow} @@ -379,11 +394,23 @@ function useLoadMoreOnce({ onLoadMore, items, total, status }: PickerProps }, [onLoadMore, status, total, items.length, visit.count]); } +function useShortcutRows(rows: readonly ListRow[], enabled: boolean): readonly number[] { + return useMemo(() => { + if (!enabled) return []; + const indexes: number[] = []; + for (let index = 0; index < rows.length && indexes.length < 9; index += 1) { + if (!rows[index]?.row.disabled) indexes.push(index); + } + return indexes; + }, [rows, enabled]); +} + interface PickerListProps { readonly listId: string; /** Changes when the tab or query starts a new list, which scrolls back to the top. */ readonly listKey: string; readonly rows: readonly ListRow[]; + readonly shortcutRows: readonly number[]; readonly activeIndex: number; readonly selectedKey?: string; readonly onHighlight: (index: number) => void; @@ -395,7 +422,7 @@ interface PickerListProps { } function PickerList(props: PickerListProps) { - const { listId, listKey, rows, activeIndex, selectedKey, onHighlight, onPick, onRunAction, onNearEnd, hairlines, selectedCheck } = props; + const { listId, listKey, rows, shortcutRows, activeIndex, selectedKey, onHighlight, onPick, onRunAction, onNearEnd, hairlines, selectedCheck } = props; const listRef = useRef(null); const measure = useCallback(() => { @@ -434,6 +461,7 @@ function PickerList(props: PickerListProps) { {reason ? : null} @@ -505,8 +539,13 @@ function optionDescriptions(id: string, row: PickerRow, reason: string | undefin }; } -function PickerOptionItem({ id, row, active, selected, selectedCheck, onHighlight, onPick, onRunAction, reason, ...triggerProps }: PickerOptionProps & { readonly reason: string | undefined }) { - const descriptions = optionDescriptions(id, row, reason); +function optionKeyShortcuts(digit: number | undefined, hasAction: boolean): string | undefined { + const pick = digit === undefined ? undefined : `${isMac ? "Meta" : "Control"}+${digit}`; + return [pick, hasAction ? ROW_ACTION_SHORTCUT.aria : undefined].filter(Boolean).join(" ") || undefined; +} + +function PickerOptionItem({ id, row, digit, active, selected, selectedCheck, onHighlight, onPick, onRunAction, reason, ...triggerProps }: PickerOptionProps & { readonly reason: string | undefined }) { + const descriptions = optionDescriptions(id, row, reason, digit); return (
  • {/* A disabled row keeps its action for the pointer, so a starred row whose model went away can be unstarred. */} - + {descriptions.spans}
    • ); } -function PickerOptionContent({ row, selected, selectedCheck, onRunAction }: { readonly row: PickerRow; readonly selected: boolean; readonly selectedCheck: boolean; readonly onRunAction: () => void }) { +function PickerOptionContent({ row, digit, selected, selectedCheck, onRunAction }: { readonly row: PickerRow; readonly digit: number | undefined; readonly selected: boolean; readonly selectedCheck: boolean; readonly onRunAction: () => void }) { return ( <> {row.icon != null ? ( @@ -553,6 +592,11 @@ function PickerOptionContent({ row, selected, selectedCheck, onRunAction }: { re ) : null} {row.tag ? {row.tag} : null} + {digit !== undefined ? ( + + {formatKeybinding(`mod+${digit}`, isMac)} + + ) : null} {row.action ? : null} {selectedCheck ? ( @@ -591,6 +635,11 @@ function isRowActionShortcut(event: KeyboardEvent): boolean { return (event.ctrlKey || event.metaKey) && !event.altKey && !event.shiftKey && event.key.toLowerCase() === "d"; } +function numberShortcutDigit(event: KeyboardEvent): number | null { + if (!(event.ctrlKey || event.metaKey) || event.altKey || event.shiftKey) return null; + return /^[1-9]$/.test(event.key) ? Number(event.key) : null; +} + function PickerFailure({ status, onRetry }: { readonly status: { readonly failed: string; readonly detail?: string }; readonly onRetry?: () => void }) { return (
    From 9c34753ac34280cd64904567f7525ad705d72947 Mon Sep 17 00:00:00 2001 From: "(CJ) Chukwudi Nwobodo" <142016413+chuks-qua@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:09:24 +0100 Subject: [PATCH 3/3] fix(web): keep model shortcuts working after a tab click Clicking a picker tab moves focus onto the tab, so Ctrl/Cmd+1..9 bound on the search field stopped picking rows, and Ctrl+1 could reach the global thread switch. Handle the chord on the picker panel and stop it there. The tests no longer refocus the search field after the tab click, which had hidden the bug. --- .../chat/__tests__/ModelSelector.test.tsx | 1 - apps/web/src/components/ui/picker.tsx | 22 +++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/apps/web/src/components/chat/__tests__/ModelSelector.test.tsx b/apps/web/src/components/chat/__tests__/ModelSelector.test.tsx index ba4ae847e..989a7c4a2 100644 --- a/apps/web/src/components/chat/__tests__/ModelSelector.test.tsx +++ b/apps/web/src/components/chat/__tests__/ModelSelector.test.tsx @@ -324,7 +324,6 @@ describe("ModelSelector", () => { await openPicker(); await userEvent.click(screen.getByRole("radio", { name: "Codex" })); await waitFor(() => expect(optionNames()).toEqual(["GPT-9 Zeta", "GPT-9 Alpha"])); - screen.getByRole("combobox").focus(); } it("picks the second model with Ctrl+2 and closes", async () => { diff --git a/apps/web/src/components/ui/picker.tsx b/apps/web/src/components/ui/picker.tsx index df64caf90..fb1dcc387 100644 --- a/apps/web/src/components/ui/picker.tsx +++ b/apps/web/src/components/ui/picker.tsx @@ -152,13 +152,6 @@ export function Picker(props: PickerProps) { const handleKeyDown = (event: KeyboardEvent) => { // Enter that ends an IME composition commits text, not a row. if (event.nativeEvent.isComposing) return; - const digit = props.numberedShortcuts ? numberShortcutDigit(event) : null; - if (digit !== null) { - event.preventDefault(); - const index = shortcutRows[digit - 1]; - if (index !== undefined) select(rows[index]); - return; - } if (event.key === "Enter") { event.preventDefault(); select(rows[activeIndex]); @@ -176,6 +169,17 @@ export function Picker(props: PickerProps) { moveHighlight(target); }; + // Bound on the panel, not the search field: clicking a tab moves focus onto it, and the chord must still pick a row. + const handleNumberShortcut = (event: KeyboardEvent) => { + const digit = props.numberedShortcuts && !event.nativeEvent.isComposing ? numberShortcutDigit(event) : null; + if (digit === null) return; + // Stopping here keeps the chord from the browser's tab switch and the global Ctrl+N thread switch. + event.preventDefault(); + event.stopPropagation(); + const index = shortcutRows[digit - 1]; + if (index !== undefined) select(rows[index]); + }; + const runRowAction = (index: number) => { const action = rows[index]?.row.action; if (!action) return; @@ -199,7 +203,7 @@ export function Picker(props: PickerProps) { const hasRows = rows.length > 0; return ( -
    +
    ): boolean { return (event.ctrlKey || event.metaKey) && !event.altKey && !event.shiftKey && event.key.toLowerCase() === "d"; } -function numberShortcutDigit(event: KeyboardEvent): number | null { +function numberShortcutDigit(event: KeyboardEvent): number | null { if (!(event.ctrlKey || event.metaKey) || event.altKey || event.shiftKey) return null; return /^[1-9]$/.test(event.key) ? Number(event.key) : null; }