Skip to content

Commit 500f4ef

Browse files
authored
Prevent ESC key propagation in overlay components (#1213)
1 parent 933218c commit 500f4ef

9 files changed

Lines changed: 87 additions & 11 deletions

File tree

apps/code/src/renderer/features/command/components/CommandMenu.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ export function CommandMenu({ open, onOpenChange }: CommandMenuProps) {
110110
justify="center"
111111
className="fixed inset-0 z-50 bg-black/20"
112112
pt="9"
113+
data-overlay="command-menu"
113114
>
114115
<div
115116
ref={commandRef}

apps/code/src/renderer/features/command/components/FilePicker.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ export function FilePicker({
8080
align="center"
8181
sideOffset={0}
8282
onInteractOutside={() => handleOpenChange(false)}
83-
onEscapeKeyDown={(e) => e.stopPropagation()}
8483
>
8584
<Command.Root shouldFilter={false} label="File picker" key={resultsKey}>
8685
<Command.Input

apps/code/src/renderer/features/message-editor/components/MessageEditor.tsx

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,11 @@ import "./message-editor.css";
22
import type { SessionConfigOption } from "@agentclientprotocol/sdk";
33
import { BranchSelector } from "@features/git-interaction/components/BranchSelector";
44
import { useGitQueries } from "@features/git-interaction/hooks/useGitQueries";
5-
import { useSettingsDialogStore } from "@features/settings/stores/settingsDialogStore";
65
import { useConnectivity } from "@hooks/useConnectivity";
76
import { ArrowUp, Circle, Stop } from "@phosphor-icons/react";
87
import { Flex, IconButton, Text, Tooltip } from "@radix-ui/themes";
9-
import { useCommandMenuStore } from "@stores/commandMenuStore";
10-
import { useShortcutsSheetStore } from "@stores/shortcutsSheetStore";
118
import { EditorContent } from "@tiptap/react";
9+
import { hasOpenOverlay } from "@utils/overlay";
1210
import { forwardRef, useEffect, useImperativeHandle } from "react";
1311
import { useHotkeys } from "react-hotkeys-hook";
1412
import { useDraftStore } from "../stores/draftStore";
@@ -148,11 +146,6 @@ export const MessageEditor = forwardRef<EditorHandle, MessageEditorProps>(
148146
const cloudBranch = context?.cloudBranch;
149147
const cloudDiffStats = context?.cloudDiffStats;
150148
const isSubmitDisabled = disabled || !isOnline;
151-
const isSettingsOpen = useSettingsDialogStore((s) => s.isOpen);
152-
const isCommandMenuOpen = useCommandMenuStore((s) => s.isOpen);
153-
const isShortcutsSheetOpen = useShortcutsSheetStore((s) => s.isOpen);
154-
const hasOverlay =
155-
isSettingsOpen || isCommandMenuOpen || isShortcutsSheetOpen;
156149

157150
const {
158151
editor,
@@ -223,6 +216,7 @@ export const MessageEditor = forwardRef<EditorHandle, MessageEditorProps>(
223216
useHotkeys(
224217
"escape",
225218
(e) => {
219+
if (hasOpenOverlay()) return;
226220
if (isLoading && onCancel) {
227221
e.preventDefault();
228222
onCancel();
@@ -231,9 +225,9 @@ export const MessageEditor = forwardRef<EditorHandle, MessageEditorProps>(
231225
{
232226
enableOnFormTags: true,
233227
enableOnContentEditable: true,
234-
enabled: isLoading && !!onCancel && !hasOverlay,
228+
enabled: isLoading && !!onCancel,
235229
},
236-
[isLoading, onCancel, hasOverlay],
230+
[isLoading, onCancel],
237231
);
238232

239233
const handleContainerClick = (e: React.MouseEvent) => {

apps/code/src/renderer/features/message-editor/tiptap/CommandMention.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ function createSuggestion(
4848
trigger: "manual",
4949
placement: "top-start",
5050
offset: [0, 12],
51+
duration: 0,
5152
});
5253
},
5354

@@ -66,6 +67,7 @@ function createSuggestion(
6667

6768
onKeyDown: (props) => {
6869
if (props.event.key === "Escape") {
70+
props.event.stopPropagation();
6971
popup?.hide();
7072
return true;
7173
}

apps/code/src/renderer/features/message-editor/tiptap/FileMention.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ function createSuggestion(
5151
trigger: "manual",
5252
placement: "top-start",
5353
offset: [0, 12],
54+
duration: 0,
5455
});
5556
},
5657

@@ -70,6 +71,7 @@ function createSuggestion(
7071

7172
onKeyDown: (props) => {
7273
if (props.event.key === "Escape") {
74+
props.event.stopPropagation();
7375
popup?.hide();
7476
return true;
7577
}

apps/code/src/renderer/features/settings/components/SettingsDialog.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ export function SettingsDialog() {
125125
<div
126126
className="fixed inset-0 z-[100] flex"
127127
style={{ backgroundColor: "var(--color-background)" }}
128+
data-overlay="settings"
128129
>
129130
<div className="flex h-full w-[256px] shrink-0 flex-col border-gray-6 border-r pt-8">
130131
<button

apps/code/src/renderer/hooks/useBlurOnEscape.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
import { SHORTCUTS } from "@renderer/constants/keyboard-shortcuts";
2+
import { hasOpenOverlay } from "@utils/overlay";
23
import { useHotkeys } from "react-hotkeys-hook";
34

45
export function useBlurOnEscape() {
56
useHotkeys(
67
SHORTCUTS.BLUR,
78
() => {
9+
if (hasOpenOverlay()) return;
810
if (document.activeElement instanceof HTMLElement) {
911
document.activeElement.blur();
1012
}
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { afterEach, describe, expect, it } from "vitest";
2+
import { hasOpenOverlay } from "./overlay";
3+
4+
describe("hasOpenOverlay", () => {
5+
let element: HTMLElement;
6+
7+
afterEach(() => {
8+
element?.remove();
9+
});
10+
11+
function addElement(tag: string, attrs: Record<string, string> = {}): void {
12+
element = document.createElement(tag);
13+
for (const [key, value] of Object.entries(attrs)) {
14+
element.setAttribute(key, value);
15+
}
16+
document.body.appendChild(element);
17+
}
18+
19+
it("returns false when no overlays exist", () => {
20+
expect(hasOpenOverlay()).toBe(false);
21+
});
22+
23+
it("detects role=dialog", () => {
24+
addElement("div", { role: "dialog" });
25+
expect(hasOpenOverlay()).toBe(true);
26+
});
27+
28+
it("detects role=alertdialog", () => {
29+
addElement("div", { role: "alertdialog" });
30+
expect(hasOpenOverlay()).toBe(true);
31+
});
32+
33+
it("detects role=menu", () => {
34+
addElement("div", { role: "menu" });
35+
expect(hasOpenOverlay()).toBe(true);
36+
});
37+
38+
it("detects data-radix-popper-content-wrapper", () => {
39+
addElement("div", { "data-radix-popper-content-wrapper": "" });
40+
expect(hasOpenOverlay()).toBe(true);
41+
});
42+
43+
it("detects data-overlay", () => {
44+
addElement("div", { "data-overlay": "command-menu" });
45+
expect(hasOpenOverlay()).toBe(true);
46+
});
47+
48+
it("does not false-positive on role=listbox (inline autocomplete)", () => {
49+
addElement("div", { role: "listbox" });
50+
expect(hasOpenOverlay()).toBe(false);
51+
});
52+
53+
it("does not false-positive on role=tooltip", () => {
54+
addElement("div", { role: "tooltip" });
55+
expect(hasOpenOverlay()).toBe(false);
56+
});
57+
58+
it("returns false after overlay is removed", () => {
59+
addElement("div", { role: "dialog" });
60+
expect(hasOpenOverlay()).toBe(true);
61+
element.remove();
62+
expect(hasOpenOverlay()).toBe(false);
63+
});
64+
});
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
const OVERLAY_SELECTORS = [
2+
"[role='dialog']",
3+
"[role='alertdialog']",
4+
"[role='menu']",
5+
"[data-radix-popper-content-wrapper]",
6+
"[data-overlay]",
7+
].join(",");
8+
9+
export function hasOpenOverlay(): boolean {
10+
return document.querySelector(OVERLAY_SELECTORS) !== null;
11+
}

0 commit comments

Comments
 (0)