fix: reduce codeblock pill flickering in chat editing response

microsoft · Oct 3, 2024 · 6e92a2d · 6e92a2d
1 parent 9ae5f5a
commit 6e92a2d
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 41 deletions.
diff --git a/src/vs/workbench/contrib/chat/browser/chat.ts b/src/vs/workbench/contrib/chat/browser/chat.ts
@@ -105,6 +105,7 @@ export interface IChatListItemRendererOptions {
 	readonly noHeader?: boolean;
 	readonly noPadding?: boolean;
 	readonly editableCodeBlock?: boolean;
+	readonly collapseCodeBlocks?: boolean;
 	readonly renderTextEditsAsSummary?: (uri: URI) => boolean;
 }
 

diff --git a/src/vs/workbench/contrib/chat/browser/chatContentParts/chatMarkdownContentPart.ts b/src/vs/workbench/contrib/chat/browser/chatContentParts/chatMarkdownContentPart.ts
@@ -36,7 +36,7 @@ export class ChatMarkdownContentPart extends Disposable implements IChatContentP
 	private static idPool = 0;
 	public readonly id = String(++ChatMarkdownContentPart.idPool);
 	public readonly domNode: HTMLElement;
-	private readonly allRefs: IDisposableReference<CodeBlockPart>[] = [];
+	private readonly allRefs: IDisposableReference<CodeBlockPart | { object?: InlineAnchorWidget; element: HTMLElement }>[] = [];
 
 	private readonly _onDidChangeHeight = this._register(new Emitter<void>());
 	public readonly onDidChangeHeight = this._onDidChangeHeight.event;
@@ -92,34 +92,68 @@ export class ChatMarkdownContentPart extends Disposable implements IChatContentP
 				}
 
 				const hideToolbar = isResponseVM(element) && element.errorDetails?.responseIsFiltered;
-				const ref = this.renderCodeBlock({ languageId, textModel, codeBlockIndex: index, element, range, hideToolbar, parentContextKeyService: contextKeyService, vulns, codemapperUri }, text, currentWidth, rendererOptions.editableCodeBlock);
-				this.allRefs.push(ref);
-
-				// Attach this after updating text/layout of the editor, so it should only be fired when the size updates later (horizontal scrollbar, wrapping)
-				// not during a renderElement OR a progressive render (when we will be firing this event anyway at the end of the render)
-				this._register(ref.object.onDidChangeContentHeight(() => this._onDidChangeHeight.fire()));
-
-				const ownerMarkdownPartId = this.id;
-				const info: IChatCodeBlockInfo = new class {
-					readonly ownerMarkdownPartId = ownerMarkdownPartId;
-					readonly codeBlockIndex = index;
-					readonly element = element;
-					codemapperUri = undefined; // will be set async
-					public get uri() {
-						// here we must do a getter because the ref.object is rendered
-						// async and the uri might be undefined when it's read immediately
-						return ref.object.uri;
-					}
-					public focus() {
-						ref.object.focus();
-					}
-					public getContent(): string {
-						return ref.object.editor.getValue();
+				const codeBlockInfo = { languageId, textModel, codeBlockIndex: index, element, range, hideToolbar, parentContextKeyService: contextKeyService, vulns, codemapperUri };
+
+				if (!rendererOptions.collapseCodeBlocks) {
+					const ref = this.renderCodeBlock(codeBlockInfo, text, currentWidth, rendererOptions.editableCodeBlock);
+					this.allRefs.push(ref);
+
+					// Attach this after updating text/layout of the editor, so it should only be fired when the size updates later (horizontal scrollbar, wrapping)
+					// not during a renderElement OR a progressive render (when we will be firing this event anyway at the end of the render)
+					this._register(ref.object.onDidChangeContentHeight(() => this._onDidChangeHeight.fire()));
+
+					const ownerMarkdownPartId = this.id;
+					const info: IChatCodeBlockInfo = new class {
+						readonly ownerMarkdownPartId = ownerMarkdownPartId;
+						readonly codeBlockIndex = index;
+						readonly element = element;
+						codemapperUri = undefined; // will be set async
+						public get uri() {
+							// here we must do a getter because the ref.object is rendered
+							// async and the uri might be undefined when it's read immediately
+							return ref.object.uri;
+						}
+						public focus() {
+							ref.object.focus();
+						}
+						public getContent(): string {
+							return ref.object.editor.getValue();
+						}
+					}();
+					this.codeblocks.push(info);
+					orderedDisposablesList.push(ref);
+					return ref.object.element;
+				} else {
+					const ref = this.renderCodeBlockPill(codeBlockInfo.codemapperUri, undefined);
+					if (isResponseVM(codeBlockInfo.element)) {
+						// TODO@joyceerhl: remove this code when we change the codeblockUri API to make the URI available synchronously
+						this.codeBlockModelCollection.update(codeBlockInfo.element.sessionId, codeBlockInfo.element, codeBlockInfo.codeBlockIndex, { text, languageId: codeBlockInfo.languageId }).then((e) => {
+							// Update the existing object's codemapperUri
+							this.codeblocks[codeBlockInfo.codeBlockIndex].codemapperUri = e.codemapperUri;
+							this._onDidChangeHeight.fire();
+						});
 					}
-				}();
-				this.codeblocks.push(info);
-				orderedDisposablesList.push(ref);
-				return ref.object.element;
+					this.allRefs.push(ref);
+					const ownerMarkdownPartId = this.id;
+					const info: IChatCodeBlockInfo = new class {
+						readonly ownerMarkdownPartId = ownerMarkdownPartId;
+						readonly codeBlockIndex = index;
+						readonly element = element;
+						codemapperUri = undefined; // will be set async
+						public get uri() {
+							return undefined;
+						}
+						public focus() {
+							return ref.object.element.focus();
+						}
+						public getContent(): string {
+							return ''; // Not needed for collapsed code blocks
+						}
+					}();
+					this.codeblocks.push(info);
+					orderedDisposablesList.push(ref);
+					return ref.object.element;
+				}
 			},
 			asyncRenderCallback: () => this._onDidChangeHeight.fire(),
 		}));
