Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions web/src/hooks/useCommandPaletteHotkey.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,44 @@ describe("useCommandPaletteHotkey", () => {
expect(document.activeElement).toBe(input);
}

it("claims the chord in the capture phase, ahead of host-page listeners", () => {
// The desktop shell renders the embed build over a CSS-hidden host page
// with its own ⌘K listener; the chord must reach us, not them. Dispatch
// from a deep node so the event actually propagates (dispatching on
// window would never reach document listeners regardless).
const onToggle = vi.fn();
const hostListener = vi.fn();
renderHook(() => useCommandPaletteHotkey(onToggle));
document.addEventListener("keydown", hostListener);
const deep = document.createElement("div");
document.body.appendChild(deep);

deep.dispatchEvent(
new KeyboardEvent("keydown", { key: "k", metaKey: true, bubbles: true, cancelable: true }),
);

expect(onToggle).toHaveBeenCalledTimes(1);
expect(hostListener).not.toHaveBeenCalled();
document.removeEventListener("keydown", hostListener);
});

it("lets the chord through to host-page listeners when a focused surface owns it", () => {
const onToggle = vi.fn();
const hostListener = vi.fn();
renderHook(() => useCommandPaletteHotkey(onToggle));
document.addEventListener("keydown", hostListener);
focusInside("monaco-editor");

// Dispatch from the focused input so the event propagates up to document.
document.activeElement?.dispatchEvent(
new KeyboardEvent("keydown", { key: "k", metaKey: true, bubbles: true, cancelable: true }),
);

expect(onToggle).not.toHaveBeenCalled();
expect(hostListener).toHaveBeenCalledTimes(1);
document.removeEventListener("keydown", hostListener);
});

it("bails on Ctrl+K in a terminal — xterm sends it to the PTY as ^K", () => {
const onToggle = vi.fn();
renderHook(() => useCommandPaletteHotkey(onToggle));
Expand Down
13 changes: 9 additions & 4 deletions web/src/hooks/useCommandPaletteHotkey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ function focusOwnsHotkey(e: globalThis.KeyboardEvent): boolean {
* Bind ⌘/Ctrl+K to toggle the command palette. Bind ONCE.
*
* @param onToggle Flip the palette open/closed.
* @param enabled Pass `false` to disable the hotkey (e.g. embedded mode, where
* ⌘K belongs to the host page). Defaults to enabled.
* @param enabled Pass `false` to disable the hotkey (e.g. embedded in a real
* host page, where ⌘K belongs to the host). Defaults to enabled.
*/
export function useCommandPaletteHotkey(onToggle: () => void, enabled = true): void {
// Held in a ref so the bound handler always calls the latest closure without
Expand All @@ -71,7 +71,12 @@ export function useCommandPaletteHotkey(onToggle: () => void, enabled = true): v
e.stopPropagation();
latest.current();
};
window.addEventListener("keydown", handler);
return () => window.removeEventListener("keydown", handler);
// Capture phase: the desktop shell renders the embed build over a
// CSS-hidden host page whose own ⌘K listener would otherwise also fire.
// Binding at the propagation root lets stopPropagation keep the chord
// from reaching those listeners; focusOwnsHotkey bails BEFORE stopping
// propagation, so Monaco/terminal chords still flow to their surfaces.
window.addEventListener("keydown", handler, true);
return () => window.removeEventListener("keydown", handler, true);
}, [enabled]);
}
7 changes: 4 additions & 3 deletions web/src/shell/AppShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1304,11 +1304,12 @@ export function AppShell() {
onToggleRight: toggleRightPanel,
});

// ⌘K (Ctrl+K) toggles the command palette. Disabled embedded, where ⌘K is the
// host page's. Bound here where the palette's open-state lives.
// ⌘K (Ctrl+K) toggles the command palette. Bound capture-phase, so in the
// embedded build we claim the chord ahead of any host-page ⌘K listener.
// Bound here where the palette's open-state lives.
const [commandPaletteOpen, setCommandPaletteOpen] = useState(false);
const isEmbedded = useIsEmbedded();
useCommandPaletteHotkey(() => setCommandPaletteOpen((prev) => !prev), !isEmbedded);
useCommandPaletteHotkey(() => setCommandPaletteOpen((prev) => !prev));
useNewSessionHotkey(!isEmbedded);

// Mobile back button: close the open file and return to the files/changes
Expand Down
Loading