Skip to content

Commit

Permalink
Improve chat scrolling (#231370)
Browse files Browse the repository at this point in the history
* Don't auto-scroll while chat response loads
Fix microsoft/vscode-copilot-release#138

* Show button all the time

* Fixes, don't auto-scroll for edit session view

* Keep autoscroll in inline and quick chat
  • Loading branch information
roblourens authored Oct 15, 2024
1 parent edeada8 commit 8d8f0cd
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 22 deletions.
1 change: 1 addition & 0 deletions build/lib/stylelint/vscode-known-variables.json
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,7 @@
"others": [
"--background-dark",
"--background-light",
"--chat-current-response-min-height",
"--dropdown-padding-bottom",
"--dropdown-padding-top",
"--insert-border-color",
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/chat/browser/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export interface IChatListItemRendererOptions {
}

export interface IChatWidgetViewOptions {
autoScroll?: boolean;
renderInputOnTop?: boolean;
renderFollowups?: boolean;
renderStyle?: 'compact' | 'minimal';
Expand Down
17 changes: 12 additions & 5 deletions src/vs/workbench/contrib/chat/browser/chatListRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,14 @@ const forceVerboseLayoutTracing = false
;

export interface IChatRendererDelegate {
container: HTMLElement;
getListLength(): number;

readonly onDidScroll?: Event<void>;
}

const mostRecentResponseClassName = 'chat-most-recent-response';

export class ChatListItemRenderer extends Disposable implements ITreeRenderer<ChatTreeItem, FuzzyScore, IChatListItemTemplate> {
static readonly ID = 'item';

Expand Down Expand Up @@ -406,8 +409,9 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
// - And it has some content
// - And the response is not complete
// - Or, we previously started a progressive rendering of this element (if the element is complete, we will finish progressive rendering with a very fast rate)
if (isResponseVM(element) && index === this.delegate.getListLength() - 1 && (!element.isComplete || element.renderData) && element.response.value.length) {
if (isResponseVM(element) && index === this.delegate.getListLength() - 1 && (!element.isComplete || element.renderData)) {
this.traceLayout('renderElement', `start progressive render, index=${index}`);
templateData.rowContainer.classList.toggle(mostRecentResponseClassName, true);

const timer = templateData.elementDisposables.add(new dom.WindowIntervalTimer());
const runProgressiveRender = (initial?: boolean) => {
Expand All @@ -423,10 +427,13 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
};
timer.cancelAndSet(runProgressiveRender, 50, dom.getWindow(templateData.rowContainer));
runProgressiveRender(true);
} else if (isResponseVM(element)) {
this.basicRenderElement(element, index, templateData);
} else if (isRequestVM(element)) {
this.basicRenderElement(element, index, templateData);
} else {
templateData.rowContainer.classList.toggle(mostRecentResponseClassName, false);
if (isResponseVM(element)) {
this.basicRenderElement(element, index, templateData);
} else if (isRequestVM(element)) {
this.basicRenderElement(element, index, templateData);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/contrib/chat/browser/chatQuick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class QuickChat extends Disposable {
ChatWidget,
ChatAgentLocation.Panel,
{ isQuickChat: true },
{ renderInputOnTop: true, renderStyle: 'compact', menus: { inputSideToolbar: MenuId.ChatInputSide } },
{ autoScroll: true, renderInputOnTop: true, renderStyle: 'compact', menus: { inputSideToolbar: MenuId.ChatInputSide } },
{
listForeground: quickInputForeground,
listBackground: quickInputBackground,
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/chat/browser/chatViewPane.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ export class ChatViewPane extends ViewPane implements IViewWelcomeDelegate {
this.chatOptions.location,
{ viewId: this.id },
{
autoScroll: this.chatOptions.location === ChatAgentLocation.EditingSession,
renderFollowups: this.chatOptions.location === ChatAgentLocation.Panel,
supportsFileReferences: true,
supportsAdditionalParticipants: this.chatOptions.location === ChatAgentLocation.Panel,
Expand Down
80 changes: 66 additions & 14 deletions src/vs/workbench/contrib/chat/browser/chatWidget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
*--------------------------------------------------------------------------------------------*/

import * as dom from '../../../../base/browser/dom.js';
import { Button } from '../../../../base/browser/ui/button/button.js';
import { ITreeContextMenuEvent, ITreeElement } from '../../../../base/browser/ui/tree/tree.js';
import { disposableTimeout, timeout } from '../../../../base/common/async.js';
import { Codicon } from '../../../../base/common/codicons.js';
import { toErrorMessage } from '../../../../base/common/errorMessage.js';
import { Emitter, Event } from '../../../../base/common/event.js';
import { MarkdownString } from '../../../../base/common/htmlContent.js';
Expand All @@ -26,6 +28,8 @@ import { ServiceCollection } from '../../../../platform/instantiation/common/ser
import { WorkbenchObjectTree } from '../../../../platform/list/browser/listService.js';
import { ILogService } from '../../../../platform/log/common/log.js';
import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js';
import { buttonSecondaryBackground, buttonSecondaryHoverBackground } from '../../../../platform/theme/common/colorRegistry.js';
import { asCssVariable } from '../../../../platform/theme/common/colorUtils.js';
import { IThemeService } from '../../../../platform/theme/common/themeService.js';
import { TerminalChatController } from '../../terminal/terminalContribChatExports.js';
import { ChatAgentLocation, IChatAgentCommand, IChatAgentData, IChatAgentService, IChatWelcomeMessageContent, isChatWelcomeMessageContent } from '../common/chatAgents.js';
Expand Down Expand Up @@ -54,7 +58,8 @@ import { ChatViewWelcomePart } from './viewsWelcome/chatViewWelcomeController.js
const $ = dom.$;

function revealLastElement(list: WorkbenchObjectTree<any>) {
list.scrollTop = list.scrollHeight - list.renderHeight;
const newScrollTop = list.scrollHeight - list.renderHeight;
list.scrollTop = newScrollTop;
}

export interface IChatViewState {
Expand Down Expand Up @@ -162,6 +167,12 @@ export class ChatWidget extends Disposable implements IChatWidget {

private previousTreeScrollHeight: number = 0;

/**
* Whether the list is scroll-locked to the bottom. Initialize to true so that we can scroll to the bottom on first render.
* The initial render leads to a lot of `onDidChangeTreeContentHeight` as the renderer works out the real heights of rows.
*/
private scrollLock = true;

private readonly viewModelDisposables = this._register(new DisposableStore());
private _viewModel: ChatViewModel | undefined;
private set viewModel(viewModel: ChatViewModel | undefined) {
Expand Down Expand Up @@ -405,6 +416,19 @@ export class ChatWidget extends Disposable implements IChatWidget {

this.createList(this.listContainer, { ...this.viewOptions.rendererOptions, renderStyle });

const scrollDownButton = this._register(new Button(this.listContainer, {
supportIcons: true,
buttonBackground: asCssVariable(buttonSecondaryBackground),
buttonHoverBackground: asCssVariable(buttonSecondaryHoverBackground),
}));
scrollDownButton.element.classList.add('chat-scroll-down');
scrollDownButton.label = `$(${Codicon.chevronDown.id})`;
scrollDownButton.setTitle(localize('scrollDownButtonLabel', "Scroll down"));
this._register(scrollDownButton.onDidClick(() => {
this.scrollLock = true;
revealLastElement(this.tree);
}));

this._register(this.editorOptions.onDidChange(() => this.onDidStyleChange()));
this.onDidStyleChange();

Expand Down Expand Up @@ -581,6 +605,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
const rendererDelegate: IChatRendererDelegate = {
getListLength: () => this.tree.getNode(null).visibleChildrenCount,
onDidScroll: this.onDidScroll,
container: listContainer
};

// Create a dom element to hold UI from editor widgets embedded in chat messages
Expand Down Expand Up @@ -654,6 +679,11 @@ export class ChatWidget extends Disposable implements IChatWidget {
}));
this._register(this.tree.onDidScroll(() => {
this._onDidScroll.fire();

const lastItem = this.viewModel?.getItems().at(-1);
const lastResponseIsRendering = isResponseVM(lastItem) && lastItem.renderData;
const isScrolledDown = this.tree.scrollTop >= this.tree.scrollHeight - this.tree.renderHeight - 2;
this.container.classList.toggle('show-scroll-down', !isScrolledDown && Boolean(lastResponseIsRendering));
}));
}

Expand All @@ -675,18 +705,31 @@ export class ChatWidget extends Disposable implements IChatWidget {
}

private onDidChangeTreeContentHeight(): void {
// If the list was previously scrolled all the way down, ensure it stays scrolled down, if scroll lock is on
if (this.tree.scrollHeight !== this.previousTreeScrollHeight) {
// Due to rounding, the scrollTop + renderHeight will not exactly match the scrollHeight.
// Consider the tree to be scrolled all the way down if it is within 2px of the bottom.
const lastElementWasVisible = this.tree.scrollTop + this.tree.renderHeight >= this.previousTreeScrollHeight - 2;
if (lastElementWasVisible) {
dom.scheduleAtNextAnimationFrame(dom.getWindow(this.listContainer), () => {
// Can't set scrollTop during this event listener, the list might overwrite the change
revealLastElement(this.tree);
}, 0);
const lastItem = this.viewModel?.getItems().at(-1);
const lastResponseIsRendering = isResponseVM(lastItem) && lastItem.renderData;
if (!lastResponseIsRendering || this.scrollLock) {
// Due to rounding, the scrollTop + renderHeight will not exactly match the scrollHeight.
// Consider the tree to be scrolled all the way down if it is within 2px of the bottom.
const lastElementWasVisible = this.tree.scrollTop + this.tree.renderHeight >= this.previousTreeScrollHeight - 2;
if (lastElementWasVisible) {
dom.scheduleAtNextAnimationFrame(dom.getWindow(this.listContainer), () => {
// Can't set scrollTop during this event listener, the list might overwrite the change

// TODO This doesn't necessarily work on the first try because of the dynamic heights of items and
// the way ListView works. Twice is usually enough. But this would work better if this lived inside ListView somehow
revealLastElement(this.tree);
revealLastElement(this.tree);
}, 0);
}
}
}

// TODO@roblourens add `show-scroll-down` class when button should show
// Show the button when content height changes, the list is not fully scrolled down, and (the latest response is currently rendering OR I haven't yet scrolled all the way down since the last response)
// So for example it would not reappear if I scroll up and delete a message

this.previousTreeScrollHeight = this.tree.scrollHeight;
this._onDidChangeContentHeight.fire();
}
Expand Down Expand Up @@ -802,8 +845,7 @@ export class ChatWidget extends Disposable implements IChatWidget {

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

if (this.chatEditingService.currentEditingSession && this.chatEditingService.currentEditingSession?.chatSessionId === this.viewModel?.sessionId) {
Expand Down Expand Up @@ -835,10 +877,11 @@ export class ChatWidget extends Disposable implements IChatWidget {
}
}));

if (this.tree) {
if (this.tree && this.visible) {
this.onDidChangeItems();
revealLastElement(this.tree);
}

this.updateChatInputContext();
}

Expand Down Expand Up @@ -906,6 +949,9 @@ export class ChatWidget extends Disposable implements IChatWidget {
private async _acceptInput(opts: { query: string } | { prefix: string } | undefined, isVoiceInput?: boolean): Promise<IChatResponseModel | undefined> {
if (this.viewModel) {
this._onDidAcceptInput.fire();
if (!this.viewOptions.autoScroll) {
this.scrollLock = false;
}

const editorValue = this.getInput();
const requestId = this.chatAccessibilityService.acceptRequest();
Expand Down Expand Up @@ -1003,9 +1049,12 @@ export class ChatWidget extends Disposable implements IChatWidget {
const inputPartMaxHeight = this._dynamicMessageLayoutData?.enabled ? this._dynamicMessageLayoutData.maxHeight : height;
this.inputPart.layout(inputPartMaxHeight, width);
const inputPartHeight = this.inputPart.inputPartHeight;
const lastElementVisible = this.tree.scrollTop + this.tree.renderHeight >= this.tree.scrollHeight;
const lastElementVisible = this.tree.scrollTop + this.tree.renderHeight >= this.tree.scrollHeight - 2;

const listHeight = Math.max(0, height - inputPartHeight);
if (!this.viewOptions.autoScroll) {
this.listContainer.style.setProperty('--chat-current-response-min-height', listHeight * .75 + 'px');
}

this.tree.layout(listHeight, width);
this.tree.getHTMLElement().style.height = `${listHeight}px`;
Expand All @@ -1015,7 +1064,10 @@ export class ChatWidget extends Disposable implements IChatWidget {
this.welcomeMessageContainer.style.height = `${listHeight - followupsOffset}px`;
this.welcomeMessageContainer.style.paddingBottom = `${followupsOffset}px`;
this.renderer.layout(width);
if (lastElementVisible) {

const lastItem = this.viewModel?.getItems().at(-1);
const lastResponseIsRendering = isResponseVM(lastItem) && lastItem.renderData;
if (lastElementVisible && !lastResponseIsRendering) {
revealLastElement(this.tree);
}

Expand Down
24 changes: 24 additions & 0 deletions src/vs/workbench/contrib/chat/browser/media/chat.css
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@
background-color: var(--vscode-inputOption-activeBackground);
}

.interactive-item-container.interactive-response.chat-most-recent-response {
min-height: var(--chat-current-response-min-height);
}

.interactive-item-container.interactive-response:not(.chat-response-loading) .chat-footer-toolbar {
/* Complete response only */
display: initial;
Expand Down Expand Up @@ -242,6 +246,7 @@

.interactive-list {
overflow: hidden;
position: relative; /* For the scroll down button */
}

.hc-black .interactive-request,
Expand Down Expand Up @@ -1222,3 +1227,22 @@ have to be updated for changes to the rules above, or to support more deeply nes
outline: none;
border: none;
}

.interactive-session .chat-scroll-down {
display: none;
position: absolute;
bottom: 7px;
right: 12px;
border-radius: 100%;
width: initial;
width: 27px;
height: 27px;

.codicon {
margin: 0px;
}
}

.interactive-session.show-scroll-down .chat-scroll-down {
display: initial;
}
12 changes: 10 additions & 2 deletions src/vs/workbench/contrib/chat/common/chatViewModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export class ChatViewModel extends Disposable implements IChatViewModel {
}

private onAddResponse(responseModel: IChatResponseModel) {
const response = this.instantiationService.createInstance(ChatResponseViewModel, responseModel);
const response = this.instantiationService.createInstance(ChatResponseViewModel, responseModel, this);
this._register(response.onDidChange(() => {
if (response.isComplete) {
this.updateCodeBlockTextModels(response);
Expand Down Expand Up @@ -393,7 +393,10 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi
}

get dataId() {
return this._model.id + `_${this._modelChangeCount}` + `_${ChatModelInitState[this._model.session.initState]}`;
return this._model.id +
`_${this._modelChangeCount}` +
`_${ChatModelInitState[this._model.session.initState]}` +
(this.isLast ? '_last' : '');
}

get sessionId() {
Expand Down Expand Up @@ -489,6 +492,10 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi
return this._model.isStale;
}

get isLast(): boolean {
return this._chatViewModel.getItems().at(-1) === this;
}

renderData: IChatResponseRenderData | undefined = undefined;
currentRenderedHeight: number | undefined;

Expand Down Expand Up @@ -521,6 +528,7 @@ export class ChatResponseViewModel extends Disposable implements IChatResponseVi

constructor(
private readonly _model: IChatResponseModel,
private readonly _chatViewModel: IChatViewModel,
@ILogService private readonly logService: ILogService,
@IChatAgentNameService private readonly chatAgentNameService: IChatAgentNameService,
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ export class InlineChatWidget {
location,
undefined,
{
autoScroll: true,
defaultElementHeight: 32,
renderStyle: 'minimal',
renderInputOnTop: false,
Expand Down

0 comments on commit 8d8f0cd

Please sign in to comment.