Skip to content

Commit 32f6e84

Browse files
roblourensroblourens
authored
Collapse 'used references' when content loads (microsoft#234089)
* Better scroll strategy Avoid sometimes not scrolling down all the way * Collapse 'used references' when content loads
1 parent 5c98072 commit 32f6e84

File tree

4 files changed

+33
-32
lines changed

4 files changed

+33
-32
lines changed

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ import { IChatVariablesService } from '../../common/chatVariables.js';
4242
import { IChatRendererContent, IChatResponseViewModel } from '../../common/chatViewModel.js';
4343
import { ChatTreeItem, IChatWidgetService } from '../chat.js';
4444
import { IDisposableReference, ResourcePool } from './chatCollections.js';
45-
import { IChatContentPart } from './chatContentParts.js';
45+
import { IChatContentPart, IChatContentPartRenderContext } from './chatContentParts.js';
4646

4747
const $ = dom.$;
4848

@@ -60,10 +60,12 @@ export class ChatCollapsibleListContentPart extends Disposable implements IChatC
6060
private readonly _onDidChangeHeight = this._register(new Emitter<void>());
6161
public readonly onDidChangeHeight = this._onDidChangeHeight.event;
6262

63+
private readonly hasFollowingContent: boolean;
64+
6365
constructor(
6466
private readonly data: ReadonlyArray<IChatCollapsibleListItem>,
6567
labelOverride: string | undefined,
66-
element: IChatResponseViewModel,
68+
context: IChatContentPartRenderContext,
6769
contentReferencesListPool: CollapsibleListPool,
6870
@IOpenerService openerService: IOpenerService,
6971
@IMenuService menuService: IMenuService,
@@ -72,6 +74,8 @@ export class ChatCollapsibleListContentPart extends Disposable implements IChatC
7274
) {
7375
super();
7476

77+
const element = context.element as IChatResponseViewModel;
78+
this.hasFollowingContent = context.contentIndex + 1 < context.content.length;
7579
const referencesLabel = labelOverride ?? (data.length > 1 ?
7680
localize('usedReferencesPlural', "Used {0} references", data.length) :
7781
localize('usedReferencesSingular', "Used {0} reference", 1));
@@ -163,8 +167,7 @@ export class ChatCollapsibleListContentPart extends Disposable implements IChatC
163167
}
164168

165169
hasSameContent(other: IChatRendererContent, followingContent: IChatRendererContent[], element: ChatTreeItem): boolean {
166-
return other.kind === 'references' && other.references.length === this.data.length ||
167-
other.kind === 'codeCitations' && other.citations.length === this.data.length;
170+
return other.kind === 'references' && other.references.length === this.data.length && (!!followingContent.length === this.hasFollowingContent);
168171
}
169172

170173
private updateAriaLabel(element: HTMLElement, label: string, expanded?: boolean): void {

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ import { Event } from '../../../../../base/common/event.js';
88
import { Disposable, IDisposable } from '../../../../../base/common/lifecycle.js';
99
import { MarkdownRenderer } from '../../../../../editor/browser/widget/markdownRenderer/browser/markdownRenderer.js';
1010
import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
11+
import { IChatProgressRenderableResponseContent } from '../../common/chatModel.js';
12+
import { IChatTask } from '../../common/chatService.js';
1113
import { IChatContentPart, IChatContentPartRenderContext } from './chatContentParts.js';
1214
import { ChatProgressContentPart } from './chatProgressContentPart.js';
1315
import { ChatCollapsibleListContentPart, CollapsibleListPool } from './chatReferencesContentPart.js';
14-
import { IChatProgressRenderableResponseContent } from '../../common/chatModel.js';
15-
import { IChatTask } from '../../common/chatService.js';
16-
import { IChatResponseViewModel } from '../../common/chatViewModel.js';
1716

1817
export class ChatTaskContentPart extends Disposable implements IChatContentPart {
1918
public readonly domNode: HTMLElement;
@@ -29,7 +28,7 @@ export class ChatTaskContentPart extends Disposable implements IChatContentPart
2928
super();
3029

3130
if (task.progress.length) {
32-
const refsPart = this._register(instantiationService.createInstance(ChatCollapsibleListContentPart, task.progress, task.content.value, context.element as IChatResponseViewModel, contentReferencesListPool));
31+
const refsPart = this._register(instantiationService.createInstance(ChatCollapsibleListContentPart, task.progress, task.content.value, context, contentReferencesListPool));
3332
this.domNode = dom.$('.chat-progress-task');
3433
this.domNode.appendChild(refsPart.domNode);
3534
this.onDidChangeHeight = refsPart.onDidChangeHeight;

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
806806
} else if (content.kind === 'references') {
807807
return this.renderContentReferencesListData(content, undefined, context, templateData);
808808
} else if (content.kind === 'codeCitations') {
809-
return this.renderCodeCitationsListData(content, context, templateData);
809+
return this.renderCodeCitations(content, context, templateData);
810810
} else if (content.kind === 'toolInvocation' || content.kind === 'toolInvocationSerialized') {
811811
return this.renderToolInvocation(content, context, templateData);
812812
}
@@ -847,15 +847,15 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
847847
}
848848

849849
private renderContentReferencesListData(references: IChatReferences, labelOverride: string | undefined, context: IChatContentPartRenderContext, templateData: IChatListItemTemplate): ChatCollapsibleListContentPart {
850-
const referencesPart = this.instantiationService.createInstance(ChatCollapsibleListContentPart, references.references, labelOverride, context.element as IChatResponseViewModel, this._contentReferencesListPool);
850+
const referencesPart = this.instantiationService.createInstance(ChatCollapsibleListContentPart, references.references, labelOverride, context, this._contentReferencesListPool);
851851
referencesPart.addDisposable(referencesPart.onDidChangeHeight(() => {
852852
this.updateItemHeight(templateData);
853853
}));
854854

855855
return referencesPart;
856856
}
857857

858-
private renderCodeCitationsListData(citations: IChatCodeCitations, context: IChatContentPartRenderContext, templateData: IChatListItemTemplate): ChatCodeCitationContentPart {
858+
private renderCodeCitations(citations: IChatCodeCitations, context: IChatContentPartRenderContext, templateData: IChatListItemTemplate): ChatCodeCitationContentPart {
859859
const citationsPart = this.instantiationService.createInstance(ChatCodeCitationContentPart, citations, context);
860860
return citationsPart;
861861
}

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

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,6 @@ import { ChatViewWelcomePart } from './viewsWelcome/chatViewWelcomeController.js
5858

5959
const $ = dom.$;
6060

61-
function revealLastElement(list: WorkbenchObjectTree<any>) {
62-
const newScrollTop = list.scrollHeight - list.renderHeight;
63-
list.scrollTop = newScrollTop;
64-
}
65-
6661
export interface IChatViewState {
6762
inputValue?: string;
6863
inputState?: IChatInputState;
@@ -146,6 +141,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
146141
private tree!: WorkbenchObjectTree<ChatTreeItem>;
147142
private renderer!: ChatListItemRenderer;
148143
private readonly _codeBlockModelCollection: CodeBlockModelCollection;
144+
private lastItem: ChatTreeItem | undefined;
149145

150146
private inputPart!: ChatInputPart;
151147
private editorOptions!: ChatEditorOptions;
@@ -434,7 +430,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
434430
scrollDownButton.setTitle(localize('scrollDownButtonLabel', "Scroll down"));
435431
this._register(scrollDownButton.onDidClick(() => {
436432
this.scrollLock = true;
437-
revealLastElement(this.tree);
433+
this.scrollToEnd();
438434
}));
439435

440436
this._register(this.editorOptions.onDidChange(() => this.onDidStyleChange()));
@@ -443,7 +439,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
443439
// Do initial render
444440
if (this.viewModel) {
445441
this.onDidChangeItems();
446-
revealLastElement(this.tree);
442+
this.scrollToEnd();
447443
}
448444

449445
this.contribs = ChatWidget.CONTRIBS.map(contrib => {
@@ -459,6 +455,13 @@ export class ChatWidget extends Disposable implements IChatWidget {
459455
this._register((this.chatWidgetService as ChatWidgetService).register(this));
460456
}
461457

458+
private scrollToEnd() {
459+
if (this.lastItem) {
460+
const offset = Math.max(this.lastItem.currentRenderedHeight ?? 0, 1e6);
461+
this.tree.reveal(this.lastItem, offset);
462+
}
463+
}
464+
462465
getContrib<T extends IChatWidgetContrib>(id: string): T | undefined {
463466
return this.contribs.find(c => c.id === id) as T;
464467
}
@@ -545,12 +548,12 @@ export class ChatWidget extends Disposable implements IChatWidget {
545548
this.layoutDynamicChatTreeItemMode();
546549
}
547550

548-
const lastItem = treeItems[treeItems.length - 1]?.element;
549-
if (lastItem) {
550-
ChatContextKeys.lastItemId.bindTo(this.contextKeyService).set([lastItem.id]);
551+
this.lastItem = treeItems[treeItems.length - 1]?.element;
552+
if (this.lastItem) {
553+
ChatContextKeys.lastItemId.bindTo(this.contextKeyService).set([this.lastItem.id]);
551554
}
552-
if (lastItem && isResponseVM(lastItem) && lastItem.isComplete) {
553-
this.renderFollowups(lastItem.replyFollowups, lastItem);
555+
if (this.lastItem && isResponseVM(this.lastItem) && this.lastItem.isComplete) {
556+
this.renderFollowups(this.lastItem.replyFollowups, this.lastItem);
554557
} else if (!treeItems.length && this.viewModel) {
555558
this.renderFollowups(this.viewModel.model.sampleQuestions);
556559
} else {
@@ -738,10 +741,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
738741
dom.scheduleAtNextAnimationFrame(dom.getWindow(this.listContainer), () => {
739742
// Can't set scrollTop during this event listener, the list might overwrite the change
740743

741-
// TODO This doesn't necessarily work on the first try because of the dynamic heights of items and
742-
// the way ListView works. Twice is usually enough. But this would work better if this lived inside ListView somehow
743-
revealLastElement(this.tree);
744-
revealLastElement(this.tree);
744+
this.scrollToEnd();
745745
}, 0);
746746
}
747747
}
@@ -868,7 +868,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
868868

869869
this.onDidChangeItems();
870870
if (events.some(e => e?.kind === 'addRequest') && this.visible) {
871-
revealLastElement(this.tree); // Now we know how big they actually are... how do we know that 2 rounds is enough
871+
this.scrollToEnd();
872872
}
873873

874874
if (this.chatEditingService.currentEditingSession && this.chatEditingService.currentEditingSession?.chatSessionId === this.viewModel?.sessionId) {
@@ -903,7 +903,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
903903

904904
if (this.tree && this.visible) {
905905
this.onDidChangeItems();
906-
revealLastElement(this.tree);
906+
this.scrollToEnd();
907907
}
908908

909909
this.updateChatInputContext();
@@ -1138,7 +1138,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
11381138
const lastItem = this.viewModel?.getItems().at(-1);
11391139
const lastResponseIsRendering = isResponseVM(lastItem) && lastItem.renderData;
11401140
if (lastElementVisible && (!lastResponseIsRendering || this.viewOptions.autoScroll)) {
1141-
revealLastElement(this.tree);
1141+
this.scrollToEnd();
11421142
}
11431143

11441144
this.listContainer.style.height = `${listHeight}px`;
@@ -1241,8 +1241,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
12411241
);
12421242

12431243
if (needsRerender || !listHeight) {
1244-
// TODO: figure out a better place to reveal the last element
1245-
revealLastElement(this.tree);
1244+
this.scrollToEnd();
12461245
}
12471246
}
12481247

0 commit comments

Comments
 (0)