From 9c8060d96db2ed9fa6b45f8b19f8ec2aefadf2eb Mon Sep 17 00:00:00 2001 From: Brendan Allan <14191578+Brendonovich@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:18:27 +0800 Subject: [PATCH] refactor(app): extract keybind settings controller (#39226) --- .../app/src/components/settings-keybinds.tsx | 350 +++++++++++++++--- .../test-browser/settings-keybinds.test.ts | 107 ++++++ 2 files changed, 396 insertions(+), 61 deletions(-) create mode 100644 packages/app/test-browser/settings-keybinds.test.ts diff --git a/packages/app/src/components/settings-keybinds.tsx b/packages/app/src/components/settings-keybinds.tsx index cdec8b435c..ce74be4586 100644 --- a/packages/app/src/components/settings-keybinds.tsx +++ b/packages/app/src/components/settings-keybinds.tsx @@ -30,6 +30,8 @@ type KeybindMeta = { type KeybindMap = Record type CommandContext = ReturnType +type LanguageContext = ReturnType +type SettingsContext = ReturnType const GROUPS: KeybindGroup[] = ["General", "Session", "Navigation", "Model and agent", "Terminal", "Prompt"] @@ -122,7 +124,7 @@ function keybinds(value: unknown): KeybindMap { return value as KeybindMap } -function listFor(command: CommandContext, map: KeybindMap, palette: string) { +function listFor(command: Pick, map: KeybindMap, palette: string) { const out = new Map() out.set(PALETTE_ID, { title: palette, group: "General" }) @@ -262,7 +264,274 @@ function useKeyCapture(input: { }) } +export function createKeybindSettingsController( + input: { + command: Pick + settings: { + current: { keybinds: unknown } + keybinds: Pick + } + target?: Document + notify?: (toast: { title: string; description: string }) => void + }, + language: Pick = useLanguage(), +) { + const [store, setStore] = createStore({ active: null as string | null }) + const overrides = createMemo(() => keybinds(input.settings.current.keybinds)) + const list = createMemo(() => { + language.locale() + return listFor(input.command, overrides(), language.t("command.palette")) + }) + const grouped = createMemo(() => groupedFor(list())) + const title = (id: string) => list().get(id)?.title ?? "" + const effective = (id: string) => { + if (id === PALETTE_ID) return input.settings.keybinds.get(id) ?? DEFAULT_PALETTE_KEYBIND + + const custom = input.settings.keybinds.get(id) + if (typeof custom === "string") return custom + + const live = input.command.options.find((item) => item.id === id) + if (live?.keybind) return live.keybind + return input.command.catalog.find((item) => item.id === id)?.keybind + } + const used = createMemo(() => { + const value = new Map() + + for (const id of list().keys()) { + for (const signature of signatures(effective(id))) { + const items = value.get(signature) + if (items) { + items.push({ id, title: title(id) }) + continue + } + value.set(signature, [{ id, title: title(id) }]) + } + } + + return value + }) + const stop = () => { + if (!store.active) return + setStore("active", null) + input.command.keybinds(true) + } + const toggle = (id: string) => { + if (store.active === id) { + stop() + return + } + if (store.active) stop() + setStore("active", id) + input.command.keybinds(false) + } + const notify = input.notify ?? ((toast: { title: string; description: string }) => showToast(toast)) + + const handle = (event: KeyboardEvent) => { + const id = store.active + if (!id) return + + event.preventDefault() + event.stopPropagation() + event.stopImmediatePropagation() + + if (event.key === "Escape") { + stop() + return + } + + const clear = + (event.key === "Backspace" || event.key === "Delete") && + !event.ctrlKey && + !event.metaKey && + !event.altKey && + !event.shiftKey + if (clear) { + input.settings.keybinds.set(id, "none") + stop() + return + } + + const next = recordKeybind(event) + if (!next) return + + const conflicts = new Map() + for (const signature of signatures(next)) { + for (const item of used().get(signature) ?? []) { + if (item.id === id) continue + conflicts.set(item.id, item.title) + } + } + + if (conflicts.size > 0) { + notify({ + title: language.t("settings.shortcuts.conflict.title"), + description: language.t("settings.shortcuts.conflict.description", { + keybind: formatKeybind(next, language.t), + titles: [...conflicts.values()].join(", "), + }), + }) + return + } + + input.settings.keybinds.set(id, next) + stop() + } + + const target = input.target ?? (typeof document === "object" ? document : undefined) + if (target) makeEventListener(target, "keydown", handle, { capture: true }) + + onCleanup(() => { + if (store.active) input.command.keybinds(true) + }) + + return { + catalog: { + groups: GROUPS, + filtered: (query: string) => + filteredFor(query, list(), grouped(), (id) => formatKeybind(effective(id) ?? "", language.t)), + title, + keybind: (id: string) => formatKeybind(effective(id) ?? "", language.t), + }, + capture: { + active: () => store.active, + toggle, + }, + settings: { + hasOverrides: () => Object.values(overrides()).some((value) => typeof value === "string"), + reset: () => { + stop() + input.settings.keybinds.resetAll() + notify({ + title: language.t("settings.shortcuts.reset.toast.title"), + description: language.t("settings.shortcuts.reset.toast.description"), + }) + }, + }, + } +} + +function SettingsKeybindsV2() { + const command = useCommand() + const settings = useSettings() + const controller = createKeybindSettingsController({ + command, + settings, + }) + + return ( + + ) +} + +function SettingsKeybindsV2View(props: { + groups: KeybindGroup[] + filtered: (query: string) => Map + title: (id: string) => string + keybind: (id: string) => string + active: () => string | null + onCapture: (id: string) => void + hasOverrides: () => boolean + onReset: () => void +}) { + const language = useLanguage() + const [store, setStore] = createStore({ filter: "" }) + const filtered = createMemo(() => props.filtered(store.filter)) + const hasResults = createMemo(() => props.groups.some((group) => (filtered().get(group)?.length ?? 0) > 0)) + + return ( + <> +
+
+

{language.t("settings.shortcuts.title")}

+ + {language.t("settings.shortcuts.reset.button")} + +
+
+ setStore("filter", event.currentTarget.value)} + placeholder={language.t("settings.shortcuts.search.placeholder")} + spellcheck={false} + autocorrect="off" + autocomplete="off" + autocapitalize="off" + aria-label={language.t("settings.shortcuts.search.placeholder")} + /> + + } + onClick={() => setStore("filter", "")} + /> + +
+
+
+
+ + {(group) => ( + 0}> +
+

{language.t(groupKey[group])}

+ + + {(id) => ( +
+ {props.title(id)} + +
+ )} + + +
+ + )} + + +
+ {language.t("settings.shortcuts.search.empty")} + "{store.filter}" +
+ +
+
+ + ) +} + export const SettingsKeybinds: Component<{ v2?: boolean }> = (props) => { + if (props.v2) return + const command = useCommand() const language = useLanguage() const settings = useSettings() @@ -476,78 +745,37 @@ export const SettingsKeybinds: Component<{ v2?: boolean }> = (props) => { ) return ( - -
-
-
-

{language.t("settings.shortcuts.title")}

- -
- -
- - setStore("filter", v)} - placeholder={language.t("settings.shortcuts.search.placeholder")} - spellcheck={false} - autocorrect="off" - autocomplete="off" - autocapitalize="off" - class="flex-1" - /> - - setStore("filter", "")} /> - -
-
-
- {groups} - - } - > - <> -
-
-

