mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-23 10:08:38 +02:00
feat(shortcuts): make browser-reserved accelerators recordable on desktop (#5327)
* feat(shortcuts): make browser-reserved accelerators recordable on desktop Cmd/Ctrl+P (and L/T/N/D/U) were rejected by the shortcut recorder on every platform because a browser tab cannot reliably own them. The Electron renderer receives these as plain keydowns — neither Electron's default menu nor the desktop shell binds any of them — so the reservation now only applies to the web runtime. Adds a ShortcutRuntime dimension (configured by CoreProvider from the client identity, with a preload-global fallback that is already correct at module-eval time so store hydration sanitizes with the right runtime). App-owned accelerators (W/R/Q, editing keys, zoom row) stay reserved everywhere. Closes MUL-4457 Co-authored-by: multica-agent <github@multica.ai> * fix(shortcuts): limit desktop unlock to bare primary browser accelerators Review follow-up (MUL-4457): the desktop skip matched primary+key with any extra modifiers, which would also unreserve OS-owned combos such as Option+Cmd+D (macOS Dock toggle) and Ctrl+Alt+T (Linux terminal). Only the bare primary chord is now recordable on desktop; every extra-modifier variant keeps the historical reservation on both runtimes. Adds regression tests for the OS combos. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: Lambda <lambda@multica.ai> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -18,7 +18,10 @@ import { defaultStorage } from "./storage";
|
||||
import { AuthInitializer } from "./auth-initializer";
|
||||
import type { CoreProviderProps, ClientIdentity } from "./types";
|
||||
import type { StorageAdapter } from "../types/storage";
|
||||
import { configureShortcutPlatform } from "../shortcuts/platform";
|
||||
import {
|
||||
configureShortcutPlatform,
|
||||
configureShortcutRuntime,
|
||||
} from "../shortcuts/platform";
|
||||
|
||||
// Module-level singletons — created once at first render, never recreated.
|
||||
// Vite HMR preserves module-level state, so these survive hot reloads.
|
||||
@@ -43,6 +46,11 @@ function initCore(
|
||||
? identity.os
|
||||
: null,
|
||||
);
|
||||
// Authoritative override; before this runs (module-eval store hydration)
|
||||
// detectShortcutRuntime() reads the preload globals and already agrees.
|
||||
configureShortcutRuntime(
|
||||
identity?.platform === "desktop" ? "desktop" : null,
|
||||
);
|
||||
|
||||
const api = new ApiClient(apiBaseUrl, {
|
||||
logger: createLogger("api"),
|
||||
|
||||
@@ -9,7 +9,13 @@ import {
|
||||
shortcutFromEvent,
|
||||
shortcutMatchesEvent,
|
||||
} from "./definitions";
|
||||
import { configureShortcutPlatform, detectShortcutPlatform } from "./platform";
|
||||
import {
|
||||
configureShortcutPlatform,
|
||||
configureShortcutRuntime,
|
||||
detectShortcutPlatform,
|
||||
detectShortcutRuntime,
|
||||
getShortcutRuntime,
|
||||
} from "./platform";
|
||||
|
||||
function keyEvent(
|
||||
key: string,
|
||||
@@ -27,6 +33,7 @@ function keyEvent(
|
||||
|
||||
afterEach(() => {
|
||||
configureShortcutPlatform(null);
|
||||
configureShortcutRuntime(null);
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
@@ -128,6 +135,83 @@ describe("keyboard shortcut definitions", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("reserves browser-owned accelerators on web but frees the bare chords on desktop", () => {
|
||||
for (const key of ["P", "L", "T", "N", "D", "U"]) {
|
||||
const chord = createShortcutChord(key, { primary: true });
|
||||
expect(isReservedShortcut(chord, "macos", "web")).toBe(true);
|
||||
expect(isReservedShortcut(chord, "windows", "web")).toBe(true);
|
||||
expect(isReservedShortcut(chord, "macos", "desktop")).toBe(false);
|
||||
expect(isReservedShortcut(chord, "windows", "desktop")).toBe(false);
|
||||
// Only the bare primary chord opens up — extra modifiers keep the
|
||||
// historical reservation on every runtime.
|
||||
for (const extra of [{ shift: true }, { alt: true }, { control: true }]) {
|
||||
const variant = createShortcutChord(key, { primary: true, ...extra });
|
||||
expect(isReservedShortcut(variant, "macos", "desktop")).toBe(true);
|
||||
expect(isReservedShortcut(variant, "macos", "web")).toBe(true);
|
||||
}
|
||||
}
|
||||
// OS-owned combos that motivated the narrowing must stay reserved on
|
||||
// desktop: Option+Cmd+D toggles the macOS Dock, Ctrl+Alt+T opens a
|
||||
// terminal on common Linux desktops.
|
||||
expect(
|
||||
isReservedShortcut(
|
||||
createShortcutChord("D", { primary: true, alt: true }),
|
||||
"macos",
|
||||
"desktop",
|
||||
),
|
||||
).toBe(true);
|
||||
expect(
|
||||
isReservedShortcut(
|
||||
createShortcutChord("T", { primary: true, alt: true }),
|
||||
"linux",
|
||||
"desktop",
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("keeps app-owned and editing accelerators reserved on desktop", () => {
|
||||
for (const key of ["W", "R", "Q", "A", "C", "V", "X", "Y", "Z", "0", "Minus", "Plus"]) {
|
||||
expect(
|
||||
isReservedShortcut(createShortcutChord(key, { primary: true }), "macos", "desktop"),
|
||||
).toBe(true);
|
||||
}
|
||||
expect(isReservedShortcut(createShortcutChord("F5"), "windows", "desktop")).toBe(true);
|
||||
expect(
|
||||
isReservedShortcut(createShortcutChord("Space", { primary: true }), "macos", "desktop"),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("allows recording bare Cmd/Ctrl+P for an action on desktop only", () => {
|
||||
const cmdP = createShortcutChord("P", { primary: true });
|
||||
expect(isShortcutAllowedForAction("openSearch", cmdP, "macos", "desktop")).toBe(true);
|
||||
expect(isShortcutAllowedForAction("openSearch", cmdP, "macos", "web")).toBe(false);
|
||||
expect(isShortcutAllowedForAction("goIssues", cmdP, "windows", "desktop")).toBe(true);
|
||||
// Modifier variants keep the historical reservation even on desktop.
|
||||
expect(
|
||||
isShortcutAllowedForAction(
|
||||
"openSearch",
|
||||
createShortcutChord("P", { primary: true, shift: true }),
|
||||
"macos",
|
||||
"desktop",
|
||||
),
|
||||
).toBe(false);
|
||||
// Send stays locked to Enter / Mod+Enter regardless of runtime.
|
||||
expect(isShortcutAllowedForAction("send", cmdP, "macos", "desktop")).toBe(false);
|
||||
});
|
||||
|
||||
it("detects the desktop runtime from preload globals with a configure override", () => {
|
||||
expect(detectShortcutRuntime()).toBe("web");
|
||||
vi.stubGlobal("window", { desktopAPI: {} });
|
||||
expect(detectShortcutRuntime()).toBe("desktop");
|
||||
vi.stubGlobal("window", { electron: {} });
|
||||
expect(detectShortcutRuntime()).toBe("desktop");
|
||||
vi.stubGlobal("window", {});
|
||||
expect(detectShortcutRuntime()).toBe("web");
|
||||
expect(getShortcutRuntime()).toBe("web");
|
||||
configureShortcutRuntime("desktop");
|
||||
expect(getShortcutRuntime()).toBe("desktop");
|
||||
});
|
||||
|
||||
it("rejects modifier-only, composition-only, and unidentified key events", () => {
|
||||
for (const key of ["Fn", "CapsLock", "Dead", "Process", "Unidentified"]) {
|
||||
expect(shortcutFromEvent(keyEvent(key), "macos")).toBeNull();
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import {
|
||||
getShortcutPlatform,
|
||||
getShortcutRuntime,
|
||||
type ShortcutPlatform,
|
||||
type ShortcutRuntime,
|
||||
} from "./platform";
|
||||
|
||||
export type ShortcutActionId =
|
||||
@@ -236,21 +238,42 @@ export function isEditableShortcutTarget(target: EventTarget | null): boolean {
|
||||
}
|
||||
|
||||
const PRIMARY_RESERVED_KEYS = new Set([
|
||||
// Window/browser operations.
|
||||
"W", "R", "L", "T", "N", "P", "Q", "D", "U",
|
||||
// Window operations the app itself owns on every runtime: W closes the
|
||||
// tab, R/F5 is the reload guard, Q quits.
|
||||
"W", "R", "Q",
|
||||
// Fundamental editing operations should never become product actions.
|
||||
"A", "C", "V", "X", "Y", "Z",
|
||||
// Zoom accelerators: fixed app shortcuts on desktop, browser zoom on web.
|
||||
"Equals", "Plus", "Minus", "Underscore", "0",
|
||||
]);
|
||||
|
||||
/** Browser/window/OS accelerators that cannot be reliably owned by the app. */
|
||||
// Accelerators owned by the browser UI around a tab: print, address bar,
|
||||
// new tab/window, bookmark, view source. A web page cannot reliably own
|
||||
// them, but the Electron renderer receives the bare primary chords as plain
|
||||
// keydowns — neither Electron's default menu nor the desktop shell binds
|
||||
// any of them — so exactly those are recordable on desktop (MUL-4457).
|
||||
// Variants with extra modifiers stay reserved on both runtimes: several
|
||||
// belong to the OS or window manager (Option+Cmd+D toggles the macOS Dock,
|
||||
// Ctrl+Alt+T opens a terminal on common Linux desktops), which even the
|
||||
// desktop app cannot own.
|
||||
const BROWSER_ONLY_PRIMARY_RESERVED_KEYS = new Set([
|
||||
"P", "L", "T", "N", "D", "U",
|
||||
]);
|
||||
|
||||
/** Browser/window/OS accelerators the app cannot reliably own in `runtime`. */
|
||||
export function isReservedShortcut(
|
||||
shortcut: ShortcutChord,
|
||||
platform: ShortcutPlatform = getShortcutPlatform(),
|
||||
runtime: ShortcutRuntime = getShortcutRuntime(),
|
||||
): boolean {
|
||||
const { modifiers, key } = shortcut;
|
||||
if (key === "F5") return true;
|
||||
if (modifiers.primary && PRIMARY_RESERVED_KEYS.has(key)) return true;
|
||||
if (modifiers.primary && BROWSER_ONLY_PRIMARY_RESERVED_KEYS.has(key)) {
|
||||
const barePrimary =
|
||||
!modifiers.control && !modifiers.meta && !modifiers.alt && !modifiers.shift;
|
||||
if (runtime !== "desktop" || !barePrimary) return true;
|
||||
}
|
||||
|
||||
if (platform === "macos") {
|
||||
if (modifiers.primary && (key === "Space" || key === "Tab" || key === "M" || key === "H")) return true;
|
||||
@@ -292,9 +315,10 @@ export function isShortcutAllowedForAction(
|
||||
actionId: ShortcutActionId,
|
||||
shortcut: ShortcutChord,
|
||||
platform: ShortcutPlatform = getShortcutPlatform(),
|
||||
runtime: ShortcutRuntime = getShortcutRuntime(),
|
||||
): boolean {
|
||||
if (!isShortcutChordActionable(shortcut)) return false;
|
||||
if (isReservedShortcut(shortcut, platform)) return false;
|
||||
if (isReservedShortcut(shortcut, platform, runtime)) return false;
|
||||
// Tab is focus navigation (and Ctrl+Tab is browser tab navigation) on every
|
||||
// supported platform. It is never a dependable product-level binding.
|
||||
if (shortcut.key === "Tab") return false;
|
||||
|
||||
@@ -18,9 +18,13 @@ export {
|
||||
} from "./definitions";
|
||||
export {
|
||||
configureShortcutPlatform,
|
||||
configureShortcutRuntime,
|
||||
detectShortcutPlatform,
|
||||
detectShortcutRuntime,
|
||||
getShortcutPlatform,
|
||||
getShortcutRuntime,
|
||||
type ShortcutPlatform,
|
||||
type ShortcutRuntime,
|
||||
} from "./platform";
|
||||
export {
|
||||
useShortcutStore,
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
export type ShortcutPlatform = "macos" | "windows" | "linux" | "unknown";
|
||||
|
||||
/** Where shortcut handling runs: a browser tab or the Electron renderer. */
|
||||
export type ShortcutRuntime = "web" | "desktop";
|
||||
|
||||
let configuredPlatform: ShortcutPlatform | null = null;
|
||||
let configuredRuntime: ShortcutRuntime | null = null;
|
||||
|
||||
function normalizePlatform(value: string): ShortcutPlatform {
|
||||
const platform = value.toLowerCase();
|
||||
@@ -37,3 +41,35 @@ export function configureShortcutPlatform(
|
||||
export function getShortcutPlatform(): ShortcutPlatform {
|
||||
return configuredPlatform ?? detectShortcutPlatform();
|
||||
}
|
||||
|
||||
/**
|
||||
* Environment fallback, and unlike the OS it must be trustworthy at
|
||||
* module-evaluation time: the shortcut store hydrates (and sanitizes
|
||||
* persisted overrides) when its module first loads, before CoreProvider
|
||||
* gets a chance to configure anything. That is safe because the desktop
|
||||
* preload exposes its bridge globals before any renderer script runs.
|
||||
* Same signals as `detectClientType` in ../analytics — not imported so
|
||||
* this module stays dependency-free.
|
||||
*/
|
||||
export function detectShortcutRuntime(): ShortcutRuntime {
|
||||
if (typeof window === "undefined") return "web";
|
||||
const w = window as unknown as { electron?: unknown; desktopAPI?: unknown };
|
||||
if (w.electron || w.desktopAPI) return "desktop";
|
||||
if (
|
||||
typeof navigator !== "undefined" &&
|
||||
/Electron/i.test(navigator.userAgent)
|
||||
) {
|
||||
return "desktop";
|
||||
}
|
||||
return "web";
|
||||
}
|
||||
|
||||
export function configureShortcutRuntime(
|
||||
runtime: ShortcutRuntime | null | undefined,
|
||||
): void {
|
||||
configuredRuntime = runtime ?? null;
|
||||
}
|
||||
|
||||
export function getShortcutRuntime(): ShortcutRuntime {
|
||||
return configuredRuntime ?? detectShortcutRuntime();
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { createShortcutChord } from "./definitions";
|
||||
import { configureShortcutRuntime } from "./platform";
|
||||
import {
|
||||
findShortcutConflict,
|
||||
getShortcut,
|
||||
@@ -10,6 +11,7 @@ import {
|
||||
|
||||
afterEach(() => {
|
||||
useShortcutStore.getState().resetAll();
|
||||
configureShortcutRuntime(null);
|
||||
});
|
||||
|
||||
describe("shortcut store", () => {
|
||||
@@ -68,6 +70,21 @@ describe("shortcut store", () => {
|
||||
).toEqual({});
|
||||
});
|
||||
|
||||
it("keeps browser-only bindings when the runtime is desktop", () => {
|
||||
const cmdP = createShortcutChord("P", { primary: true });
|
||||
|
||||
configureShortcutRuntime("desktop");
|
||||
expect(sanitizeShortcutOverrides({ openSearch: cmdP })).toEqual({
|
||||
openSearch: cmdP,
|
||||
});
|
||||
useShortcutStore.getState().setShortcut("openSearch", cmdP);
|
||||
expect(getShortcut("openSearch")).toEqual(cmdP);
|
||||
|
||||
// The same persisted value hydrating in a browser falls back to default.
|
||||
configureShortcutRuntime("web");
|
||||
expect(sanitizeShortcutOverrides({ openSearch: cmdP })).toEqual({});
|
||||
});
|
||||
|
||||
it("finds conflicts against defaults and overrides", () => {
|
||||
expect(
|
||||
findShortcutConflict(
|
||||
|
||||
Reference in New Issue
Block a user