Skip to content

Commit d612de4

Browse files
mjbvzmjbvz
authored
Merge pull request microsoft#257874 from mjbvz/splendid-skink
Polish output renderer
2 parents 1a1212a + 7083fc3 commit d612de4

File tree

7 files changed

+116
-17
lines changed

7 files changed

+116
-17
lines changed

‎src/vs/workbench/contrib/chat/browser/chat.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ export interface IChatWidget {
177177
readonly onDidChangeViewModel: Event<void>;
178178
readonly onDidAcceptInput: Event<void>;
179179
readonly onDidHide: Event<void>;
180+
readonly onDidShow: Event<void>;
180181
readonly onDidSubmitAgent: Event<{ agent: IChatAgentData; slashCommand?: IChatAgentCommand }>;
181182
readonly onDidChangeAgent: Event<{ agent: IChatAgentData; slashCommand?: IChatAgentCommand }>;
182183
readonly onDidChangeParsedInput: Event<void>;

‎src/vs/workbench/contrib/chat/browser/chatContentParts/toolInvocationParts/chatToolOutputPart.ts

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,34 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as dom from '../../../../../../base/browser/dom.js';
7+
import { renderMarkdown } from '../../../../../../base/browser/markdownRenderer.js';
78
import { decodeBase64 } from '../../../../../../base/common/buffer.js';
89
import { CancellationTokenSource } from '../../../../../../base/common/cancellation.js';
910
import { Codicon } from '../../../../../../base/common/codicons.js';
1011
import { ThemeIcon } from '../../../../../../base/common/themables.js';
12+
import { generateUuid } from '../../../../../../base/common/uuid.js';
1113
import { localize } from '../../../../../../nls.js';
1214
import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js';
1315
import { IChatToolInvocation, IChatToolInvocationSerialized, IToolResultOutputDetailsSerialized } from '../../../common/chatService.js';
16+
import { IChatViewModel } from '../../../common/chatViewModel.js';
1417
import { IToolResultOutputDetails } from '../../../common/languageModelToolsService.js';
1518
import { IChatCodeBlockInfo, IChatWidgetService } from '../../chat.js';
1619
import { IChatOutputRendererService } from '../../chatOutputItemRenderer.js';
1720
import { IChatContentPartRenderContext } from '../chatContentParts.js';
1821
import { ChatCustomProgressPart } from '../chatProgressContentPart.js';
1922
import { BaseChatToolInvocationSubPart } from './chatToolInvocationSubPart.js';
2023

24+
interface OutputState {
25+
readonly webviewOrigin: string;
26+
height: number;
27+
}
28+
2129
// TODO: see if we can reuse existing types instead of adding ChatToolOutputSubPart
2230
export class ChatToolOutputSubPart extends BaseChatToolInvocationSubPart {
31+
32+
/** Remembers cached state on re-render */
33+
private static readonly _cachedStates = new WeakMap<IChatViewModel | IChatToolInvocationSerialized, Map<string, OutputState>>();
34+
2335
public readonly domNode: HTMLElement;
2436

2537
public override readonly codeblocks: IChatCodeBlockInfo[] = [];
@@ -45,26 +57,58 @@ export class ChatToolOutputSubPart extends BaseChatToolInvocationSubPart {
4557
},
4658
};
4759

48-
this.domNode = this.createOutputPart(details);
60+
this.domNode = dom.$('div.tool-output-part');
61+
62+
const titleEl = dom.$('.output-title');
63+
this.domNode.appendChild(titleEl);
64+
if (typeof toolInvocation.invocationMessage === 'string') {
65+
titleEl.textContent = toolInvocation.invocationMessage;
66+
} else {
67+
const md = this._register(renderMarkdown(toolInvocation.invocationMessage));
68+
titleEl.appendChild(md.element);
69+
}
70+
71+
this.domNode.appendChild(this.createOutputPart(toolInvocation, details));
4972
}
5073

5174
public override dispose(): void {
5275
this._disposeCts.dispose(true);
5376
super.dispose();
5477
}
5578

