From a55c17f3979e99feed8a35fbdb6e0965ed7e2950 Mon Sep 17 00:00:00 2001
From: yetone <yetoneful@gmail.com>
Date: Sat, 3 Oct 2026 20:54:05 +0800
Subject: [PATCH] =?UTF-8?q?gui:=20Cmd+,=20opens=20Settings,=20Ctrl+,=20on?=
 =?UTF-8?q?=20Windows=20and=20Linux=20(#670:=20App=E8=BF=9B=E5=85=A5?=
 =?UTF-8?q?=E8=AE=BE=E7=BD=AE=E6=B2=A1=E6=9C=89=E5=BF=AB=E6=8D=B7=E9=94=AE?=
 =?UTF-8?q?=E5=90=97=EF=BC=9Fmac=E4=B8=80=E8=88=AC=E6=98=AFcommand+?=
 =?UTF-8?q?=E9=80=97=E5=8F=B7)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

magpie had no shortcut for Settings: only the gear in the header opened it. Now Cmd+, on the Mac and Ctrl+, elsewhere opens the Settings page from any page of the window, from a text box too, as a Mac app's does; in the tray panel it opens the window on Settings, as the gear does there. Another modifier with the comma (Ctrl on the Mac, Alt, Shift) doesn't, and a browser tab (magpie web) leaves the keys to the browser. The gear's tooltip says the shortcut, "Settings (⌘,)" or "Settings (Ctrl+,)" (data-keys, kept through a language change). magpie has no app menu of its own (Wails' default Mac menu has no Settings item), so the shortcut is the page's, beside the text size's Cmd/Ctrl + − 0.

settings-shortcut.test.cjs (window, a focused text box, the panel asking for window/main?view=settings, other modifiers, the browser tab, the tooltip; en and zh) fails on the old code. go vet ./internal/..., go build -tags nogui ./... and GOOS=windows go build ./... pass; the GUI tests under WebKit pass but those that fail on main as well (33 there: cli-update, group-from-model-add, menu-scroll, old-webkit, provider-levels-scroll, redact-rule-row, routing-compact/manual/time, seg-contrast, session-identity, signin-callback, zcode-site). Chromium doesn't launch on this machine (icudtl.dat missing), so it was tried in WebKit only, not in the app's own webview.
---
 internal/gui/assets/app.js                    |  19 +++-
 internal/gui/assets/i18n.js                   |   4 +
 internal/gui/assets/index.html                |   2 +-
 internal/gui/tests/settings-shortcut.test.cjs | 103 ++++++++++++++++++
 4 files changed, 126 insertions(+), 2 deletions(-)
 create mode 100644 internal/gui/tests/settings-shortcut.test.cjs

diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js
index 6e81be83..99b89658 100644
--- a/internal/gui/assets/app.js
+++ b/internal/gui/assets/app.js
@@ -12691,6 +12691,15 @@ if (!web) addEventListener("keydown", (e) => {
   setTextSize(n);
   status(t("Text size {n}%", { n }));
 }, true);
+// Cmd+, (Ctrl+, off the Mac) opens Settings, as it does in a Mac app
+// (#670), from the window or the panel alike and from a text box too; in a
+// browser tab it is the browser's.
+if (!web) addEventListener("keydown", (e) => {
+  const mod = /^Mac/.test(navigator.platform) ? e.metaKey && !e.ctrlKey : e.ctrlKey && !e.metaKey;
+  if (!mod || e.altKey || e.shiftKey || e.repeat || (e.key !== "," && e.code !== "Comma")) return;
+  e.preventDefault();
+  openSettings();
+}, true);
 
 // applyPrefs paints and speaks as the saved settings say, costs at the
 // exchange rate given (rate, /api/state's fx) or the settings' own. A
@@ -14447,7 +14456,15 @@ function syncURL() {
   if (s !== location.search) history.replaceState(null, "", s);
 }
 if (mode === "window") for (const b of $("#nav").querySelectorAll("button")) b.onclick = () => { show(b.dataset.view); b.blur(); };
