From ea08fc7aa9661f673b263cfc0605617e95f20bb4 Mon Sep 17 00:00:00 2001 From: Maruthan G Date: Fri, 17 Apr 2026 16:15:54 +0530 Subject: [PATCH] fix(chat): preserve scroll anchor when user toggles collapsible thinking/subagent sections (#274731) --- .../chatCollapsibleContentPart.ts | 14 ++++ .../chat/browser/widget/chatListWidget.ts | 37 ++++++++++ .../chatThinkingContentPart.test.ts | 71 +++++++++++++++++++ 3 files changed, 122 insertions(+) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatCollapsibleContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatCollapsibleContentPart.ts index f805e40ec94baa..b83dab8b526787 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatCollapsibleContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatCollapsibleContentPart.ts @@ -26,6 +26,14 @@ import { ThemeIcon } from '../../../../../../base/common/themables.js'; export abstract class ChatCollapsibleContentPart extends Disposable implements IChatContentPart { + /** + * Bubbling DOM event dispatched from the collapsible's root when the user + * clicks the collapse/expand button. Ancestors (e.g. the chat list widget) + * can listen to preserve the scroll anchor across the resulting height + * change instead of auto-scrolling to the bottom. + */ + public static readonly USER_TOGGLE_EVENT = 'chatCollapsibleUserToggle'; + private _domNode?: HTMLElement; private readonly _renderedTitleWithWidgets = this._register(new MutableDisposable()); @@ -100,6 +108,12 @@ export abstract class ChatCollapsibleContentPart extends Disposable implements I this._register(collapseButton.onDidClick(() => { const value = this._isExpanded.get(); this._isExpanded.set(!value, undefined); + + // Signal to ancestors (e.g. the chat list widget) that this height change + // was caused by a user-initiated collapse/expand, so the list can preserve + // the user's scroll anchor instead of auto-scrolling to the bottom. + // See issue #274731. + this._domNode?.dispatchEvent(new CustomEvent(ChatCollapsibleContentPart.USER_TOGGLE_EVENT, { bubbles: true })); })); // Initialize the expanded state based on the subclass's isExpanded() method diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts index 6391bb329b556b..dafb6d12ce5d8c 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts @@ -31,6 +31,7 @@ import { IChatRequestViewModel, IChatResponseViewModel, IChatViewModel, isReques import { ChatAccessibilityProvider } from '../accessibility/chatAccessibilityProvider.js'; import { ChatTreeItem, IChatAccessibilityService, IChatCodeBlockInfo, IChatFileTreeInfo, IChatListItemRendererOptions } from '../chat.js'; import { CodeBlockPart } from './chatContentParts/codeBlockPart.js'; +import { ChatCollapsibleContentPart } from './chatContentParts/chatCollapsibleContentPart.js'; import { ChatListDelegate, ChatListItemRenderer, IChatListItemTemplate, IChatRendererDelegate } from './chatListRenderer.js'; import { ChatEditorOptions } from './chatOptions.js'; import { ChatPendingDragController } from './chatPendingDragAndDrop.js'; @@ -187,6 +188,16 @@ export class ChatListWidget extends Disposable { private _settingChangeCounter: number = 0; private _visibleChangeCount: number = 0; + /** + * Timestamp (performance.now()) of the last user-initiated collapsible + * toggle inside this list. Height changes that happen shortly after a user + * toggle are treated as user-driven layout growth and should preserve the + * user's scroll anchor instead of snapping the viewport to the bottom. + * See issue #274731. + */ + private _lastUserToggleTime: number = 0; + private static readonly USER_TOGGLE_SUPPRESS_WINDOW_MS = 250; + private readonly _container: HTMLElement; private readonly _scrollDownButton: Button; private readonly _lastItemIdContextKey: IContextKey; @@ -453,6 +464,14 @@ export class ChatListWidget extends Disposable { // Set initial at-bottom state (scrollLock defaults to true) this.updateScrollDownButtonVisibility(); + // Observe user-initiated collapsible toggles (thinking section, tool + // wrappers, etc.). When the user expands/collapses a section, we want + // the resulting height change to preserve their scroll position rather + // than auto-scrolling to the bottom (issue #274731). + this._register(dom.addDisposableListener(this._container, ChatCollapsibleContentPart.USER_TOGGLE_EVENT, () => { + this._lastUserToggleTime = performance.now(); + })); + // Handle context menu internally this._register(this._tree.onContextMenu(e => { this.handleContextMenu(e); @@ -654,12 +673,30 @@ export class ChatListWidget extends Disposable { */ private _updateElementHeight(element: ChatTreeItem, height?: number): void { if (this._tree.hasElement(element) && this._visible) { + // If the user just clicked to expand/collapse a section inside this + // list, treat the incoming height change as user-driven and skip + // auto-scroll so the expanded content grows downward from its + // current position instead of pushing visible content upward + // (issue #274731). + if (this._isWithinUserToggleWindow()) { + this._tree.updateElementHeight(element, height); + return; + } this._withPersistedAutoScroll(() => { this._tree.updateElementHeight(element, height); }); } } + /** + * Returns true if a user-initiated collapsible toggle happened recently + * enough that the resulting height change should not trigger auto-scroll. + */ + private _isWithinUserToggleWindow(): boolean { + return this._lastUserToggleTime !== 0 + && performance.now() - this._lastUserToggleTime <= ChatListWidget.USER_TOGGLE_SUPPRESS_WINDOW_MS; + } + /** * Scroll to reveal an element. */ diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts index 6dd8a6d73e9b40..4e4f4304bb25fa 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts @@ -13,6 +13,7 @@ import { mainWindow } from '../../../../../../../base/browser/window.js'; import { workbenchInstantiationService } from '../../../../../../test/browser/workbenchTestServices.js'; import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { ChatCollapsibleContentPart } from '../../../../browser/widget/chatContentParts/chatCollapsibleContentPart.js'; import { ChatThinkingContentPart } from '../../../../browser/widget/chatContentParts/chatThinkingContentPart.js'; import { IChatMarkdownContent, IChatThinkingPart, IChatToolInvocation, IChatToolInvocationSerialized } from '../../../../common/chatService/chatService.js'; import { IChatContentPartRenderContext, InlineTextModelCollection } from '../../../../browser/widget/chatContentParts/chatContentParts.js'; @@ -1847,4 +1848,74 @@ suite('ChatThinkingContentPart', () => { assert.strictEqual(disposed, true, 'Factory disposable should be disposed with thinking part'); }); }); + + suite('User toggle signal for scroll anchoring (issue #274731)', () => { + setup(() => { + mockConfigurationService.setUserConfiguration('chat.agent.thinkingStyle', ThinkingDisplayMode.Collapsed); + }); + + test('clicking the collapse button fires a bubbling USER_TOGGLE_EVENT so the chat list can preserve scroll anchor', () => { + const content = createThinkingPart('**Analyzing code**'); + const context = createMockRenderContext(true); // isComplete = true so click actually toggles expand state + + const part = store.add(instantiationService.createInstance( + ChatThinkingContentPart, + content, + context, + mockMarkdownRenderer, + true + )); + + // Wrap in an ancestor to verify the event bubbles (chat list widget + // listens at the container level, not on the part itself). + const ancestor = mainWindow.document.createElement('div'); + ancestor.appendChild(part.domNode); + mainWindow.document.body.appendChild(ancestor); + disposables.add(toDisposable(() => ancestor.remove())); + + let toggleEventCount = 0; + const listener = () => { toggleEventCount++; }; + ancestor.addEventListener(ChatCollapsibleContentPart.USER_TOGGLE_EVENT, listener); + disposables.add(toDisposable(() => ancestor.removeEventListener(ChatCollapsibleContentPart.USER_TOGGLE_EVENT, listener))); + + // Simulate a user clicking the collapse/expand button. + const button = part.domNode.querySelector('.monaco-button') as HTMLElement; + assert.ok(button, 'Should have collapse button'); + button.click(); + + assert.strictEqual( + toggleEventCount, 1, + 'Clicking the collapse button should dispatch exactly one bubbling USER_TOGGLE_EVENT so the chat list preserves the scroll anchor instead of pushing content upward.' + ); + }); + + test('no click means no USER_TOGGLE_EVENT (baseline: rendering and streaming updates do not falsely trigger scroll-anchor preservation)', () => { + const content = createThinkingPart('**Analyzing code**'); + const context = createMockRenderContext(false); + + const part = store.add(instantiationService.createInstance( + ChatThinkingContentPart, + content, + context, + mockMarkdownRenderer, + false + )); + + mainWindow.document.body.appendChild(part.domNode); + disposables.add(toDisposable(() => part.domNode.remove())); + + let toggleEventCount = 0; + const listener = () => { toggleEventCount++; }; + part.domNode.addEventListener(ChatCollapsibleContentPart.USER_TOGGLE_EVENT, listener); + disposables.add(toDisposable(() => part.domNode.removeEventListener(ChatCollapsibleContentPart.USER_TOGGLE_EVENT, listener))); + + // No user click: mounting, rendering, and automatic state changes + // must NOT dispatch the user-toggle event. Otherwise the chat list + // would incorrectly suppress auto-scroll during streaming. + assert.strictEqual( + toggleEventCount, 0, + 'USER_TOGGLE_EVENT must only fire in response to explicit user clicks, not construction or rendering.' + ); + }); + }); });