Guard shortcuts during text composition

[DhruvShah-Dev]

Summary

• Add a shared guard for IME/dead-key keyboard events.
• Skip global shortcut handling while text composition is active.
• Skip Monaco shortcut interception during composition so typed characters are left to the editor/input.

Why

Issue #655 reports intermittent focus/selection disruption while typing, possibly around alternate-character input. Composition/dead-key events can look like shortcut keydown events before the final character is committed, so shortcut handlers should not consume them.

Fixes #655.

Validation

• pnpm.cmd exec vitest run tests/hooks/useGlobalShortcuts.test.ts tests/components/ui/SqlEditorWrapper.test.tsx
• pnpm.cmd typecheck
• pnpm.cmd lint

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)

• src/utils/keyboardEvents.ts
• src/hooks/useGlobalShortcuts.ts
• src/components/ui/SqlEditorWrapper.tsx
• tests/components/ui/SqlEditorWrapper.test.tsx
• tests/hooks/useGlobalShortcuts.test.ts

[debba]

Pulled the branch and went through this locally. Nice clean fix.

I dug a bit into why composition events were matching shortcuts in the first place: matchesEvent compares event.key plus exact modifier state, and on Linux a dead key or compose sequence can deliver a keydown carrying the final character together with spurious modifiers (AltGr shows up as Ctrl+Alt on some layouts). That also lets the TYPING_SAFE shortcuts fire inside inputs, which lines up exactly with the "selects what I just typed" behavior from #655. So guarding both handlers at the top is the right call, and the five signals you check are the standard set for this.

What I ran on the branch:

• the two touched test files: 23/23 green, including the three new cases
• pnpm typecheck clean, pnpm lint only shows the pre-existing ThemeProvider warning

One small thing before merge: our testing conventions want every exported util in src/utils/ to have its own test file under tests/utils/. The new helper is only covered indirectly through the hook and component tests. Could you add tests/utils/keyboardEvents.test.ts? This passes locally and covers all five branches plus the negative case:

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);
    });
  });
});

Approving since that's just a convention nit. I'll still give this a spin with a dead-key layout before cutting the next release. Thanks for the fix!