-$("#prefs").onclick = () => { if (mode === "window") show("settings"); else api("window/main?view=settings", {}); $("#prefs").blur(); };
+// openSettings: the Settings page, in the window (the panel opens it there)
+function openSettings() {
+  if (mode !== "window") { api("window/main?view=settings", {}); return; }
+  // a pick list or a menu open over the page is put away with it
+  closeProtoMenu();
+  closeAgentMenu();
+  show("settings");
+}
+$("#prefs").onclick = () => { openSettings(); $("#prefs").blur(); };
 
 $("#sync").onclick = async () => {
   const b = $("#sync");
diff --git a/internal/gui/assets/i18n.js b/internal/gui/assets/i18n.js
index b16078e6..eda776bf 100644
--- a/internal/gui/assets/i18n.js
+++ b/internal/gui/assets/i18n.js
@@ -2662,6 +2662,10 @@ function translateStatic() {
   for (const e of document.querySelectorAll("[data-tt]")) {
     if (e.dataset.enTitle === undefined) e.dataset.enTitle = e.dataset.tt || e.title;
     e.title = t(e.dataset.enTitle);
+    // data-keys: its shortcut, the Mac's before the | and the others' after
+    // (Settings' ⌘, #670); a browser tab keeps its own
+    const keys = e.dataset.keys?.split("|")[/^Mac/.test(navigator.platform) ? 0 : 1];
+    if (keys && !window.bootPrefs?.web) e.title += " (" + keys + ")";
   }
   for (const e of document.querySelectorAll("[data-tp]")) {
     if (e.dataset.enPlaceholder === undefined) e.dataset.enPlaceholder = e.placeholder;
diff --git a/internal/gui/assets/index.html b/internal/gui/assets/index.html
index f6f86dd0..8f68ac4b 100644
--- a/internal/gui/assets/index.html
+++ b/internal/gui/assets/index.html
@@ -47,7 +47,7 @@
     <button class="icon" id="open" title="Open as a window" data-tt>
       <svg viewBox="0 0 16 16" width="14" height="14"><path d="M6 3H3.5A1.5 1.5 0 0 0 2 4.5v8A1.5 1.5 0 0 0 3.5 14h8a1.5 1.5 0 0 0 1.5-1.5V10M9 2h5v5M14 2 8 8" fill="none" stroke="currentColor" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round"/></svg>
     </button>
-    <button class="icon" id="prefs" title="Settings" data-tt>
+    <button class="icon" id="prefs" title="Settings" data-tt data-keys="⌘,|Ctrl+,">
       <svg viewBox="0 0 24 24" width="14" height="14"><g fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="M12.22 2h-.44a2 2 0 0 0-2 2v.18a2 2 0 0 1-1 1.73l-.43.25a2 2 0 0 1-2 0l-.15-.08a2 2 0 0 0-2.73.73l-.22.38a2 2 0 0 0 .73 2.73l.15.1a2 2 0 0 1 1 1.72v.51a2 2 0 0 1-1 1.74l-.15.09a2 2 0 0 0-.73 2.73l.22.38a2 2 0 0 0 2.73.73l.15-.08a2 2 0 0 1 2 0l.43.25a2 2 0 0 1 1 1.73V20a2 2 0 0 0 2 2h.44a2 2 0 0 0 2-2v-.18a2 2 0 0 1 1-1.73l.43-.25a2 2 0 0 1 2 0l.15.08a2 2 0 0 0 2.73-.73l.22-.39a2 2 0 0 0-.73-2.73l-.15-.08a2 2 0 0 1-1-1.74v-.5a2 2 0 0 1 1-1.74l.15-.09a2 2 0 0 0 .73-2.73l-.22-.38a2 2 0 0 0-2.73-.73l-.15.08a2 2 0 0 1-2 0l-.43-.25a2 2 0 0 1-1-1.73V4a2 2 0 0 0-2-2z"/><circle cx="12" cy="12" r="3"/></g></svg>
     </button>
     <button class="icon winclose" id="winclose" title="Close" data-tt>
diff --git a/internal/gui/tests/settings-shortcut.test.cjs b/internal/gui/tests/settings-shortcut.test.cjs
new file mode 100644
index 00000000..19db6643
--- /dev/null
+++ b/internal/gui/tests/settings-shortcut.test.cjs
@@ -0,0 +1,103 @@
+// Run with Node's test runner and Playwright on the module path; see README.md.
+// Cmd+, (Ctrl+, off the Mac) opens Settings (#670: no shortcut for it; a
+// Mac app's is ⌘,): from the Agents page, from a text box, and in the tray
+// panel by asking for the window on its Settings page. Another modifier
+// (Ctrl on the Mac, Alt) doesn't, a browser tab (magpie web) leaves the
+// keys to the browser, and the gear's tooltip says the shortcut. English and
+// Chinese; no backend, the API is faked here.
+const assert = require("node:assert/strict");
+const fs = require("node:fs/promises");
+const path = require("node:path");
+const { test } = require("node:test");
+const { chromium, webkit } = require("playwright");
+
+const assets = path.resolve(__dirname, "../assets");
+
+function serve(lang, web, asked) {
+  const settings = {
+    theme: "light", lang, tray: "panel", currency: "usd", textSize: 100, version: "0.1.700", dir: "~/.config/magpie",
+    gateway: "http://127.0.0.1:3999", proxy: "", proxyNow: "none", proxySource: "none", redactWords: [], visionModels: [], imageGenModels: [],
+    workbuddyCheckins: [], lanURLs: [], fx: { rate: 7.2, at: new Date().toISOString(), stale: false },
+  };
+  return async (route) => {
+    const req = route.request(), url = new URL(req.url());
+    const json = (data) => route.fulfill({ json: data });
+    if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs = {lang:"${lang}",theme:"light",web:${web}};` });
+    if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" });
+    if (url.pathname.startsWith("/api/window/")) { if (url.pathname === "/api/window/main") asked.push(url.pathname + url.search); return json({}); }
+    if (url.pathname === "/api/state") return json({ agents: [], profiles: [], settings: { lang, theme: "light" } });
+    if (url.pathname === "/api/settings") return json(settings);
+    if (url.pathname === "/api/providers") return json({ providers: [], presets: [], excluded: [], gateway: { running: true, window: true } });
+    if (url.pathname === "/api/usage/quotas") return json([]);
+    if (url.pathname === "/api/plugins") return json({ plugins: [] });
+    if (url.pathname === "/api/groups") return json({ groups: [], models: [] });
+    if (url.pathname.startsWith("/api/")) return json({});
+    const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname);
+    const contentType = { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)];
+    try { await route.fulfill({ body: await fs.readFile(file), contentType }); } catch { await route.fulfill({ status: 404, body: "" }); }
+  };
+}
+
+const words = { en: "Settings", zh: "设置" };
+
+for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) {
+  for (const lang of ["en", "zh"]) {
+    test(`${engine} ${lang}: Cmd/Ctrl+, opens Settings`, async (t) => {
+      const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" }));
+      t.after(() => browser.close());
+      const open = async (url, web, asked) => {
+        const page = await (await browser.newContext({ viewport: { width: 900, height: 700 }, reducedMotion: "reduce" })).newPage();
+        page.setDefaultTimeout(5000);
+        const errors = [];
+        page.on("pageerror", (e) => errors.push(e.message));
+        await page.route("**/*", serve(lang, web, asked));
+        await page.goto("http://magpie.test" + url);
+        await page.locator("#prefs").waitFor();
+        return { page, errors };
+      };
+      const shown = (page) => page.evaluate(() => !document.querySelector("#view-settings").hidden && document.querySelector("#prefs").classList.contains("on"));
+
+      const asked = [];
+      const { page, errors } = await open("/", false, asked);
+      const mod = await page.evaluate(() => /^Mac/.test(navigator.platform) ? "Meta" : "Control");
+      assert.equal(await shown(page), false);
+      // the gear says its shortcut
+      assert.equal(await page.locator("#prefs").getAttribute("title"), words[lang] + (mod === "Meta" ? " (⌘,)" : " (Ctrl+,)"));
+      // another modifier isn't it
+      await page.keyboard.press((mod === "Meta" ? "Control" : "Alt") + "+Comma");
+      await page.keyboard.press(mod + "+Alt+Comma");
+      assert.equal(await shown(page), false);
+      await page.keyboard.press(mod + "+Comma");
+      await page.waitForFunction(() => !document.querySelector("#view-settings").hidden);
+      assert.equal(await shown(page), true);
+      assert.ok(new URL(page.url()).searchParams.get("view") === "settings", page.url());
+      // from a text box on another page too
+      await page.locator('#nav button[data-view="providers"]').click();
+      assert.equal(await shown(page), false);
+      const box = page.locator("#view-providers input:visible").first();
+      if (await box.count()) { await box.focus(); }
+      await page.keyboard.press(mod + "+Comma");
+      await page.waitForFunction(() => !document.querySelector("#view-settings").hidden);
+      assert.deepEqual(asked, []);
+      assert.deepEqual(errors, []);
+
+      // the panel asks for the window, on its Settings page
+      const panelAsked = [];
+      const panel = await open("/?mode=panel", false, panelAsked);
+      await panel.page.keyboard.press(mod + "+Comma");
+      for (let i = 0; i < 20 && !panelAsked.length; i++) await panel.page.waitForTimeout(50);
+      assert.deepEqual(panelAsked, ["/api/window/main?view=settings"]);
+      assert.deepEqual(panel.errors, []);
+
+      // a browser tab leaves the keys to the browser, and says no shortcut
+      const webAsked = [];
+      const tab = await open("/", true, webAsked);
+      assert.equal(await tab.page.locator("#prefs").getAttribute("title"), words[lang]);
+      await tab.page.keyboard.press(mod + "+Comma");
+      await tab.page.waitForTimeout(150);
+      assert.equal(await shown(tab.page), false);
+      assert.deepEqual(webAsked, []);
+      assert.deepEqual(tab.errors, []);
+    });
+  }
+}