Fix shortcut capture ignoring - = ; ' keys (#2047)
* Fix shortcut capture ignoring - = ; ' keys KEY_MAP was missing Minus/Equal/Semicolon/Quote, so keyboardEventToComboString returned null for those physical keys and the capture UI silently dropped them. Add the four entries with their shifted variants. Also suppress the Electron View > Zoom accelerators (Cmd+-/=/0) while capturing, so those combos reach the renderer instead of zooming the window. Driven by the existing capturingShortcut flag via a new paseo:menu:set-capturing-shortcut IPC. * fix: address review feedback from PR #2047 Trim the capturing useEffect so it only fires when capture is active (removes the redundant on-mount false and the double-fire on true->false), and reset the main-process capturingShortcut flag on renderer reload via browser-window-created + did-finish-load so a Cmd+R mid-capture can't leave the zoom accelerators disabled. --------- Co-authored-by: Mohamed Boudra <boudra.moha@gmail.com>
This commit is contained in:
co-authored by
Mohamed Boudra
parent
9ea58aae0c
commit
c0b801b805
@@ -85,6 +85,7 @@ export interface DesktopWebUtilsBridge {
|
|||||||
|
|
||||||
export interface DesktopMenuBridge {
|
export interface DesktopMenuBridge {
|
||||||
showContextMenu?: (input?: { kind?: "terminal"; hasSelection?: boolean }) => Promise<void>;
|
showContextMenu?: (input?: { kind?: "terminal"; hasSelection?: boolean }) => Promise<void>;
|
||||||
|
setCapturingShortcut?: (capturing: boolean) => Promise<void>;
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface DesktopWindowControlsOverlayUpdate {
|
export interface DesktopWindowControlsOverlayUpdate {
|
||||||
|
|||||||
@@ -0,0 +1,70 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import {
|
||||||
|
keyComboToString,
|
||||||
|
keyboardEventToComboString,
|
||||||
|
parseShortcutString,
|
||||||
|
} from "./shortcut-string";
|
||||||
|
|
||||||
|
function keyboardEvent(overrides: Partial<KeyboardEvent>): KeyboardEvent {
|
||||||
|
return {
|
||||||
|
key: "",
|
||||||
|
code: "",
|
||||||
|
altKey: false,
|
||||||
|
ctrlKey: false,
|
||||||
|
metaKey: false,
|
||||||
|
shiftKey: false,
|
||||||
|
repeat: false,
|
||||||
|
...overrides,
|
||||||
|
} as KeyboardEvent;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("keyboardEventToComboString", () => {
|
||||||
|
it("captures the minus key (regression: previously returned null)", () => {
|
||||||
|
expect(keyboardEventToComboString(keyboardEvent({ key: "-", code: "Minus" }))).toBe("-");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("captures minus with modifiers", () => {
|
||||||
|
expect(
|
||||||
|
keyboardEventToComboString(keyboardEvent({ key: "-", code: "Minus", metaKey: true })),
|
||||||
|
).toBe("Cmd+-");
|
||||||
|
expect(
|
||||||
|
keyboardEventToComboString(keyboardEvent({ key: "_", code: "Minus", shiftKey: true })),
|
||||||
|
).toBe("Shift+-");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("captures equal, semicolon, and quote", () => {
|
||||||
|
expect(keyboardEventToComboString(keyboardEvent({ key: "=", code: "Equal" }))).toBe("=");
|
||||||
|
expect(keyboardEventToComboString(keyboardEvent({ key: ";", code: "Semicolon" }))).toBe(";");
|
||||||
|
expect(keyboardEventToComboString(keyboardEvent({ key: "'", code: "Quote" }))).toBe("'");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still returns null for modifier-only presses", () => {
|
||||||
|
expect(keyboardEventToComboString(keyboardEvent({ code: "ShiftLeft" }))).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("parseShortcutString round-trips punctuation keys", () => {
|
||||||
|
const cases: ReadonlyArray<[string, string, string, string]> = [
|
||||||
|
["-", "Minus", "-", "_"],
|
||||||
|
["=", "Equal", "=", "+"],
|
||||||
|
[";", "Semicolon", ";", ":"],
|
||||||
|
["'", "Quote", "'", '"'],
|
||||||
|
];
|
||||||
|
|
||||||
|
for (const [humanKey, code, key, shiftedKey] of cases) {
|
||||||
|
it(`round-trips ${humanKey}`, () => {
|
||||||
|
const combo = parseShortcutString(humanKey);
|
||||||
|
expect(combo.code).toBe(code);
|
||||||
|
expect(combo.key).toBe(key);
|
||||||
|
expect(combo.shiftedKey).toBe(shiftedKey);
|
||||||
|
expect(keyComboToString(combo)).toBe(humanKey);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it("round-trips a modifier combo with minus", () => {
|
||||||
|
const combo = parseShortcutString("Cmd+-");
|
||||||
|
expect(combo.meta).toBe(true);
|
||||||
|
expect(combo.code).toBe("Minus");
|
||||||
|
expect(keyComboToString(combo)).toBe("Cmd+-");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -42,9 +42,13 @@ KEY_MAP["9"].shiftedKey = "(";
|
|||||||
KEY_MAP["0"].shiftedKey = ")";
|
KEY_MAP["0"].shiftedKey = ")";
|
||||||
|
|
||||||
KEY_MAP["Digit"] = { code: "Digit" };
|
KEY_MAP["Digit"] = { code: "Digit" };
|
||||||
|
KEY_MAP["-"] = { code: "Minus", key: "-", shiftedKey: "_" };
|
||||||
|
KEY_MAP["="] = { code: "Equal", key: "=", shiftedKey: "+" };
|
||||||
KEY_MAP["\\"] = { code: "Backslash", key: "\\", shiftedKey: "|" };
|
KEY_MAP["\\"] = { code: "Backslash", key: "\\", shiftedKey: "|" };
|
||||||
KEY_MAP["["] = { code: "BracketLeft", key: "[", shiftedKey: "{" };
|
KEY_MAP["["] = { code: "BracketLeft", key: "[", shiftedKey: "{" };
|
||||||
KEY_MAP["]"] = { code: "BracketRight", key: "]", shiftedKey: "}" };
|
KEY_MAP["]"] = { code: "BracketRight", key: "]", shiftedKey: "}" };
|
||||||
|
KEY_MAP[";"] = { code: "Semicolon", key: ";", shiftedKey: ":" };
|
||||||
|
KEY_MAP["'"] = { code: "Quote", key: "'", shiftedKey: '"' };
|
||||||
KEY_MAP[","] = { code: "Comma", key: ",", shiftedKey: "<" };
|
KEY_MAP[","] = { code: "Comma", key: ",", shiftedKey: "<" };
|
||||||
KEY_MAP["."] = { code: "Period", key: ".", shiftedKey: ">" };
|
KEY_MAP["."] = { code: "Period", key: ".", shiftedKey: ">" };
|
||||||
KEY_MAP["`"] = { code: "Backquote", key: "`", shiftedKey: "~" };
|
KEY_MAP["`"] = { code: "Backquote", key: "`", shiftedKey: "~" };
|
||||||
|
|||||||
@@ -23,6 +23,7 @@ import { useKeyboardShortcutsStore } from "@/stores/keyboard-shortcuts-store";
|
|||||||
import { getShortcutOs } from "@/utils/shortcut-platform";
|
import { getShortcutOs } from "@/utils/shortcut-platform";
|
||||||
import { getIsElectronRuntime } from "@/constants/layout";
|
import { getIsElectronRuntime } from "@/constants/layout";
|
||||||
import { isNative } from "@/constants/platform";
|
import { isNative } from "@/constants/platform";
|
||||||
|
import { getDesktopHost } from "@/desktop/host";
|
||||||
|
|
||||||
const EMPTY_CAPTURED_COMBOS: string[] = [];
|
const EMPTY_CAPTURED_COMBOS: string[] = [];
|
||||||
|
|
||||||
@@ -169,6 +170,7 @@ export function KeyboardShortcutsSection() {
|
|||||||
const { overrides, hasOverrides, setOverride, removeOverride, resetAll } =
|
const { overrides, hasOverrides, setOverride, removeOverride, resetAll } =
|
||||||
useKeyboardShortcutOverrides();
|
useKeyboardShortcutOverrides();
|
||||||
const setCapturingShortcut = useKeyboardShortcutsStore((s) => s.setCapturingShortcut);
|
const setCapturingShortcut = useKeyboardShortcutsStore((s) => s.setCapturingShortcut);
|
||||||
|
const capturing = useKeyboardShortcutsStore((s) => s.capturingShortcut);
|
||||||
|
|
||||||
const isFocused = useIsFocused();
|
const isFocused = useIsFocused();
|
||||||
const isMac = getShortcutOs() === "mac";
|
const isMac = getShortcutOs() === "mac";
|
||||||
@@ -242,6 +244,17 @@ export function KeyboardShortcutsSection() {
|
|||||||
};
|
};
|
||||||
}, [setCapturingShortcut]);
|
}, [setCapturingShortcut]);
|
||||||
|
|
||||||
|
// Suppress desktop zoom accelerators while capturing so combos like Cmd+- are
|
||||||
|
// recorded instead of zooming the window. No-op outside Electron.
|
||||||
|
useEffect(() => {
|
||||||
|
if (isNative || !capturing) return;
|
||||||
|
const menu = getDesktopHost()?.menu;
|
||||||
|
void menu?.setCapturingShortcut?.(true);
|
||||||
|
return () => {
|
||||||
|
void menu?.setCapturingShortcut?.(false);
|
||||||
|
};
|
||||||
|
}, [capturing]);
|
||||||
|
|
||||||
const handleResetAll = useCallback(() => void resetAll(), [resetAll]);
|
const handleResetAll = useCallback(() => void resetAll(), [resetAll]);
|
||||||
const handleRemoveOverride = useCallback(
|
const handleRemoveOverride = useCallback(
|
||||||
(bindingId: string) => void removeOverride(bindingId),
|
(bindingId: string) => void removeOverride(bindingId),
|
||||||
|
|||||||
@@ -47,8 +47,10 @@ function reloadFocusedContentsOrWindow(win: BrowserWindow, options?: { ignoreCac
|
|||||||
|
|
||||||
function buildApplicationMenuTemplate(
|
function buildApplicationMenuTemplate(
|
||||||
options: ApplicationMenuOptions,
|
options: ApplicationMenuOptions,
|
||||||
|
capturing: boolean,
|
||||||
): Electron.MenuItemConstructorOptions[] {
|
): Electron.MenuItemConstructorOptions[] {
|
||||||
const isMac = process.platform === "darwin";
|
const isMac = process.platform === "darwin";
|
||||||
|
const zoomEnabled = !capturing;
|
||||||
|
|
||||||
return [
|
return [
|
||||||
...(isMac
|
...(isMac
|
||||||
@@ -99,6 +101,7 @@ function buildApplicationMenuTemplate(
|
|||||||
{
|
{
|
||||||
label: "Zoom In",
|
label: "Zoom In",
|
||||||
accelerator: "CmdOrCtrl+=",
|
accelerator: "CmdOrCtrl+=",
|
||||||
|
enabled: zoomEnabled,
|
||||||
click: withBrowserWindow((win) => {
|
click: withBrowserWindow((win) => {
|
||||||
win.webContents.setZoomLevel(win.webContents.getZoomLevel() + 0.5);
|
win.webContents.setZoomLevel(win.webContents.getZoomLevel() + 0.5);
|
||||||
}),
|
}),
|
||||||
@@ -106,6 +109,7 @@ function buildApplicationMenuTemplate(
|
|||||||
{
|
{
|
||||||
label: "Zoom Out",
|
label: "Zoom Out",
|
||||||
accelerator: "CmdOrCtrl+-",
|
accelerator: "CmdOrCtrl+-",
|
||||||
|
enabled: zoomEnabled,
|
||||||
click: withBrowserWindow((win) => {
|
click: withBrowserWindow((win) => {
|
||||||
win.webContents.setZoomLevel(win.webContents.getZoomLevel() - 0.5);
|
win.webContents.setZoomLevel(win.webContents.getZoomLevel() - 0.5);
|
||||||
}),
|
}),
|
||||||
@@ -113,6 +117,7 @@ function buildApplicationMenuTemplate(
|
|||||||
{
|
{
|
||||||
label: "Actual Size",
|
label: "Actual Size",
|
||||||
accelerator: "CmdOrCtrl+0",
|
accelerator: "CmdOrCtrl+0",
|
||||||
|
enabled: zoomEnabled,
|
||||||
click: withBrowserWindow((win) => {
|
click: withBrowserWindow((win) => {
|
||||||
win.webContents.setZoomLevel(0);
|
win.webContents.setZoomLevel(0);
|
||||||
}),
|
}),
|
||||||
@@ -150,9 +155,20 @@ function buildApplicationMenuTemplate(
|
|||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
export function setupApplicationMenu(options: ApplicationMenuOptions): void {
|
let applicationMenuOptions: ApplicationMenuOptions | null = null;
|
||||||
const menu = Menu.buildFromTemplate(buildApplicationMenuTemplate(options));
|
let capturingShortcut = false;
|
||||||
|
|
||||||
|
function rebuildApplicationMenu(): void {
|
||||||
|
if (!applicationMenuOptions) return;
|
||||||
|
const menu = Menu.buildFromTemplate(
|
||||||
|
buildApplicationMenuTemplate(applicationMenuOptions, capturingShortcut),
|
||||||
|
);
|
||||||
Menu.setApplicationMenu(menu);
|
Menu.setApplicationMenu(menu);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function setupApplicationMenu(options: ApplicationMenuOptions): void {
|
||||||
|
applicationMenuOptions = options;
|
||||||
|
rebuildApplicationMenu();
|
||||||
|
|
||||||
ipcMain.handle("paseo:menu:showContextMenu", (event, input?: ShowContextMenuInput) => {
|
ipcMain.handle("paseo:menu:showContextMenu", (event, input?: ShowContextMenuInput) => {
|
||||||
const win = BrowserWindow.fromWebContents(event.sender);
|
const win = BrowserWindow.fromWebContents(event.sender);
|
||||||
@@ -185,4 +201,23 @@ export function setupApplicationMenu(options: ApplicationMenuOptions): void {
|
|||||||
|
|
||||||
contextMenu.popup({ window: win });
|
contextMenu.popup({ window: win });
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Disable the zoom accelerators while capturing a shortcut so combos like
|
||||||
|
// Cmd+- / Cmd+= reach the renderer instead of zooming the window.
|
||||||
|
ipcMain.handle("paseo:menu:set-capturing-shortcut", (_event, capturing?: boolean) => {
|
||||||
|
capturingShortcut = capturing === true;
|
||||||
|
rebuildApplicationMenu();
|
||||||
|
});
|
||||||
|
|
||||||
|
// If the renderer reloads mid-capture (e.g. Cmd+R) the renderer-side effect
|
||||||
|
// never gets to send `false`, so reset the flag from the main process when a
|
||||||
|
// main window finishes loading. Workspace browser webviews are not
|
||||||
|
// BrowserWindows, so they don't trigger this.
|
||||||
|
app.on("browser-window-created", (_event, win) => {
|
||||||
|
win.webContents.on("did-finish-load", () => {
|
||||||
|
if (!capturingShortcut) return;
|
||||||
|
capturingShortcut = false;
|
||||||
|
rebuildApplicationMenu();
|
||||||
|
});
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -72,6 +72,8 @@ contextBridge.exposeInMainWorld("paseoDesktop", {
|
|||||||
menu: {
|
menu: {
|
||||||
showContextMenu: (input?: Record<string, unknown>) =>
|
showContextMenu: (input?: Record<string, unknown>) =>
|
||||||
ipcRenderer.invoke("paseo:menu:showContextMenu", input),
|
ipcRenderer.invoke("paseo:menu:showContextMenu", input),
|
||||||
|
setCapturingShortcut: (capturing: boolean) =>
|
||||||
|
ipcRenderer.invoke("paseo:menu:set-capturing-shortcut", capturing),
|
||||||
},
|
},
|
||||||
browser: {
|
browser: {
|
||||||
registerWorkspaceBrowser: (input: { browserId: string; workspaceId: string }) =>
|
registerWorkspaceBrowser: (input: { browserId: string; workspaceId: string }) =>
|
||||||
|
|||||||
Reference in New Issue
Block a user