@@ -130,6 +164,24 @@ export class ChatMarkdownContentPart extends Disposable implements IChatContentP
 		this.domNode = result.element;
 	}
 
+	private renderCodeBlockPill(codemapperUri: URI | undefined, anchor: HTMLElement | undefined): IDisposableReference<{ object?: InlineAnchorWidget; element: HTMLElement }> {
+		const fileWidgetAnchor = anchor ?? $('.chat-codeblock');
+		if (codemapperUri) {
+			const inlineAnchor = this._register(this.instantiationService.createInstance(InlineAnchorWidget, fileWidgetAnchor, { uri: codemapperUri }, { handleClick: (uri) => this.editorService.openEditor({ resource: uri }) }));
+			this._register(this.chatMarkdownAnchorService.register(inlineAnchor));
+			return {
+				object: { object: inlineAnchor, element: fileWidgetAnchor },
+				isStale() { return false; },
+				dispose() { }
+			};
+		}
+		return {
+			object: { object: undefined, element: fileWidgetAnchor },
+			isStale() { return false; },
+			dispose() { }
+		};
+	}
+
 	private renderCodeBlock(data: ICodeBlockData, text: string, currentWidth: number, editableCodeBlock: boolean | undefined): IDisposableReference<CodeBlockPart> {
 		const ref = this.editorPool.get();
 		const editorInfo = ref.object;
@@ -152,18 +204,14 @@ export class ChatMarkdownContentPart extends Disposable implements IChatContentP
 
 	layout(width: number): void {
 		this.allRefs.forEach((ref, index) => {
-			const codeblockModel = this.codeblocks[index];
-			if (codeblockModel.codemapperUri) {
-				const fileWidgetAnchor = $('.chat-codeblock');
-				const inlineAnchor = this._register(this.instantiationService.createInstance(InlineAnchorWidget, fileWidgetAnchor, { uri: codeblockModel.codemapperUri }, { handleClick: (uri) => this.editorService.openEditor({ resource: uri }) }));
-				this._register(this.chatMarkdownAnchorService.register(inlineAnchor));
-				const existingCodeblock = ref.object.element.parentElement?.querySelector('.chat-codeblock');
-				if (!existingCodeblock) {
-					ref.object.element.parentElement?.appendChild(fileWidgetAnchor);
-					ref.object.element.style.display = 'none';
-				}
-			} else {
+			if (ref.object instanceof CodeBlockPart) {
 				ref.object.layout(width);
+			} else if (ref.object.element && !ref.object.object) {
+				const codeblockModel = this.codeblocks[index];
+				if (codeblockModel.codemapperUri) {
+					const pill = this.renderCodeBlockPill(codeblockModel.codemapperUri, ref.object.element);
+					this.allRefs[index] = pill;
+				}
 			}
 		});
 	}

diff --git a/src/vs/workbench/contrib/chat/browser/chatViewPane.ts b/src/vs/workbench/contrib/chat/browser/chatViewPane.ts
@@ -156,13 +156,14 @@ export class ChatViewPane extends ViewPane {
 				ChatWidget,
 				this.chatOptions.location,
 				{ viewId: this.id },
-				{ supportsFileReferences: true, supportsAdditionalParticipants: this.chatOptions.location === ChatAgentLocation.Panel },
+				{ supportsFileReferences: true, supportsAdditionalParticipants: this.chatOptions.location === ChatAgentLocation.Panel, rendererOptions: { collapseCodeBlocks: this.chatOptions.location === ChatAgentLocation.EditingSession } },
 				{
 					listForeground: SIDE_BAR_FOREGROUND,
 					listBackground: locationBasedColors.background,
 					overlayBackground: locationBasedColors.overlayBackground,
 					inputEditorBackground: locationBasedColors.background,
-					resultEditorBackground: editorBackground
+					resultEditorBackground: editorBackground,
+
 				}));
 			this._register(this.onDidChangeBodyVisibility(visible => {
 				this._widget.setVisible(visible);