From 199aae9dde46838538d74f16c9f035ac9c74885f Mon Sep 17 00:00:00 2001 From: luke Date: Thu, 15 Oct 2026 14:25:34 +0800 Subject: [PATCH] test(app): cover Escape with a popover open through the shortcut path The first test only checked hasOpenDomPopover and would still pass with the guard removed from useKeyboardShortcuts. The check is now keyBelongsToDomPopover, called by the hook, and a browser test drives it the way the app does: a window capture listener for shortcuts, a popover closed by its own document Escape listener, focus in a textarea. The first Escape closes the popover with no interrupt; the second interrupts. Both new tests fail with the guard removed. Co-Authored-By: Claude Opus 5.5 Co-authored-by: RoundRedDot --- .../app/src/hooks/use-keyboard-shortcuts.ts | 7 +-- .../app/src/lib/overlay-root.browser.test.tsx | 54 ++++++++++++++++++- packages/app/src/lib/overlay-root.ts | 10 ++++ 3 files changed, 65 insertions(+), 6 deletions(-) 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]",