From ebc4ebc007ae4cad2de24afb554c4d3cc698ddf6 Mon Sep 17 00:00:00 2001 From: Hannah Date: Tue, 6 Oct 2026 15:58:37 +0800 Subject: [PATCH] fix(app): retain deferred terminal focus within overlays Keep autofocus pending when focus moves inside the top registered overlay scope, while preserving cancellation for outside controls and workspace or pane loss. Add a real-browser regression that fails before the fix and passes with terminal typing after overlay dismissal. --- docs/floating-panels.md | 5 +++++ .../terminal-retained-tab-stream.spec.ts | 22 +++++++++++++++++++ .../app/e2e/support/helpers/terminal-dsl.ts | 7 ++++++ packages/app/src/components/terminal-pane.tsx | 16 ++++++++++---- packages/app/src/lib/overlay-root.ts | 4 ++++ 5 files changed, 50 insertions(+), 4 deletions(-) diff --git a/docs/floating-panels.md b/docs/floating-panels.md index 0d3983c343..c08076b85c 100644 --- a/docs/floating-panels.md +++ b/docs/floating-panels.md @@ -95,6 +95,11 @@ highest painted scope alone receives overlay keys, traps focus, and restores focus when it closes. Hover cards register with `manageFocus={false}`: they claim overlay keys without moving or trapping focus on pointer hover. Do not add component-local global Escape listeners: two stacked overlays would both close on one keypress. +When deferring workspace terminal autofocus, distinguish movement within the +top registered overlay scope from a new focus owner outside it. Internal focus +movement must keep the request pending until the overlay closes; an outside +control cancels it so its next keystroke is not sent to the terminal. + If an overlay is rendered by a global host rather than beneath its opener in the React tree, carry the opener's current layer through the host store and restore it with `OverlayLayerProvider`. Otherwise painting and keyboard diff --git a/packages/app/e2e/browser/terminal-retained-tab-stream.spec.ts b/packages/app/e2e/browser/terminal-retained-tab-stream.spec.ts index 63425dfef3..61379549ac 100644 --- a/packages/app/e2e/browser/terminal-retained-tab-stream.spec.ts +++ b/packages/app/e2e/browser/terminal-retained-tab-stream.spec.ts @@ -103,6 +103,28 @@ test.describe("retained terminal tab streams", () => { }); }); + test("moving focus inside an overlay preserves deferred terminal focus", async ({ page }) => { + const first = await harness.createTerminal({ name: "overlay-move-first" }); + const other = await harness.createOtherWorkspace(); + const second = await other.createTerminal({ name: "overlay-move-second" }); + await harness.openTerminal(page, { terminalId: first.id }); + await other.openTerminal(page, { terminalId: second.id }); + await harness.switchToWorkspaceByShortcut(page); + await harness.expectTerminalFocused(page); + await harness.rememberWorkspaceForHistoryReturn(page); + await other.switchToWorkspaceByShortcut(page); + await openCommandCenter(page); + await harness.returnToWorkspaceThroughHistory(page); + await harness.moveFocusWithinCommandCenter(page); + await closeCommandCenter(page); + await harness.expectTerminalFocused(page); + await harness.typeCommandAndExpectOutput(page, { + terminalId: first.id, + command: "printf 'OVERLAY_MOVE_FOCUS_OK\\n'", + output: "OVERLAY_MOVE_FOCUS_OK", + }); + }); + test("deferred terminal focus does not steal focus from a button", async ({ page }) => { const first = await harness.createTerminal({ name: "button-first" }); const other = await harness.createOtherWorkspace(); diff --git a/packages/app/e2e/support/helpers/terminal-dsl.ts b/packages/app/e2e/support/helpers/terminal-dsl.ts index eb1dc76d59..4fff26095d 100644 --- a/packages/app/e2e/support/helpers/terminal-dsl.ts +++ b/packages/app/e2e/support/helpers/terminal-dsl.ts @@ -164,6 +164,13 @@ export class TerminalE2EHarness { await expect(page).toHaveURL(new RegExp(`/workspace/${this.workspaceId}`)); } + async moveFocusWithinCommandCenter(page: Page): Promise { + const panel = page.getByTestId("command-center-panel"); + await expect(panel.getByTestId("command-center-input")).toBeFocused(); + await page.keyboard.press("Tab"); + await expect(panel.getByRole("button").first()).toBeFocused(); + } + async closeOverlayAndFocusButtonBeforeRetry(page: Page): Promise { await page.clock.install(); await page.clock.pauseAt(new Date()); diff --git a/packages/app/src/components/terminal-pane.tsx b/packages/app/src/components/terminal-pane.tsx index 61f1e4024d..3296dbfb55 100644 --- a/packages/app/src/components/terminal-pane.tsx +++ b/packages/app/src/components/terminal-pane.tsx @@ -2,7 +2,11 @@ import { TerminalFind, type TerminalPaneFindHandle } from "@/terminal/find"; import type { TerminalFindResult } from "@/terminal/runtime/terminal-emulator-runtime"; import { LoadingSpinner } from "@/components/ui/loading-spinner"; import { useRetainedPanelActive } from "@/components/retained-panel"; -import { hasActiveWebOverlay, subscribeWebOverlayChanges } from "@/lib/overlay-root"; +import { + hasActiveWebOverlay, + isWithinActiveWebOverlay, + subscribeWebOverlayChanges, +} from "@/lib/overlay-root"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import * as Clipboard from "expo-clipboard"; import { @@ -408,10 +412,14 @@ export function TerminalPane({ let retryFrame: number | null = null; function stopRetrying(): void { document.removeEventListener("focusout", scheduleRetry); - document.removeEventListener("focusin", stopRetrying); + document.removeEventListener("focusin", handleFocusIn); unsubscribeOverlayChanges(); if (retryFrame !== null) window.cancelAnimationFrame(retryFrame); } + function handleFocusIn(event: FocusEvent): void { + // Moving within the overlay does not replace the deferred workspace focus owner. + if (!isWithinActiveWebOverlay(event.target)) stopRetrying(); + } function scheduleRetry(): void { if (retryFrame !== null) return; // Let blur, overlay teardown, and focus restoration settle before checking ownership. @@ -422,8 +430,8 @@ export function TerminalPane({ } const unsubscribeOverlayChanges = subscribeWebOverlayChanges(scheduleRetry); document.addEventListener("focusout", scheduleRetry); - // A new focus owner wins over a deferred workspace autofocus request. - document.addEventListener("focusin", stopRetrying); + // A new focus owner outside the overlay wins over deferred workspace autofocus. + document.addEventListener("focusin", handleFocusIn); return stopRetrying; }, [ isMobile, diff --git a/packages/app/src/lib/overlay-root.ts b/packages/app/src/lib/overlay-root.ts index 0043456509..60df676d24 100644 --- a/packages/app/src/lib/overlay-root.ts +++ b/packages/app/src/lib/overlay-root.ts @@ -113,6 +113,10 @@ export function hasActiveWebOverlay(): boolean { return getTopWebOverlay() !== undefined; } +export function isWithinActiveWebOverlay(target: EventTarget | null): boolean { + return target instanceof Node && (getTopWebOverlay()?.getScope()?.contains(target) ?? false); +} + export function subscribeWebOverlayChanges(listener: () => void): () => void { webOverlayChangeListeners.add(listener); return () => {