diff --git a/dev-docs/NATIVE_INTERRUPTION_UX.md b/dev-docs/NATIVE_INTERRUPTION_UX.md index 3f6b9ddf6..7c945493d 100644 --- a/dev-docs/NATIVE_INTERRUPTION_UX.md +++ b/dev-docs/NATIVE_INTERRUPTION_UX.md @@ -35,6 +35,9 @@ instead of clipping controls; field scrolling can chain into that outer scroller by request kind/id, preserving partial answers and rejection reasons through native object replacement, queue navigation and session navigation. - Collapse only hides the panel body. It never cancels or refuses a request. +- Deferred session-activation focus must recheck the current input and modal owner + when it runs, and expire when the pane deactivates or unmounts. An answer field + focused during navigation retains keyboard input instead of yielding to the composer. - Pending questions appear only in the dock. There are no “View in discussion” or “Answer in dock” links and no interruption-specific transcript reveal state. - Only the panel submits replies. Transcript tools no longer register document-wide diff --git a/packages/ui/src/components/session/session-view.tsx b/packages/ui/src/components/session/session-view.tsx index 63106be2a..9ddc55795 100644 --- a/packages/ui/src/components/session/session-view.tsx +++ b/packages/ui/src/components/session/session-view.tsx @@ -294,8 +294,8 @@ export const SessionView: Component = (props) => { createEffect( on( - () => props.isActive, - (isActive) => { + () => [props.isActive, props.sessionId] as const, + ([isActive]) => { if (!isActive) { if (props.focusConversationOnActivate) props.onConversationFocusHandled?.() clearConversationPlaybackForSession(props.instanceId, props.sessionId) @@ -307,33 +307,42 @@ export const SessionView: Component = (props) => { // Don't steal focus from other inputs (command palette, dialogs, selectors, etc.) if (typeof document === "undefined") return - const activeEl = document.activeElement as HTMLElement | null - const activeIsInput = - activeEl?.tagName === "INPUT" || - activeEl?.tagName === "TEXTAREA" || - activeEl?.tagName === "SELECT" || - Boolean(activeEl?.isContentEditable) - if (activeIsInput) return - - const modalOpen = Boolean(document.querySelector('[role="dialog"][aria-modal="true"]')) - if (modalOpen) return + const activeEl = document.activeElement + const focusIsProtected = () => { + const current = document.activeElement as HTMLElement | null + return current?.matches("input, textarea, select") || current?.isContentEditable + || Boolean(current?.closest(".interruption-dock")) + || Boolean(document.querySelector('[role="dialog"][aria-modal="true"]')) + } + if (focusIsProtected()) return // Defer until the session pane is visible and the textarea is mounted. - requestAnimationFrame(() => { - requestAnimationFrame(() => { - if (!props.isActive) return + // Cleanup also fences already-dispatched frames across rapid reactivation. + let cancelled = false + let frame: number + onCleanup(() => { + cancelled = true + cancelAnimationFrame(frame) + }) + frame = requestAnimationFrame(function waitForActivatedSession() { + if (cancelled || !props.isActive) return + frame = requestAnimationFrame(function focusActivatedSession() { + if (cancelled || !props.isActive || !rootRef?.isConnected) return + const activeElement = document.activeElement + const focusIsUnclaimed = + !activeElement || activeElement === document.body || activeElement === document.documentElement + // Input/modal ownership can change while either frame is pending. + // Preserve a newly claimed control, including the dock's own chrome. + const focusIsBlocked = focusIsProtected() || (!focusIsUnclaimed && activeElement !== activeEl) if (props.focusConversationOnActivate) { - const activeElement = document.activeElement - const focusIsUnclaimed = - !activeElement || activeElement === document.body || activeElement === document.documentElement - const modalIsOpen = Boolean(document.querySelector('[role="dialog"][aria-modal="true"]')) - if (focusIsUnclaimed && !modalIsOpen && focusConversationStream(rootRef)) { + if (focusIsUnclaimed && !focusIsBlocked && focusConversationStream(rootRef)) { props.onConversationFocusHandled?.() return } props.onConversationFocusHandled?.() - if (!focusIsUnclaimed || modalIsOpen) return + if (!focusIsUnclaimed) return } + if (focusIsBlocked) return if (promptInputApi) { promptInputApi.focus() return diff --git a/packages/ui/tests/browser/fixtures/activation-frame-gate.ts b/packages/ui/tests/browser/fixtures/activation-frame-gate.ts new file mode 100644 index 000000000..9682ced76 --- /dev/null +++ b/packages/ui/tests/browser/fixtures/activation-frame-gate.ts @@ -0,0 +1,33 @@ +// Hold only SessionView's final activation-focus frame. Transcript/layout frames +// keep running, and native frame IDs/cancellation retain their normal meaning. +export function installActivationFrameGate() { + const request = window.requestAnimationFrame.bind(window) + const cancel = window.cancelAnimationFrame.bind(window) + const held = new Map() + let paused = false + window.requestAnimationFrame = callback => { + if (!paused || callback.name !== "focusActivatedSession") return request(callback) + const id = request(() => {}) + held.set(id, { callback, cancelled: false }) + return id + } + window.cancelAnimationFrame = id => { + const entry = held.get(id) + if (entry) entry.cancelled = true + cancel(id) + } + return { + pause: () => { paused = true }, + pending: () => [...held.values()].filter(entry => !entry.cancelled).length, + cancelled: () => [...held.values()].filter(entry => entry.cancelled).length, + flush: (cancelledOnly = false) => { + for (const [id, entry] of [...held]) { + if (cancelledOnly && !entry.cancelled) continue + held.delete(id) + cancel(id) + // Explicitly exercise callbacks already dispatched when cleanup occurs. + if (cancelledOnly || !entry.cancelled) entry.callback(performance.now()) + } + }, + } +} diff --git a/packages/ui/tests/browser/fixtures/interruption-dock.tsx b/packages/ui/tests/browser/fixtures/interruption-dock.tsx index 682c2b879..0b412f6e6 100644 --- a/packages/ui/tests/browser/fixtures/interruption-dock.tsx +++ b/packages/ui/tests/browser/fixtures/interruption-dock.tsx @@ -1,4 +1,4 @@ -import { For } from "solid-js" +import { For, createSignal } from "solid-js" import { render } from "solid-js/web" import SessionView from "../../../src/components/session/session-view" import { InterruptionDock } from "../../../src/components/interruption-dock" @@ -18,6 +18,7 @@ import { loadMessages, loadMessageAnchor } from "../../../src/stores/session-api import { applyUiSettings } from "./ui-settings" import { getQuestionToolSearchText } from "../../../src/components/tool-call/search-text" import "../../../src/index.css" +import { installActivationFrameGate } from "./activation-frame-gate" const instanceId = "interruptions", sessionId = "s", toolId = "question-tool" let messageId = "msg_0000" @@ -110,18 +111,27 @@ serverApi.patchStateOwner = async (_owner, patch) => { serverApi.fetchPermissionReceipts = async () => ({ receipts: [] }) await applyUiSettings({ locale: "en", showMessageTimeline: false, toolInputsVisibility: "hidden", toolOutputExpansion: "expanded", toolCallExpansionDefaults: { preset: "custom", thinking: "collapsed", tools: { other: "expanded" } } }) +const activationFrames = installActivationFrameGate() +const [active, setActive] = createSignal(true) +const [conversationFocus, setConversationFocus] = createSignal(false) +const [phone, setPhone] = createSignal(false) +let focusHandled = 0 function App() { const panel = return
focusInterruption(instanceId)} /> {id => {}} onModelChange={async () => {}} />} + escapeInDebounce={false} isActive={active()} isPhoneLayout={phone()} focusConversationOnActivate={conversationFocus()} + onConversationFocusHandled={() => { focusHandled++ }} + interruptionPanel={panel} onAgentChange={async () => {}} onModelChange={async () => {}} />}
} render(() => , document.getElementById("root")!) const store = messageStoreBus.getOrCreate(instanceId) ;(window as any).fixture = { + activationFrames, active: setActive, conversationFocus: setConversationFocus, phone: setPhone, + focusHandled: () => focusHandled, replies, windows, theme: setThemePreference, ask: () => emit("form.created", { form: form() }), diff --git a/packages/ui/tests/browser/interruption-selection.test.ts b/packages/ui/tests/browser/interruption-selection.test.ts index c217dffb4..9e4a69afd 100644 --- a/packages/ui/tests/browser/interruption-selection.test.ts +++ b/packages/ui/tests/browser/interruption-selection.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict" import { after, before, test } from "node:test" import { fileURLToPath } from "node:url" -import { chromium, type Browser } from "playwright" +import { chromium, type Browser, type Page } from "playwright" import { createServer, type ViteDevServer } from "vite" import solid from "vite-plugin-solid" @@ -47,9 +47,16 @@ test("a question stays selected and focused when the newly visited session recei await answer.fill("Draft before switching") // This session has no request: the same question remains visible after the pane remount. - await page.evaluate(() => (window as any).fixture.switch("other")) + await page.evaluate(() => { + const fixture = (window as any).fixture + fixture.activationFrames.pause() + fixture.switch("other") + }) + await page.waitForFunction(() => (window as any).fixture.activationFrames.pending() === 1) assert.equal(await answer.inputValue(), "Draft before switching") await answer.fill("Continue the original answer") + await page.evaluate(() => (window as any).fixture.activationFrames.flush()) + assert.equal(await answer.evaluate(element => element === document.activeElement), true) const pending = await page.evaluate(async modulePath => { const { addPermissionToQueue, getPermissionQueue } = await import(/* @vite-ignore */ modulePath) @@ -63,6 +70,7 @@ test("a question stays selected and focused when the newly visited session recei assert.deepEqual(pending, [{ id: "permission-other", sessionID: "other" }]) assert.equal(await answer.inputValue(), "Continue the original answer") + assert.equal(await page.locator(".prompt-input").inputValue(), "") assert.equal(await answer.evaluate(element => element === document.activeElement), true) assert.equal(await page.locator(".interruption-session").innerText(), "Main session") assert.equal(await page.locator(".interruption-position").innerText(), "2 / 2") @@ -73,3 +81,135 @@ test("a question stays selected and focused when the newly visited session recei await page.close() } }) + +async function withActivationPage(run: (page: Page) => Promise) { + const page = await browser.newPage({ viewport: { width: 1100, height: 800 } }) + const errors: string[] = [] + page.on("pageerror", error => errors.push(error.message)) + try { + await page.route("**/api/**", route => route.fulfill({ contentType: "application/json", body: "{}" })) + await page.goto(url, { waitUntil: "domcontentloaded", timeout: 60000 }) + await page.waitForFunction(() => Boolean((window as any).fixture?.snapshot().ids.length), undefined, { timeout: 60000 }) + await page.evaluate(() => { + const fixture = (window as any).fixture + fixture.active(false) + fixture.activationFrames.pause() + ;(document.activeElement as HTMLElement)?.blur() + }) + await run(page) + assert.deepEqual(errors, []) + } finally { + await page.close() + } +} + +async function activate(page: Page, conversation: boolean) { + await page.evaluate(conversation => { + const fixture = (window as any).fixture + fixture.active(false) + ;(document.activeElement as HTMLElement)?.blur() + fixture.conversationFocus(conversation) + fixture.active(true) + }, conversation) + await page.waitForFunction(() => (window as any).fixture.activationFrames.pending() === 1) +} + +for (const conversation of [false, true]) { + const mode = conversation ? "conversation" : "composer" + test(`deferred ${mode} activation respects inputs, dock focus and a newly opened modal`, async () => { + await withActivationPage(async page => { + await page.evaluate(() => (window as any).fixture.ask()) + for (const target of ["input", "textarea", "select", "editable", "dock", "dock-button", "modal"]) { + await activate(page, conversation) + const preserved = await page.evaluate(target => { + let control: HTMLElement + if (target === "dock" || target === "dock-button") { + control = document.querySelector(target === "dock" ? ".interruption-dock" : ".interruption-toggle")! + } else { + control = document.createElement(target === "editable" || target === "modal" ? "div" : target) + control.id = "competing-focus" + if (target === "editable") control.contentEditable = "true" + if (target === "modal") { + control.setAttribute("role", "dialog") + control.setAttribute("aria-modal", "true") + } + document.body.append(control) + } + if (target !== "modal") control.focus() + const focused = document.activeElement + ;(window as any).fixture.activationFrames.flush() + return focused === document.activeElement + }, target) + assert.equal(preserved, true, `${mode} must preserve ${target} ownership`) + await page.evaluate(() => document.getElementById("competing-focus")?.remove()) + } + }) + }) + + test(`ordinary ${mode} activation still focuses the requested surface`, async () => { + await withActivationPage(async page => { + await activate(page, conversation) + const handled = await page.evaluate(() => { + const fixture = (window as any).fixture + const before = fixture.focusHandled() + fixture.activationFrames.flush() + return fixture.focusHandled() - before + }) + assert.equal(handled, conversation ? 1 : 0) + assert.equal(await page.locator(conversation ? ".message-stream" : ".prompt-input") + .evaluate(element => element === document.activeElement), true) + }) + }) +} + +test("inactive, disposed and superseded activation frames cannot reclaim focus", async () => { + await withActivationPage(async page => { + await activate(page, false) + await page.evaluate(() => (window as any).fixture.active(false)) + assert.equal(await page.evaluate(() => (window as any).fixture.activationFrames.cancelled()), 1) + assert.equal(await page.evaluate(() => { + ;(window as any).fixture.activationFrames.flush(true) + return document.activeElement === document.body + }), true, "an inactive callback must be inert even if already dispatched") + + await activate(page, false) + await page.evaluate(() => { + const fixture = (window as any).fixture + fixture.active(false) + fixture.active(true) + }) + await page.waitForFunction(() => (window as any).fixture.activationFrames.pending() === 1) + assert.equal(await page.evaluate(() => { + ;(window as any).fixture.activationFrames.flush(true) + return document.activeElement === document.body + }), true, "an old activation must not become valid again after reactivation") + await page.evaluate(() => (window as any).fixture.activationFrames.flush()) + assert.equal(await page.locator(".prompt-input").evaluate(element => element === document.activeElement), true) + + await activate(page, false) + await page.evaluate(() => (window as any).fixture.switch("other")) + await page.waitForFunction(() => (window as any).fixture.activationFrames.pending() === 1) + assert.equal(await page.evaluate(() => { + ;(window as any).fixture.activationFrames.flush(true) + return document.activeElement === document.body + }), true, "a disposed pane must not focus its detached composer") + await page.evaluate(() => (window as any).fixture.activationFrames.flush()) + assert.equal(await page.locator(".prompt-input").evaluate(element => element === document.activeElement), true) + }) +}) + +test("phone activation does not summon the composer keyboard", async () => { + await withActivationPage(async page => { + await page.evaluate(() => { + const fixture = (window as any).fixture + fixture.phone(true) + fixture.active(true) + }) + await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(() => resolve())))) + assert.equal(await page.evaluate(() => (window as any).fixture.activationFrames.pending()), 0) + assert.equal(await page.evaluate(() => document.activeElement === document.body), true) + await activate(page, true) + await page.evaluate(() => (window as any).fixture.activationFrames.flush()) + assert.equal(await page.locator(".message-stream").evaluate(element => element === document.activeElement), true) + }) +}) diff --git a/packages/ui/tests/browser/session-rendering.test.ts b/packages/ui/tests/browser/session-rendering.test.ts index 265ff33cb..ef87d9a47 100644 --- a/packages/ui/tests/browser/session-rendering.test.ts +++ b/packages/ui/tests/browser/session-rendering.test.ts @@ -54,7 +54,12 @@ async function open(name: string, run: (page: Page) => Promise) { if (window.fixtureScrollEvents.length > 80) window.fixtureScrollEvents.shift(); }, { capture: true, passive: true }); })()`) - await page.route("**/api/**", route => route.fulfill({ contentType: route.request().url().includes("events") ? "text/event-stream" : "application/json", body: "" })) + await page.route("**/api/**", route => { + if (new URL(route.request().url()).pathname.endsWith("/permission-receipts")) { + return route.fulfill({ contentType: "application/json", body: JSON.stringify({ receipts: [] }) }) + } + return route.fulfill({ contentType: route.request().url().includes("events") ? "text/event-stream" : "application/json", body: "" }) + }) await runWithDiagnosticCleanup({ run: async () => { await page.goto(`${baseUrl}/fixture?${name}`)