From 2601bcbc525970d788b1d5e1d3972a567a855e6d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 18 Aug 2026 02:28:26 -0700 Subject: [PATCH] refactor(ui): make task detail a detail-slot document (#125748) * refactor(ui): make task detail a slot document * test(ui): cover task detail slot teardown --- ui/src/pages/chat/chat-pane-render.ts | 7 +- .../pages/chat/components/chat-detail-slot.ts | 81 ++++++++------ ui/src/pages/chat/components/chat-sidebar.ts | 25 +++-- .../components/chat-task-detail-state.test.ts | 103 ++++++++++++++++++ 4 files changed, 166 insertions(+), 50 deletions(-) diff --git a/ui/src/pages/chat/chat-pane-render.ts b/ui/src/pages/chat/chat-pane-render.ts index dd3ffda31f72..8421c51000eb 100644 --- a/ui/src/pages/chat/chat-pane-render.ts +++ b/ui/src/pages/chat/chat-pane-render.ts @@ -45,7 +45,7 @@ import { } from "./chat-state-route.ts"; import type { ChatProps } from "./chat-view.ts"; import { createBackgroundTasksProps } from "./components/chat-background-tasks.ts"; -import { detailSlotOpen } from "./components/chat-detail-slot.ts"; +import { openTaskDetailId } from "./components/chat-detail-slot.ts"; import { chatPullRequestId, createPullRequestBranch } from "./components/chat-pull-requests.ts"; import { createSessionWorkspaceProps, @@ -241,10 +241,7 @@ export class ChatPane extends ChatPaneLayoutRender { }; const backgroundTasksBase = createBackgroundTasksProps(state, { narrowLayout: false, - openTaskId: - state.sidebarContent?.kind === "task" && detailSlotOpen(sidebarLayout) - ? state.sidebarContent.taskId - : undefined, + openTaskId: openTaskDetailId(state.sidebarContent, sidebarLayout), onOpenTaskDetail: (task) => state.handleOpenSidebar({ kind: "task", taskId: task.id }), }); const backgroundTasks = { diff --git a/ui/src/pages/chat/components/chat-detail-slot.ts b/ui/src/pages/chat/components/chat-detail-slot.ts index 6020b4963d3c..a6b4a069132a 100644 --- a/ui/src/pages/chat/components/chat-detail-slot.ts +++ b/ui/src/pages/chat/components/chat-detail-slot.ts @@ -6,7 +6,7 @@ import type { BackgroundTasksProps } from "./chat-background-tasks.types.ts"; import "./chat-sidebar.ts"; import { openSessionWorkspaceFile, revealSessionWorkspaceFile } from "./chat-session-workspace.ts"; import type { SidebarContent, SidebarFullMessageLoader } from "./chat-sidebar.ts"; -import { resetTaskDetail } from "./chat-task-detail-state.ts"; +import { resetTaskDetail, type TaskDetailHost } from "./chat-task-detail-state.ts"; import { renderTaskDetailPanel } from "./chat-task-detail.ts"; import type { ChatTranscriptController } from "./chat-transcript-controller.ts"; @@ -17,6 +17,13 @@ export function detailSlotOpen(layout: SidebarLayout): boolean { return layout.columns.some((column) => column.panels.some((panel) => panel.slot === "detail")); } +export function openTaskDetailId( + content: SidebarContent | null | undefined, + layout: SidebarLayout, +): string | undefined { + return content?.kind === "task" && detailSlotOpen(layout) ? content.taskId : undefined; +} + export function renderChatDetailSlot(params: { backgroundTasks: BackgroundTasksProps; chat: ChatProps; @@ -27,38 +34,44 @@ export function renderChatDetailSlot(params: { transcript: ChatTranscriptController; }): TemplateResult { const { content, host } = params; - if (content.kind === "task") { - if (!detailSlotOpen(params.layout)) { - resetTaskDetail(host); - return html``; - } - return renderTaskDetailPanel({ - backgroundTasks: params.backgroundTasks, - chat: params.chat, - host, - task: params.backgroundTasks.tasks?.find((task) => task.id === content.taskId) ?? undefined, - transcript: params.transcript, - }); + const taskDetailHost: TaskDetailHost = host; + const taskId = openTaskDetailId(content, params.layout); + if (taskId === undefined && taskDetailHost.taskDetailState !== undefined) { + resetTaskDetail(taskDetailHost); } - resetTaskDetail(host); - return html` - openSessionWorkspaceFile(host, target)} - .onOpenSessionLink=${params.chat.onOpenSessionLink} - .onRevealInWorkspace=${(path: string) => { - revealSessionWorkspaceFile(host, path); - host.updateSidebarLayout(openSlot(host.sidebarLayout, "workspace")); - }} - .onOpenImage=${(item: Parameters[0]) => - host.handleOpenImage(item, host.beginImageOpen())} - .embedded=${true} - @chat-detail-panel-close=${() => host.handleCloseSidebar()} - >`; + const documents: Partial> = { + task: + taskId === undefined + ? html`` + : renderTaskDetailPanel({ + backgroundTasks: params.backgroundTasks, + chat: params.chat, + host, + task: params.backgroundTasks.tasks?.find((task) => task.id === taskId) ?? undefined, + transcript: params.transcript, + }), + }; + return ( + documents[content.kind] ?? + html` + openSessionWorkspaceFile(host, target)} + .onOpenSessionLink=${params.chat.onOpenSessionLink} + .onRevealInWorkspace=${(path: string) => { + revealSessionWorkspaceFile(host, path); + host.updateSidebarLayout(openSlot(host.sidebarLayout, "workspace")); + }} + .onOpenImage=${(item: Parameters[0]) => + host.handleOpenImage(item, host.beginImageOpen())} + .embedded=${true} + @chat-detail-panel-close=${() => host.handleCloseSidebar()} + >` + ); } diff --git a/ui/src/pages/chat/components/chat-sidebar.ts b/ui/src/pages/chat/components/chat-sidebar.ts index 515698a05251..3536f59eb2d4 100644 --- a/ui/src/pages/chat/components/chat-sidebar.ts +++ b/ui/src/pages/chat/components/chat-sidebar.ts @@ -151,16 +151,19 @@ function setRetainedFileDraft(content: FileSidebarContent, draft: RetainedFileDr retainedFileDrafts.set(key, draft); } -type ChatDetailContent = +export type SidebarContent = | MarkdownSidebarContent | CanvasSidebarContent | ImageSidebarContent | FileSidebarContent - | SessionDiffSidebarContent; + | SessionDiffSidebarContent + | { kind: "task"; taskId: string }; -export type SidebarContent = ChatDetailContent | { kind: "task"; taskId: string }; +type ChatDetailPanelContent = Exclude; -function hasFullMessageRequest(content: ChatDetailContent): content is ChatDetailContent & { +function hasFullMessageRequest( + content: ChatDetailPanelContent, +): content is ChatDetailPanelContent & { fullMessageRequest: SidebarFullMessageRequest; } { return Boolean( @@ -196,8 +199,8 @@ function toPlainTextCodeFence(value: string, language = ""): string { } function buildRawSidebarContent( - content: ChatDetailContent | null | undefined, -): ChatDetailContent | null { + content: ChatDetailPanelContent | null | undefined, +): ChatDetailPanelContent | null { if (!content) { return null; } @@ -505,7 +508,7 @@ function renderFileSidebarContent( } function resolveSidebarCanvasSandbox( - content: ChatDetailContent, + content: ChatDetailPanelContent, embedSandboxMode: EmbedSandboxMode, ): string { return content.kind === "canvas" @@ -514,7 +517,7 @@ function resolveSidebarCanvasSandbox( } type MarkdownSidebarProps = { - content: ChatDetailContent | null; + content: ChatDetailPanelContent | null; error: string | null; fileView?: FileViewControls; onClose: () => void; @@ -704,7 +707,7 @@ function renderMarkdownSidebar(props: MarkdownSidebarProps) { } class ChatDetailPanel extends OpenClawLightDomElement { - @property({ attribute: false }) content: ChatDetailContent | null = null; + @property({ attribute: false }) content: ChatDetailPanelContent | null = null; @property({ attribute: false }) loadFullMessage?: SidebarFullMessageLoader | null = null; @property() basePath = ""; @property() canvasPluginSurfaceUrl: string | null = null; @@ -719,7 +722,7 @@ class ChatDetailPanel extends OpenClawLightDomElement { @property({ attribute: false }) onRevealInWorkspace?: ((path: string) => void) | null = null; @property({ attribute: false }) onOpenImage?: ((item: ImageLightboxItem) => void) | null = null; - @state() private visibleContent: ChatDetailContent | null = null; + @state() private visibleContent: ChatDetailPanelContent | null = null; @state() private error: string | null = null; @state() private fileSearchOpen = false; @state() private fileSearchQuery = ""; @@ -1249,7 +1252,7 @@ class ChatDetailPanel extends OpenClawLightDomElement { }); }; - private async upgradeToFullMessage(content: ChatDetailContent, version: number) { + private async upgradeToFullMessage(content: ChatDetailPanelContent, version: number) { if (!hasFullMessageRequest(content) || !this.loadFullMessage) { return; } diff --git a/ui/src/pages/chat/components/chat-task-detail-state.test.ts b/ui/src/pages/chat/components/chat-task-detail-state.test.ts index 1afa66f64528..775d7874c908 100644 --- a/ui/src/pages/chat/components/chat-task-detail-state.test.ts +++ b/ui/src/pages/chat/components/chat-task-detail-state.test.ts @@ -1,11 +1,19 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { GatewayBrowserClient } from "../../../api/gateway.ts"; import type { TaskSummary } from "../../../lib/tasks/task-summary.ts"; +import type { ChatPageHost } from "../chat-state-host.ts"; +import type { ChatProps } from "../chat-view.ts"; +import { closeSlot, openSlot, type SidebarLayout } from "../sidebar-layout.ts"; +import type { BackgroundTasksProps } from "./chat-background-tasks.types.ts"; +import { renderChatDetailSlot } from "./chat-detail-slot.ts"; +import type { SidebarContent } from "./chat-sidebar.ts"; +import * as taskDetailState from "./chat-task-detail-state.ts"; import { observeTaskDetailEvent, readTaskTranscript, type TaskDetailHost, } from "./chat-task-detail-state.ts"; +import type { ChatTranscriptController } from "./chat-transcript-controller.ts"; function deferred() { let resolve!: (value: T) => void; @@ -49,6 +57,55 @@ function task(status: TaskSummary["status"]): TaskSummary { }; } +function backgroundTasks(selectedTask: TaskSummary): BackgroundTasksProps { + return { + sessionKey: "agent:main:main", + statusRowId: "chat-tasks-status-test", + collapsed: false, + narrowLayout: false, + connected: true, + canCancel: false, + loading: false, + error: null, + tasks: [selectedTask], + subagentActivity: { + rows: [], + overflowWorking: 0, + taskIds: new Set(), + nextExpiryAt: null, + }, + taskDetails: new Map(), + taskDetailErrors: new Map(), + taskDetailLoadingIds: new Set(), + cancellingTaskIds: new Set(), + finishedCollapsed: false, + onToggleCollapsed: () => undefined, + onToggleFinished: () => undefined, + onRefresh: () => undefined, + onCancel: () => undefined, + }; +} + +const taskContent = { kind: "task", taskId: "task-1" } satisfies SidebarContent; +const fileContent = { + kind: "file", + path: "notes.txt", + name: "notes.txt", + content: "Non-task detail", +} satisfies SidebarContent; + +function renderDetail(host: TaskDetailHost, content: SidebarContent, layout: SidebarLayout) { + renderChatDetailSlot({ + backgroundTasks: backgroundTasks(task("running")), + chat: { paneId: "pane-1" } as ChatProps, + content, + fullMessageLoader: null, + host: host as ChatPageHost, + layout, + transcript: {} as ChatTranscriptController, + }); +} + async function flushAsync() { await Promise.resolve(); await Promise.resolve(); @@ -60,6 +117,52 @@ afterEach(() => { }); describe("task detail transcript state", () => { + it.each([ + { + label: "the detail slot closes", + nextContent: taskContent, + nextLayout: closeSlot(openSlot({ columns: [] }, "detail"), "detail"), + }, + { + label: "the detail slot switches to a file", + nextContent: fileContent, + nextLayout: openSlot({ columns: [] }, "detail"), + }, + ])("clears transcript state when $label", ({ nextContent, nextLayout }) => { + const pending = deferred(); + const host = hostWith(vi.fn().mockReturnValue(pending.promise)); + const openDetailLayout = openSlot({ columns: [] }, "detail"); + + renderDetail(host, taskContent, openDetailLayout); + expect(host.taskDetailState).toBeDefined(); + + renderDetail(host, nextContent, nextLayout); + expect(host.taskDetailState).toBeUndefined(); + }); + + it("does not reset transcript state during stable task or non-task renders", () => { + const pending = deferred(); + const request = vi.fn().mockReturnValue(pending.promise); + const host = hostWith(request); + const openDetailLayout = openSlot({ columns: [] }, "detail"); + const reset = vi.spyOn(taskDetailState, "resetTaskDetail"); + + renderDetail(host, taskContent, openDetailLayout); + const openTaskState = host.taskDetailState; + renderDetail(host, taskContent, openDetailLayout); + + expect(host.taskDetailState).toBe(openTaskState); + expect(request).toHaveBeenCalledOnce(); + expect(reset).not.toHaveBeenCalled(); + + renderDetail(host, fileContent, openDetailLayout); + expect(host.taskDetailState).toBeUndefined(); + expect(reset).toHaveBeenCalledOnce(); + + renderDetail(host, fileContent, openDetailLayout); + expect(reset).toHaveBeenCalledOnce(); + }); + it("loads the selected child transcript", async () => { const pending = deferred>(); const request = vi.fn().mockReturnValue(pending.promise);