diff --git a/packages/app/src/hooks/use-keyboard-shortcuts.ts b/packages/app/src/hooks/use-keyboard-shortcuts.ts index 4354d364ff..6f332b0a98 100644 --- a/packages/app/src/hooks/use-keyboard-shortcuts.ts +++ b/packages/app/src/hooks/use-keyboard-shortcuts.ts @@ -40,7 +40,7 @@ import { navigateToLastWorkspace, useActiveWorkspaceSelection, } from "@/stores/navigation-active-workspace-store"; -import { dispatchTopWebOverlayKeyDown, hasOpenDomPopover } from "@/lib/overlay-root"; +import { dispatchTopWebOverlayKeyDown, keyBelongsToDomPopover } from "@/lib/overlay-root"; export function useKeyboardShortcuts({ enabled, @@ -321,10 +321,7 @@ export function useKeyboardShortcuts({ return; } - // An open popover (a plugin's panel) is the top layer and closes on its - // own Escape listener, which runs after this one. Escape is not an app - // shortcut then: it must not also interrupt the running agent. - if (event.key === "Escape" && hasOpenDomPopover()) { + if (keyBelongsToDomPopover(event)) { return; } diff --git a/packages/app/src/lib/overlay-root.browser.test.tsx b/packages/app/src/lib/overlay-root.browser.test.tsx index 8cf09fcf72..5737adbd54 100644 --- a/packages/app/src/lib/overlay-root.browser.test.tsx +++ b/packages/app/src/lib/overlay-root.browser.test.tsx @@ -2,7 +2,11 @@ import React, { type RefCallback } from "react"; import { flushSync } from "react-dom"; import { createRoot, type Root } from "react-dom/client"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { hasOpenDomPopover, useWebOverlayRegistration } from "./overlay-root"; +import { + hasOpenDomPopover, + keyBelongsToDomPopover, + useWebOverlayRegistration, +} from "./overlay-root"; function nextFrame(): Promise { return new Promise((resolve) => requestAnimationFrame(() => resolve())); @@ -85,3 +89,51 @@ describe("hasOpenDomPopover", () => { expect(hasOpenDomPopover()).toBe(false); }); }); + +describe("Escape with a plugin popover open", () => { + afterEach(() => { + document.body.replaceChildren(); + }); + + it("closes the popover first, and interrupts only on the next press", () => { + // A plugin's panel: shown with the Popover API, closed by its own Escape listener. + const panel = document.createElement("div"); + panel.setAttribute("popover", "manual"); + document.addEventListener("keydown", (event) => { + if (event.key === "Escape" && panel.matches(":popover-open")) panel.hidePopover(); + }); + const composer = document.createElement("textarea"); + document.body.append(composer, panel); + // The shortcut engine: a window capture listener that interrupts on Escape. + let interrupts = 0; + const shortcuts = (event: KeyboardEvent) => { + if (keyBelongsToDomPopover(event)) return; + if (event.key === "Escape") interrupts += 1; + }; + window.addEventListener("keydown", shortcuts, true); + try { + composer.focus(); + panel.showPopover(); + const escape = () => + composer.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + + escape(); + expect(panel.matches(":popover-open")).toBe(false); + expect(interrupts).toBe(0); + + escape(); + expect(interrupts).toBe(1); + } finally { + window.removeEventListener("keydown", shortcuts, true); + } + }); + + it("leaves other keys to the shortcuts while a popover is open", () => { + const panel = document.createElement("div"); + panel.setAttribute("popover", "manual"); + document.body.append(panel); + panel.showPopover(); + expect(keyBelongsToDomPopover({ key: "Enter" })).toBe(false); + expect(keyBelongsToDomPopover({ key: "Escape" })).toBe(true); + }); +}); diff --git a/packages/app/src/lib/overlay-root.ts b/packages/app/src/lib/overlay-root.ts index bb0ec6d168..d1ab0b9787 100644 --- a/packages/app/src/lib/overlay-root.ts +++ b/packages/app/src/lib/overlay-root.ts @@ -127,6 +127,16 @@ export function hasOpenDomPopover(): boolean { } } +/** + * Whether a key belongs to an open popover rather than to the app's shortcuts. + * An open popover (a plugin's panel) is the top layer and closes on its own + * Escape listener, which runs after the shortcut engine's capture listener: + * Escape must not also interrupt the running agent. + */ +export function keyBelongsToDomPopover(event: Pick): boolean { + return event.key === "Escape" && hasOpenDomPopover(); +} + function getFocusableElements(scope: HTMLElement): HTMLElement[] { const selector = [ "a[href]",