diff --git a/webviews/src/agent-session/acpmux/App.tsx b/webviews/src/agent-session/acpmux/App.tsx index 7f491167af3f..737f9845bab4 100644 --- a/webviews/src/agent-session/acpmux/App.tsx +++ b/webviews/src/agent-session/acpmux/App.tsx @@ -669,6 +669,7 @@ import { SubagentStopContext, running, } from "./subagents/SubagentGroup"; +import { useEscapeClosesSubagent } from "./subagents/subagentEscape"; import { endedEditRow, markRows, type RowMarks } from "./rowMarks"; import { ShellActionsContext, ShellRow, type ShellActions } from "./shell/ShellRow"; import { SwitchNotice } from "./SwitchNotice"; @@ -8740,6 +8741,8 @@ function AcpmuxPane({ initialChat }: { initialChat?: InitialChat }) { // the subagent it was opened on has shown, or the name the app gave the tab would flash away; and not // while this page has no rows for the subagent yet (they load after the pane opens), which would name // it "Subagent of" over the name the chat's page gave it. + // Escape closes a subagent's view to the chat's, as its pill's × does (subagents/subagentEscape.ts). + useEscapeClosesSubagent(!quick && !!subagentView, () => setSubagentView(undefined)); const shownSubagentKnown = !!subagentView && freshAgents.some((agent) => agent.id === subagentView); const shownSubagentTitle = ownSubagentPane && subagentView && shownSubagentKnown ? subagentTitle(subagentView) : undefined; diff --git a/webviews/src/agent-session/acpmux/Localizable.xcstrings b/webviews/src/agent-session/acpmux/Localizable.xcstrings index a24cb29b9403..55bb11795135 100644 --- a/webviews/src/agent-session/acpmux/Localizable.xcstrings +++ b/webviews/src/agent-session/acpmux/Localizable.xcstrings @@ -122856,6 +122856,138 @@ } } }, + "subagents.close": { + "comment": "Agent pane: the × on a subagent's \"Subagent of\" pill that closes its view back to the chat (subagents/SubagentGroup.tsx)", + "extractionState": "manual", + "localizations": { + "ar": { + "stringUnit": { + "state": "translated", + "value": "إغلاق الوكيل الفرعي" + } + }, + "bs": { + "stringUnit": { + "state": "translated", + "value": "Zatvori podagenta" + } + }, + "da": { + "stringUnit": { + "state": "translated", + "value": "Luk underagent" + } + }, + "de": { + "stringUnit": { + "state": "translated", + "value": "Subagent schließen" + } + }, + "en": { + "stringUnit": { + "state": "translated", + "value": "Close subagent" + } + }, + "es": { + "stringUnit": { + "state": "translated", + "value": "Cerrar subagente" + } + }, + "fr": { + "stringUnit": { + "state": "translated", + "value": "Fermer le sous-agent" + } + }, + "it": { + "stringUnit": { + "state": "translated", + "value": "Chiudi subagente" + } + }, + "ja": { + "stringUnit": { + "state": "translated", + "value": "サブエージェントを閉じる" + } + }, + "km": { + "stringUnit": { + "state": "translated", + "value": "បិទភ្នាក់ងាររង" + } + }, + "ko": { + "stringUnit": { + "state": "translated", + "value": "하위 에이전트 닫기" + } + }, + "nb": { + "stringUnit": { + "state": "translated", + "value": "Lukk underagent" + } + }, + "pl": { + "stringUnit": { + "state": "translated", + "value": "Zamknij podagenta" + } + }, + "pt-BR": { + "stringUnit": { + "state": "translated", + "value": "Fechar subagente" + } + }, + "ru": { + "stringUnit": { + "state": "translated", + "value": "Закрыть субагента" + } + }, + "th": { + "stringUnit": { + "state": "translated", + "value": "ปิดเอเจนต์ย่อย" + } + }, + "tr": { + "stringUnit": { + "state": "translated", + "value": "Alt ajanı kapat" + } + }, + "uk": { + "stringUnit": { + "state": "translated", + "value": "Закрити субагента" + } + }, + "vi": { + "stringUnit": { + "state": "translated", + "value": "Đóng tác tử con" + } + }, + "zh-Hans": { + "stringUnit": { + "state": "translated", + "value": "关闭子代理" + } + }, + "zh-Hant": { + "stringUnit": { + "state": "translated", + "value": "關閉子代理" + } + } + } + }, "subagents.of": { "comment": "Agent pane", "extractionState": "manual", diff --git a/webviews/src/agent-session/acpmux/paneSurface.ts b/webviews/src/agent-session/acpmux/paneSurface.ts index 4d750de6b9ba..c495f9c77566 100644 --- a/webviews/src/agent-session/acpmux/paneSurface.ts +++ b/webviews/src/agent-session/acpmux/paneSurface.ts @@ -37,7 +37,7 @@ type EscapeDismissOptions = { capture?: boolean; }; -function escapeSurfaceOpen(document: Document): boolean { +export function escapeSurfaceOpen(document: Document): boolean { return [ ...document.querySelectorAll( '[role="menu"], [role="listbox"], [role="dialog"], .ui-popup, [aria-haspopup][data-popup-open], ' + diff --git a/webviews/src/agent-session/acpmux/subagents/SubagentGroup.tsx b/webviews/src/agent-session/acpmux/subagents/SubagentGroup.tsx index 578388414ee5..b1abad449793 100644 --- a/webviews/src/agent-session/acpmux/subagents/SubagentGroup.tsx +++ b/webviews/src/agent-session/acpmux/subagents/SubagentGroup.tsx @@ -258,21 +258,35 @@ export function SubagentOfRow({ row }: { row: AcpmuxRow }) { const open = useContext(SubagentNavContext); const agent = row.subagents?.[0]; if (!agent) return null; + // The pill's text goes back to the parent (its parent subagent's view, else the chat's); its × + // closes the subagent's view to the chat's, as Escape does (subagentEscape.ts). Leo, 2026-11-03: + // "this opened subagent has no close button". return (
- + + + +
); } diff --git a/webviews/src/agent-session/acpmux/subagents/subagentEscape.ts b/webviews/src/agent-session/acpmux/subagents/subagentEscape.ts new file mode 100644 index 000000000000..8d69e11833b6 --- /dev/null +++ b/webviews/src/agent-session/acpmux/subagents/subagentEscape.ts @@ -0,0 +1,42 @@ +// Escape closes a subagent's view to the chat's, as the "Subagent of" pill's × does +// (SubagentGroup.tsx SubagentOfRow). Leo, 2026-11-03: "this opened subagent has no close button". +import { useEffect, useRef } from "react"; +import { escapeSurfaceOpen } from "../paneSurface"; + +const EDITABLE = "input, textarea, select, [contenteditable]:not([contenteditable=false])"; + +/// Whether an Escape keydown may close the subagent's view. Not one with a modifier or inside an IME +/// composition, not one a control already took, not one typed in a text field (the composer's first +/// Escape lets go of the caret, composerFocus.ts), and not while a menu, popover or dialog that owns +/// Escape is open. +export function escapeClosesSubagent(event: KeyboardEvent, document: Document): boolean { + if (event.key !== "Escape" || event.defaultPrevented) return false; + if (event.isComposing || event.keyCode === 229) return false; + if (event.shiftKey || event.altKey || event.metaKey || event.ctrlKey) return false; + const target = event.target; + const element = target && (target as Node).nodeType === 1 ? (target as Element) : undefined; + if (element?.closest(EDITABLE)) return false; + return !escapeSurfaceOpen(document); +} + +/// While a subagent's view is shown (`enabled`), an Escape nothing else took calls `onClose`. The +/// decision waits a microtask, past the listeners that run before it, and claims the key: with a turn +/// running, the composer's page-level double press (escapeStop.ts, which waits a task) then leaves it +/// alone, so the press closes the view and stops nothing. +export function useEscapeClosesSubagent(enabled: boolean, onClose: () => void) { + const latest = useRef(onClose); + latest.current = onClose; + useEffect(() => { + if (!enabled) return; + const onKey = (event: KeyboardEvent) => { + if (!escapeClosesSubagent(event, document)) return; + queueMicrotask(() => { + if (event.defaultPrevented) return; + event.preventDefault(); + latest.current(); + }); + }; + document.addEventListener("keydown", onKey); + return () => document.removeEventListener("keydown", onKey); + }, [enabled]); +} diff --git a/webviews/src/agent-session/acpmux/subagents/subagents.css b/webviews/src/agent-session/acpmux/subagents/subagents.css index 3ed63820c284..40f03b4f8fee 100644 --- a/webviews/src/agent-session/acpmux/subagents/subagents.css +++ b/webviews/src/agent-session/acpmux/subagents/subagents.css @@ -202,7 +202,8 @@ button.cv-subagent > :last-child { opacity: 0.5; } /* The child view's first line (model.ts: 44px): a 28px "Subagent of" chip centred on a rule, - 8px above and below. A click goes back to the parent's view. */ + 8px above and below. Its text goes back to the parent's view; its × closes the subagent's view + (the summary card's Subagents tab close: 18px, 12px glyph, its hover on the × alone). */ .cv-subagent-of { box-sizing: border-box; height: 44px; @@ -218,17 +219,27 @@ button.cv-subagent > :last-child { height: 1px; background: var(--cv-card-ring); } -.cv-subagent-of__pill { - all: unset; +.cv-subagent-of__frame { box-sizing: border-box; height: 28px; max-width: 70%; - padding: 0 var(--ds-space-2); + min-width: 0; display: inline-flex; align-items: center; - gap: var(--ds-space-2); border-radius: var(--ds-radius-control); box-shadow: 0 0 0 1px var(--cv-card-ring); +} +.cv-subagent-of__pill { + all: unset; + box-sizing: border-box; + height: 100%; + min-width: 0; + flex: 0 1 auto; + padding: 0 var(--ds-space-1) 0 var(--ds-space-2); + display: inline-flex; + align-items: center; + gap: var(--ds-space-2); + border-radius: var(--ds-radius-control); font-family: var(--cv-font); font-size: var(--text-detail); color: var(--cv-text-muted); @@ -243,6 +254,28 @@ button.cv-subagent > :last-child { outline: 2px solid var(--agent-accent); outline-offset: 2px; } +.cv-subagent-of__close { + flex: none; + display: grid; + place-items: center; + width: 18px; + height: 18px; + margin: 0 var(--ds-space-1) 0 2px; + padding: 0; + border: 0; + border-radius: var(--ds-radius-sm); + background: none; + color: var(--agent-muted); + transition: background-color var(--acpmux-motion-hover, 80ms) var(--acpmux-motion-ease, ease-out); +} +.cv-subagent-of__close:hover { + background: color-mix(in srgb, currentColor 12%, transparent); + color: var(--agent-text); +} +.cv-subagent-of__close:focus-visible { + outline: 1.5px solid var(--agent-text); + outline-offset: -1.5px; +} .cv-subagent-of__label { flex: none; font-weight: var(--weight-emphasis); diff --git a/webviews/src/agent-session/acpmux/subagents/subagents.gallery.ts b/webviews/src/agent-session/acpmux/subagents/subagents.gallery.ts index f545dd045100..92462e4f44c7 100644 --- a/webviews/src/agent-session/acpmux/subagents/subagents.gallery.ts +++ b/webviews/src/agent-session/acpmux/subagents/subagents.gallery.ts @@ -412,6 +412,41 @@ export default agentPaneEntry({ ); }, }, + "child-close-hover": { + note: "The pill's × closes the subagent's view; hovered, its highlight covers the × alone, not the pill.", + height: 640, + snapshot: NESTED_MIXED, + play: async (ctx) => { + await openChild("Test coverage")(ctx); + await ctx.waitFor(() => ctx.document.querySelector(".cv-subagent-of__close")); + await ctx.hover({ selector: ".cv-subagent-of__close" }); + }, + }, + "child-close": { + note: "The pill's × goes back to the chat's transcript, its group still open.", + height: 680, + snapshot: NESTED_MIXED, + play: async (ctx) => { + await openChild("Test coverage")(ctx); + await ctx.click({ role: "button", name: "Close subagent" }); + await ctx.waitFor( + () => !ctx.document.querySelector(".cv-subagent-of") && ctx.document.querySelector(".cv-subagent"), + ); + }, + }, + "child-escape": { + note: "Escape closes the subagent's view, as the × does.", + height: 680, + snapshot: NESTED_MIXED, + play: async (ctx) => { + await openChild("Test coverage")(ctx); + (ctx.document.activeElement as HTMLElement | null)?.blur?.(); + await ctx.press("Escape"); + await ctx.waitFor( + () => !ctx.document.querySelector(".cv-subagent-of") && ctx.document.querySelector(".cv-subagent"), + ); + }, + }, "own-pane": { note: "A pane the app opened on a subagent (a subagent: reveal): its view, read only, with a line naming the chat in place of the composer.", height: 640, diff --git a/webviews/test/subagent-of-close.test.tsx b/webviews/test/subagent-of-close.test.tsx new file mode 100644 index 000000000000..e8e55e6e6984 --- /dev/null +++ b/webviews/test/subagent-of-close.test.tsx @@ -0,0 +1,148 @@ +// l10n-allow-file: regression fixture text, not shipped UI. +import { afterAll, expect, test } from "bun:test"; +import { JSDOM, VirtualConsole } from "jsdom"; +import type { AcpmuxRow } from "../src/agent-session/acpmux/model"; + +const dom = new JSDOM("
", { + url: "http://localhost/", + pretendToBeVisual: true, + virtualConsole: new VirtualConsole(), +}); +const scope = globalThis as Record; +const replacements: Record = { + window: dom.window, + document: dom.window.document, + navigator: dom.window.navigator, + HTMLElement: dom.window.HTMLElement, + Element: dom.window.Element, + customElements: dom.window.customElements, + Node: dom.window.Node, + MutationObserver: dom.window.MutationObserver, + IntersectionObserver: class { + observe() {} + unobserve() {} + disconnect() {} + }, + ResizeObserver: class { + observe() {} + unobserve() {} + disconnect() {} + }, + requestAnimationFrame: dom.window.requestAnimationFrame.bind(dom.window), + cancelAnimationFrame: dom.window.cancelAnimationFrame.bind(dom.window), + IS_REACT_ACT_ENVIRONMENT: true, +}; +// Pierre's registered elements read their own DOM constructors at import time. +for (const key of Object.getOwnPropertyNames(dom.window)) + if (/^(HTML|SVG|CSS|Shadow|Document|Mutation)/.test(key) && !(key in scope)) + replacements[key] = (dom.window as unknown as Record)[key]; +const saved = Object.keys(replacements).map((key) => [key, key in scope, scope[key]] as const); +Object.assign(scope, replacements); + +// jsdom has no layout. A tall viewport mounts every row of these small conversations. +Object.defineProperty(dom.window.HTMLElement.prototype, "clientHeight", { configurable: true, get: () => 10_000 }); +Object.defineProperty(dom.window.HTMLElement.prototype, "clientWidth", { configurable: true, get: () => 760 }); + +const { act } = await import("react"); +const { createRoot } = await import("react-dom/client"); +const { VirtualTranscript } = await import("../src/agent-session/acpmux/App"); +const { setPaneLanguage } = await import("../src/agent-session/acpmux/i18n"); +const { SUBAGENTS } = await import("../src/agent-session/acpmux/subagents/subagentFold"); +const { childView } = await import("../src/agent-session/acpmux/subagents/subagentRows"); +const { SubagentNavContext } = await import("../src/agent-session/acpmux/subagents/SubagentGroup"); +const { escapeClosesSubagent, useEscapeClosesSubagent } = await import( + "../src/agent-session/acpmux/subagents/subagentEscape" +); + +afterAll(() => { + setPaneLanguage("en"); + dom.window.close(); + for (const [key, had, value] of saved) { + if (had) scope[key] = value; + else delete scope[key]; + } +}); + +// Leo, 2026-11-03: "this opened subagent has no close button". +const group: AcpmuxRow = { + id: "group", + kind: SUBAGENTS, + version: 1, + at: 1, + subagents: [ + { id: "lead", parent: null, name: "Lead", state: "running", startedAt: 1 }, + { id: "unit", parent: "lead", name: "Unit tests", state: "completed", startedAt: 1 }, + ], +}; + +test("the Subagent of pill has a close button that closes to the chat, and its text goes to the parent", async () => { + const root = createRoot(document.getElementById("root")!); + const opened: (string | undefined)[] = []; + const rows = childView("unit", [group], { unit: [] }, new Set(), "Release review")!; + try { + await act(async () => + root.render( + opened.push(id)}> + {}} expanded={new Set()} /> + , + ), + ); + const close = document.querySelector(".cv-subagent-of__close"); + expect(close).not.toBeNull(); + expect(close!.getAttribute("aria-label")).toBe("Close subagent"); + // The × is its own control beside the pill's text, not inside the back button. + expect(close!.closest(".cv-subagent-of__pill")).toBeNull(); + await act(async () => close!.click()); + expect(opened).toEqual([undefined]); + await act(async () => document.querySelector(".cv-subagent-of__pill")!.click()); + expect(opened).toEqual([undefined, "lead"]); + } finally { + await act(async () => root.unmount()); + } +}); + +const escape = (init: KeyboardEventInit = {}) => + new dom.window.KeyboardEvent("keydown", { key: "Escape", bubbles: true, cancelable: true, ...init }); + +test("Escape closes the subagent's view unless a text field, a modifier or an open menu has it", () => { + const doc = dom.window.document; + expect(escapeClosesSubagent(escape() as unknown as KeyboardEvent, doc)).toBe(true); + expect(escapeClosesSubagent(escape({ metaKey: true }) as unknown as KeyboardEvent, doc)).toBe(false); + const taken = escape(); + taken.preventDefault(); + expect(escapeClosesSubagent(taken as unknown as KeyboardEvent, doc)).toBe(false); + const field = doc.createElement("textarea"); + doc.body.append(field); + const typed = escape(); + field.dispatchEvent(typed); + expect(escapeClosesSubagent(typed as unknown as KeyboardEvent, doc)).toBe(false); + field.remove(); + const menu = doc.createElement("div"); + menu.setAttribute("role", "menu"); + doc.body.append(menu); + expect(escapeClosesSubagent(escape() as unknown as KeyboardEvent, doc)).toBe(false); + menu.remove(); +}); + +test("the hook closes on an unclaimed Escape and claims it, and does nothing while off", async () => { + const root = createRoot(document.getElementById("root")!); + let closed = 0; + function Probe({ on }: { on: boolean }) { + useEscapeClosesSubagent(on, () => closed++); + return null; + } + try { + await act(async () => root.render()); + document.body.dispatchEvent(escape()); + await Promise.resolve(); + expect(closed).toBe(0); + await act(async () => root.render()); + const event = escape(); + document.body.dispatchEvent(event); + await Promise.resolve(); + expect(closed).toBe(1); + expect(event.defaultPrevented).toBe(true); + } finally { + await act(async () => root.unmount()); + } +});