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
3 changes: 3 additions & 0 deletions src/components/ui/SqlEditorWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
type Statement,
} from "../../utils/sqlSplitter";
import { formatSql } from "../../utils/sqlFormat";
import { isTextCompositionKeyEvent } from "../../utils/keyboardEvents";
import type { SqlDialect } from "../../utils/sql";
import type { RunContext } from "../../utils/runTarget";
import {
Expand Down Expand Up @@ -403,6 +404,8 @@ const SqlEditorInternal = ({
// Monaco binds Ctrl+Shift+A to block comments on Linux. Handle the
// user-configurable palette shortcut before Monaco consumes it.
editor.onKeyDown((e) => {
if (isTextCompositionKeyEvent(e.browserEvent)) return;

const togglePalette = togglePaletteRef.current;
if (
togglePalette &&
Expand Down
3 changes: 3 additions & 0 deletions src/hooks/useGlobalShortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { useNavigate } from "react-router-dom";
import { useCommandPaletteDispatch } from "./useCommandPalette";
import { useConnectionManager } from "./useConnectionManager";
import { useKeybindings } from "./useKeybindings";
import { isTextCompositionKeyEvent } from "../utils/keyboardEvents";

/** Shortcuts that must still fire while the user is typing in a field. */
const TYPING_SAFE_SHORTCUTS = [
Expand All @@ -24,6 +25,8 @@ export function useGlobalShortcuts() {

useEffect(() => {
const handleKeyDown = (e: KeyboardEvent) => {
if (isTextCompositionKeyEvent(e)) return;

const target = e.target as HTMLElement;
const isTypingTarget =
target.tagName === "INPUT" ||
Expand Down
9 changes: 9 additions & 0 deletions src/utils/keyboardEvents.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export function isTextCompositionKeyEvent(event: KeyboardEvent): boolean {
return (
event.isComposing ||
event.key === "Dead" ||
event.key === "Process" ||
event.key === "Unidentified" ||
event.keyCode === 229
);
}
32 changes: 32 additions & 0 deletions tests/components/ui/SqlEditorWrapper.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,38 @@ describe('SqlEditorWrapper', () => {
expect(trigger).not.toHaveBeenCalled();
});

it('does not intercept composing editor key events as palette shortcuts', () => {
matchesShortcutMock.mockImplementation(
(_event, id) => id === 'command_palette_actions',
);
render(
<SqlEditorWrapper
initialValue=""
onChange={mockOnChange}
onRun={mockOnRun}
editorKey="palette-composition"
/>,
{ wrapper }
);
const { keyDownHandlers } = mountCapturedEditor();
const event = {
browserEvent: new KeyboardEvent('keydown', {
key: 'a',
ctrlKey: true,
shiftKey: true,
isComposing: true,
}),
preventDefault: vi.fn(),
stopPropagation: vi.fn(),
};

keyDownHandlers[0](event);

expect(event.preventDefault).not.toHaveBeenCalled();
expect(event.stopPropagation).not.toHaveBeenCalled();
expect(togglePaletteMock).not.toHaveBeenCalled();
});

it('renders without a command palette provider and leaves its shortcut to Monaco', () => {
matchesShortcutMock.mockImplementation(
(_event, id) => id === 'command_palette_actions',
Expand Down
33 changes: 33 additions & 0 deletions tests/hooks/useGlobalShortcuts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,39 @@ describe("useGlobalShortcuts", () => {
input.remove();
});

it("ignores composing key events while focus is inside an input", () => {
renderHook(() => useGlobalShortcuts());
const input = document.createElement("input");
document.body.appendChild(input);
input.focus();

fireEvent.keyDown(input, {
key: "a",
metaKey: true,
shiftKey: true,
isComposing: true,
});

expect(togglePaletteMock).not.toHaveBeenCalled();
input.remove();
});

it("ignores dead-key events while focus is inside an input", () => {
renderHook(() => useGlobalShortcuts());
const input = document.createElement("input");
document.body.appendChild(input);
input.focus();

fireEvent.keyDown(input, {
key: "Dead",
metaKey: true,
shiftKey: true,
});

expect(togglePaletteMock).not.toHaveBeenCalled();
input.remove();
});

it("should open object search through the shared palette controller", () => {
activeShortcutId = "quick_navigator";
renderHook(() => useGlobalShortcuts());
Expand Down
40 changes: 40 additions & 0 deletions tests/utils/keyboardEvents.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { describe, expect, it } from "vitest";
import { isTextCompositionKeyEvent } from "../../src/utils/keyboardEvents";

describe("keyboardEvents", () => {
describe("isTextCompositionKeyEvent", () => {
it("detects active IME composition", () => {
const event = new KeyboardEvent("keydown", { key: "a", isComposing: true });
expect(isTextCompositionKeyEvent(event)).toBe(true);
});

it("detects dead keys", () => {
const event = new KeyboardEvent("keydown", { key: "Dead" });
expect(isTextCompositionKeyEvent(event)).toBe(true);
});

it("detects IME process keys", () => {
const event = new KeyboardEvent("keydown", { key: "Process" });
expect(isTextCompositionKeyEvent(event)).toBe(true);
});

it("detects unidentified keys", () => {
const event = new KeyboardEvent("keydown", { key: "Unidentified" });
expect(isTextCompositionKeyEvent(event)).toBe(true);
});

it("detects legacy IME keyCode 229", () => {
const event = new KeyboardEvent("keydown", { key: "a", keyCode: 229 } as KeyboardEventInit);
expect(isTextCompositionKeyEvent(event)).toBe(true);
});

it("lets ordinary shortcut key events through", () => {
const event = new KeyboardEvent("keydown", {
key: "p",
ctrlKey: true,
shiftKey: true,
});
expect(isTextCompositionKeyEvent(event)).toBe(false);
});
});
});