diff --git a/packages/app/e2e/browser/bottom-sheet-reopen.spec.ts b/packages/app/e2e/browser/bottom-sheet-reopen.spec.ts index 89ee10af22..bc65ae65de 100644 --- a/packages/app/e2e/browser/bottom-sheet-reopen.spec.ts +++ b/packages/app/e2e/browser/bottom-sheet-reopen.spec.ts @@ -89,9 +89,11 @@ async function dismissStackedModelPickerWithBackdrop(page: Page) { .getByTestId("agent-controls-settings-list") .getByRole("button", { name: /Select model/ }) .click(); - await expect(page.getByTestId("agent-controls-model-browser-sheet")).toBeVisible(); + await expect(page.getByTestId("agent-controls-model-browser-sheet")).toBeInViewport(); await page.mouse.click(MOBILE_VIEWPORT.width / 2, 24); - await expect(page.getByTestId("agent-controls-model-browser-sheet")).not.toBeVisible(); + // In CI, Gorhom retains the dismissed sheet below the viewport. + // Dismissal is complete when it leaves the displayed surface. + await expect(page.getByTestId("agent-controls-model-browser-sheet")).not.toBeInViewport(); await expect(page.getByTestId("agent-controls-model-sheet")).toBeVisible(); await expect(page.getByTestId("agent-controls-settings-list")).toBeVisible(); } @@ -139,7 +141,7 @@ test.describe("mobile bottom sheet reopen", () => { await expect(sheet).toBeVisible(); await expect(page.getByTestId("agent-controls-settings-list")).toBeVisible(); await expect(page.getByTestId("agent-controls-model")).toContainText("Ten second stream"); - await expect(page.getByTestId("agent-controls-model-browser-sheet")).not.toBeVisible(); + await expect(page.getByTestId("agent-controls-model-browser-sheet")).not.toBeInViewport(); }); } finally { await session.cleanup(); diff --git a/packages/app/e2e/browser/plugin-popover.spec.ts b/packages/app/e2e/browser/plugin-popover.spec.ts new file mode 100644 index 0000000000..8ce3b35513 --- /dev/null +++ b/packages/app/e2e/browser/plugin-popover.spec.ts @@ -0,0 +1,47 @@ +import { expect, test, type Page } from "../support/fixtures"; +import { composerLocator, submitMessageWithButton } from "../support/helpers/composer"; +import { seedMockAgentWorkspace, openAgentRoute } from "../support/helpers/mock-agent"; +import { withPopoverTimelinePlugin } from "../support/helpers/plugin-timeline"; + +async function openPreviewWhileTyping(page: Page): Promise { + await composerLocator(page).focus(); + await page.getByRole("button", { name: "Hover plugin preview", exact: true }).hover(); + await expect(page.getByRole("dialog", { name: "Plugin preview" })).toBeVisible(); + await expect(composerLocator(page)).toBeFocused(); +} + +async function dismissPreview(page: Page): Promise { + await page.keyboard.press("Escape"); + await expect(page.getByRole("dialog", { name: "Plugin preview" })).not.toBeVisible(); +} + +test("Escape dismisses a plugin preview before interrupting the agent", async ({ page }) => { + test.setTimeout(120_000); + const agent = await seedMockAgentWorkspace({ + repoPrefix: "plugin-popover-", + title: "Plugin preview keyboard regression", + model: "e2e-fast-stream", + }); + try { + await withPopoverTimelinePlugin(async () => { + await openAgentRoute(page, agent); + await submitMessageWithButton(page, "Show the plugin preview."); + await agent.client.waitForFinish(agent.agentId, 30_000); + await expect(page.getByRole("button", { name: "Hover plugin preview" })).toBeVisible(); + await submitMessageWithButton(page, "withhold synthetic user message until interrupted"); + const stop = page.getByRole("button", { name: "Stop agent", exact: true }); + await expect(stop).toBeVisible(); + + await openPreviewWhileTyping(page); + await dismissPreview(page); + await expect(stop).toBeVisible(); + await expect(composerLocator(page)).toBeFocused(); + + await page.keyboard.press("Escape"); + await expect(stop).not.toBeVisible(); + await agent.client.waitForFinish(agent.agentId, 15_000); + }); + } finally { + await agent.cleanup(); + } +}); diff --git a/packages/app/e2e/support/helpers/plugin-timeline.ts b/packages/app/e2e/support/helpers/plugin-timeline.ts index 8ad7e0a632..8b0f680e2d 100644 --- a/packages/app/e2e/support/helpers/plugin-timeline.ts +++ b/packages/app/e2e/support/helpers/plugin-timeline.ts @@ -231,3 +231,46 @@ export async function attachPromptTimelineEvidence(page: Page, info: TestInfo): contentType: "image/png", }); } + +const POPOVER_CLIENT_SOURCE = `import React, { useEffect, useRef } from "react"; +import { z } from "zod"; + +function Preview() { + const panel = useRef(null); + useEffect(() => { + const close = (event) => { + if (event.key === "Escape" && panel.current.matches(":popover-open")) { + panel.current.hidePopover(); + } + }; + document.addEventListener("keydown", close, true); + return () => document.removeEventListener("keydown", close, true); + }, []); + return
+ +
Plugin hover preview
+
; +} + +export default function contribute(client) { + client.addTimelineTransformer({ + id: "preview", query: { itemType: "user_message" }, + transform({ item }) { + if (item.text !== "Show the plugin preview.") return; + return { items: [{ type: "plugin", kind: "preview", version: 1, data: {} }] }; + }, + }); + client.addTimelineRenderer({ + kind: "preview", version: 1, schema: z.object({}), Component: Preview, + }); + return () => {}; +}`; + +/** Installs a timeline preview that owns Escape through the DOM Popover API. */ +export async function withPopoverTimelinePlugin(run: () => Promise): Promise { + await withDirectoryPlugin({ + pluginId: "timeline-popover-e2e", + source: POPOVER_CLIENT_SOURCE, + run, + }); +} diff --git a/packages/app/src/hooks/use-keyboard-shortcuts.ts b/packages/app/src/hooks/use-keyboard-shortcuts.ts index d723b5be04..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 } from "@/lib/overlay-root"; +import { dispatchTopWebOverlayKeyDown, keyBelongsToDomPopover } from "@/lib/overlay-root"; export function useKeyboardShortcuts({ enabled, @@ -321,6 +321,10 @@ export function useKeyboardShortcuts({ return; } + if (keyBelongsToDomPopover(event)) { + return; + } + // During IME composition, Enter confirms the candidate selection and must // not route through global shortcuts like message send. if (isImeComposingKeyboardEvent(event)) { diff --git a/packages/app/src/lib/overlay-root.browser.test.tsx b/packages/app/src/lib/overlay-root.browser.test.tsx index a21403dd8d..bd20d2aeeb 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 { useWebOverlayRegistration } from "./overlay-root"; +import { + hasOpenDomPopover, + keyBelongsToDomPopover, + useWebOverlayRegistration, +} from "./overlay-root"; function nextFrame(): Promise { return new Promise((resolve) => requestAnimationFrame(() => resolve())); @@ -68,3 +72,35 @@ describe("useWebOverlayRegistration in the browser", () => { expect(openerFocusCalls).toBe(1); }); }); + +describe("hasOpenDomPopover", () => { + afterEach(() => { + document.body.replaceChildren(); + }); + + it("is true only while a popover is shown", () => { + const panel = document.createElement("div"); + panel.setAttribute("popover", "manual"); + document.body.append(panel); + expect(hasOpenDomPopover()).toBe(false); + panel.showPopover(); + expect(hasOpenDomPopover()).toBe(true); + panel.hidePopover(); + expect(hasOpenDomPopover()).toBe(false); + }); +}); + +describe("Escape with a plugin popover open", () => { + afterEach(() => { + document.body.replaceChildren(); + }); + + 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 9ffd81725f..d1ab0b9787 100644 --- a/packages/app/src/lib/overlay-root.ts +++ b/packages/app/src/lib/overlay-root.ts @@ -112,6 +112,31 @@ export function hasActiveWebOverlay(): boolean { return getTopWebOverlay() !== undefined; } +/** + * True while an element shown with the Popover API is open. Such a layer + * floats above the app like a registered overlay but lives outside the + * registry: plugins draw their panels this way, since they cannot register. + */ +export function hasOpenDomPopover(): boolean { + if (typeof document === "undefined") return false; + try { + return document.querySelector(":popover-open") !== null; + } catch { + // An engine without the Popover API rejects the selector, and has none open. + return false; + } +} + +/** + * 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]",