{language.t("settings.shortcuts.title")}

- +
+
+
+
+

{language.t("settings.shortcuts.title")}

+
-
- + + setStore("filter", event.currentTarget.value)} + onChange={(v) => setStore("filter", v)} placeholder={language.t("settings.shortcuts.search.placeholder")} spellcheck={false} autocorrect="off" autocomplete="off" autocapitalize="off" - aria-label={language.t("settings.shortcuts.search.placeholder")} + class="flex-1" /> - } - onClick={() => setStore("filter", "")} - /> + setStore("filter", "")} />
-
{groups}
- - +
+ {groups} +
) } diff --git a/packages/app/test-browser/settings-keybinds.test.ts b/packages/app/test-browser/settings-keybinds.test.ts new file mode 100644 index 0000000000..75f0b75543 --- /dev/null +++ b/packages/app/test-browser/settings-keybinds.test.ts @@ -0,0 +1,107 @@ +import { describe, expect, test } from "bun:test" +import { createRoot } from "solid-js" +import { createKeybindSettingsController } from "../src/components/settings-keybinds" + +function setup(overrides: Record = {}) { + const changes: [string, string][] = [] + const suppression: boolean[] = [] + const notifications: { title: string; description?: string }[] = [] + let resets = 0 + let controller: ReturnType + + const dispose = createRoot((dispose) => { + controller = createKeybindSettingsController( + { + command: { + catalog: [ + { id: "session.alpha", title: "Alpha", keybind: "mod+a" }, + { id: "session.beta", title: "Beta", keybind: "mod+b" }, + ], + options: [], + keybinds: (enabled) => suppression.push(enabled), + }, + settings: { + current: { keybinds: overrides }, + keybinds: { + get: (id) => overrides[id], + set: (id, value) => { + overrides[id] = value + changes.push([id, value]) + }, + resetAll: () => { + resets++ + }, + }, + }, + notify: (toast) => notifications.push(toast), + }, + { + locale: () => "en", + t: (key, params) => { + if (params) return `${key}:${Object.values(params).join("|")}` + if (key === "common.key.alt") return "Alt" + return String(key) + }, + }, + ) + return dispose + }) + + return { + controller: controller!, + changes, + suppression, + notifications, + resets: () => resets, + dispose, + } +} + +function modKey(key: string) { + const mac = /(Mac|iPod|iPhone|iPad)/.test(navigator.platform) + return new KeyboardEvent("keydown", { key, ctrlKey: !mac, metaKey: mac, bubbles: true, cancelable: true }) +} + +describe("keybind settings controller", () => { + test("derives the catalog, effective bindings, and filtered groups", () => { + const state = setup({ "session.beta": "alt+k" }) + + expect(state.controller.catalog.title("session.alpha")).toBe("Alpha") + expect(state.controller.catalog.keybind("session.beta")).toBe("Alt+K") + expect(state.controller.catalog.filtered("alt k").get("Session")).toEqual(["session.beta"]) + expect(state.controller.settings.hasOverrides()).toBe(true) + + state.dispose() + }) + + test("captures bindings, rejects conflicts, and restores command handling", () => { + const state = setup() + + state.controller.capture.toggle("session.beta") + document.dispatchEvent(modKey("a")) + expect(state.changes).toEqual([]) + expect(state.notifications).toHaveLength(1) + expect(state.controller.capture.active()).toBe("session.beta") + + document.dispatchEvent(modKey("x")) + expect(state.changes).toEqual([["session.beta", "mod+x"]]) + expect(state.suppression).toEqual([false, true]) + expect(state.controller.capture.active()).toBeNull() + + state.controller.capture.toggle("session.alpha") + state.dispose() + expect(state.suppression).toEqual([false, true, false, true]) + document.dispatchEvent(modKey("z")) + expect(state.changes).toEqual([["session.beta", "mod+x"]]) + }) + + test("resets persisted overrides and reports success", () => { + const state = setup({ "session.alpha": "none" }) + + state.controller.settings.reset() + expect(state.resets()).toBe(1) + expect(state.notifications[0]?.title).toBe("settings.shortcuts.reset.toast.title") + + state.dispose() + }) +})