56-
private createOutputPart(details: IToolResultOutputDetails): HTMLElement {
79+
private createOutputPart(toolInvocation: IChatToolInvocation | IChatToolInvocationSerialized, details: IToolResultOutputDetails): HTMLElement {
80+
const vm = this.chatWidgetService.getWidgetBySessionId(this.context.element.sessionId)?.viewModel;
81+
5782
const parent = dom.$('div.webview-output');
5883
parent.style.maxHeight = '80vh';
59-
// TODO: we should cache the height when restoring to avoid extra layout shifts
84+
85+
let partState: OutputState = { height: 0, webviewOrigin: generateUuid() };
86+
if (vm) {
87+
let allStates = ChatToolOutputSubPart._cachedStates.get(vm);
88+
if (!allStates) {
89+
allStates = new Map<string, OutputState>();
90+
ChatToolOutputSubPart._cachedStates.set(vm, allStates);
91+
}
92+
93+
const cachedState = allStates.get(toolInvocation.toolCallId);
94+
if (cachedState) {
95+
partState = cachedState;
96+
} else {
97+
allStates.set(toolInvocation.toolCallId, partState);
98+
}
99+
}
100+
101+
if (partState.height) {
102+
parent.style.height = `${partState.height}px`;
103+
}
60104

61105
const progressMessage = dom.$('span');
62106
progressMessage.textContent = localize('loading', 'Rendering tool output...');
63107
const progressPart = this.instantiationService.createInstance(ChatCustomProgressPart, progressMessage, ThemeIcon.modify(Codicon.loading, 'spin'));
64108
parent.appendChild(progressPart.domNode);
65109

66110
// TODO: we also need to show the tool output in the UI
67-
this.chatOutputItemRendererService.renderOutputPart(details.output.mimeType, details.output.value.buffer, parent, this._disposeCts.token).then((renderedItem) => {
111+
this.chatOutputItemRendererService.renderOutputPart(details.output.mimeType, details.output.value.buffer, parent, { origin: partState.webviewOrigin }, this._disposeCts.token).then((renderedItem) => {
68112
if (this._disposeCts.token.isCancellationRequested) {
69113
return;
70114
}
@@ -74,8 +118,9 @@ export class ChatToolOutputSubPart extends BaseChatToolInvocationSubPart {
74118
progressPart.domNode.remove();
75119

76120
this._onDidChangeHeight.fire();
77-
this._register(renderedItem.onDidChangeHeight(() => {
121+
this._register(renderedItem.onDidChangeHeight(newHeight => {
78122
this._onDidChangeHeight.fire();
123+
partState.height = newHeight;
79124
}));
80125

81126
this._register(renderedItem.webview.onDidWheel(e => {
@@ -85,6 +130,14 @@ export class ChatToolOutputSubPart extends BaseChatToolInvocationSubPart {
85130
stopPropagation: () => { }
86131
});
87132
}));
133+
134+
// When the webview is disconnected from the DOM due to being hidden, we need to reload it when it is shown again.
135+
const widget = this.chatWidgetService.getWidgetBySessionId(this.context.element.sessionId);
136+
if (widget) {
137+
this._register(widget?.onDidShow(() => {
138+
renderedItem.reinitialize();
139+
}));
140+
}
88141
}, (error) => {
89142
// TODO: show error in UI too
90143
console.error('Error rendering tool output:', error);

‎src/vs/workbench/contrib/chat/browser/chatOutputItemRenderer.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,28 +21,35 @@ export interface IChatOutputItemRenderer {
2121
renderOutputPart(mime: string, data: Uint8Array, webview: IWebview, token: CancellationToken): Promise<void>;
2222
}
2323

24-
export const IChatOutputRendererService = createDecorator<IChatOutputRendererService>('chatOutputRendererService');
25-
2624
interface RegisterOptions {
2725
readonly extension?: {
2826
readonly id: ExtensionIdentifier;
2927
readonly location: URI;
3028
};
3129
}
3230

33-
export interface RenderedOutputPart extends IDisposable {
34-
readonly onDidChangeHeight: Event<number>;
35-
readonly webview: IWebview;
36-
}
31+
export const IChatOutputRendererService = createDecorator<IChatOutputRendererService>('chatOutputRendererService');
3732

3833
export interface IChatOutputRendererService {
3934
readonly _serviceBrand: undefined;
4035

4136
registerRenderer(mime: string, renderer: IChatOutputItemRenderer, options: RegisterOptions): IDisposable;
4237

43-
renderOutputPart(mime: string, data: Uint8Array, parent: HTMLElement, token: CancellationToken): Promise<RenderedOutputPart>;
38+
renderOutputPart(mime: string, data: Uint8Array, parent: HTMLElement, webviewOptions: RenderOutputPartWebviewOptions, token: CancellationToken): Promise<RenderedOutputPart>;
39+
}
40+
41+
export interface RenderedOutputPart extends IDisposable {
42+
readonly onDidChangeHeight: Event<number>;
43+
readonly webview: IWebview;
44+
45+
reinitialize(): void;
46+
}
47+
48+
interface RenderOutputPartWebviewOptions {
49+
readonly origin?: string;
4450
}
4551

52+
4653
export class ChatOutputRendererService extends Disposable implements IChatOutputRendererService {
4754
_serviceBrand: undefined;
4855

@@ -67,7 +74,7 @@ export class ChatOutputRendererService extends Disposable implements IChatOutput
6774
};
6875
}
6976

70-
async renderOutputPart(mime: string, data: Uint8Array, parent: HTMLElement, token: CancellationToken): Promise<RenderedOutputPart> {
77+
async renderOutputPart(mime: string, data: Uint8Array, parent: HTMLElement, webviewOptions: RenderOutputPartWebviewOptions, token: CancellationToken): Promise<RenderedOutputPart> {
7178
// Activate extensions that contribute to chatOutputRenderer for this mime type
7279
await this._extensionService.activateByEvent(`onChatOutputRenderer:${mime}`);
7380

@@ -80,7 +87,7 @@ export class ChatOutputRendererService extends Disposable implements IChatOutput
8087

8188
const webview = store.add(this._webviewService.createWebviewElement({
8289
title: '',
83-
origin: generateUuid(),
90+
origin: webviewOptions.origin ?? generateUuid(),
8491
options: {
8592
enableFindWidget: false,
8693
purpose: WebviewContentPurpose.ChatOutputItem,
@@ -103,11 +110,14 @@ export class ChatOutputRendererService extends Disposable implements IChatOutput
103110
await rendererData.renderer.renderOutputPart(mime, data, webview, token);
104111

105112
return {
106-
webview,
113+
get webview() { return webview; },
107114
onDidChangeHeight: onDidChangeHeight.event,
108115
dispose: () => {
109116
store.dispose();
110-
}
117+
},
118+
reinitialize: () => {
119+
webview.reinitializeAfterDismount();
120+
},
111121
};
112122
}
113123
}
@@ -144,3 +154,4 @@ ExtensionsRegistry.registerExtensionPoint<IChatOutputRendererContribution[]>({
144154
}
145155
}
146156
});
157+

‎src/vs/workbench/contrib/chat/browser/chatWidget.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,9 @@ export class ChatWidget extends Disposable implements IChatWidget {
147147
private _onDidHide = this._register(new Emitter<void>());
148148
readonly onDidHide = this._onDidHide.event;
149149

150+
private _onDidShow = this._register(new Emitter<void>());
151+
readonly onDidShow = this._onDidShow.event;
152+
150153
private _onDidChangeParsedInput = this._register(new Emitter<void>());
151154
readonly onDidChangeParsedInput = this._onDidChangeParsedInput.event;
152155

@@ -977,6 +980,12 @@ export class ChatWidget extends Disposable implements IChatWidget {
977980
this.onDidChangeItems(true);
978981
}
979982
}, 0));
983+
984+
if (!wasVisible) {
985+
dom.scheduleAtNextAnimationFrame(dom.getWindow(this.listContainer), () => {
986+
this._onDidShow.fire();
987+
});
988+
}
980989
} else if (wasVisible) {
981990
this._onDidHide.fire();
982991
}

‎src/vs/workbench/contrib/chat/browser/media/chat.css

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,21 @@
466466
display: inherit;
467467
}
468468

469+
.tool-output-part {
470+
border: 1px solid var(--vscode-widget-border);
471+
border-radius: 6px;
472+
background: var(--vscode-editor-background);
473+
margin: 4px 0;
474+
overflow: hidden;
475+
476+
.output-title {
477+
font-size: 13px;
478+
padding: 8px 12px;
479+
background: var(--vscode-editorWidget-background);
480+
border-bottom: 1px solid var(--vscode-widget-border);
481+
}
482+
}
483+
469484
&:not(:last-child) {
470485
margin-bottom: 8px;
471486
}

‎src/vs/workbench/contrib/webview/browser/webview.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ export interface IWebviewElement extends IWebview {
286286
* @param parent Element to append the webview to.
287287
*/
288288
mountTo(parent: HTMLElement, targetWindow: CodeWindow): void;
289+
290+
reinitializeAfterDismount(): void;
289291
}
290292

291293
/**

‎src/vs/workbench/contrib/webview/browser/webviewElement.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { isFirefox } from '../../../../base/browser/browser.js';
7-
import { addDisposableListener, EventType, getWindowById } from '../../../../base/browser/dom.js';
7+
import { addDisposableListener, EventType, getWindow, getWindowById } from '../../../../base/browser/dom.js';
88
import { parentOriginHash } from '../../../../base/browser/iframe.js';
99
import { IMouseWheelEvent } from '../../../../base/browser/mouseEvent.js';
1010
import { CodeWindow } from '../../../../base/browser/window.js';
@@ -596,6 +596,14 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD
596596
this.doUpdateContent(this._content);
597597
}
598598

599+
public reinitializeAfterDismount(): void {
600+
this._state = new WebviewState.Initializing([]);
601+
this._messagePort = undefined;
602+
603+
this.mountTo(this.element!.parentElement!, getWindow(this.element));
604+
this.reload();
605+
}
606+
599607
public setHtml(html: string) {
600608
this.doUpdateContent({ ...this._content, html });
601609
this._onDidHtmlChange.fire(html);

0 commit comments

Comments
 (0)