fix(chat): preserve scroll anchor when user toggles collapsible thinking/subagent sections (#274731)#310966
Conversation
…ing/subagent sections (microsoft#274731)
|
must've been out when this was assigned to me - giving this a look for this coming iteration, thanks! |
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #274731, where expanding a collapsed thinking/subagent/hook/markdown section in the chat list snapped the viewport to the bottom instead of growing downward from the user's current scroll position. The root cause is that ChatListWidget._updateElementHeight auto-scrolls to the bottom on any height growth while the user is pinned to the bottom — correct for streaming, wrong for a user-initiated expand. The fix distinguishes user-driven from programmatic height changes: ChatCollapsibleContentPart dispatches a bubbling CustomEvent on click, and ChatListWidget skips auto-scroll for height changes within a 250 ms window after that event.
Changes:
ChatCollapsibleContentPartexposes a staticUSER_TOGGLE_EVENTand dispatches a bubblingCustomEventfrom the collapse-button click handler (programmaticsetExpandedstays silent).ChatListWidgetlistens on its container, records the toggle timestamp, and suppresses_withPersistedAutoScrollfor_updateElementHeightcalls within a 250 ms window.- Two unit tests assert exactly one bubbling event fires on click and none fire without a click.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatCollapsibleContentPart.ts |
Adds the USER_TOGGLE_EVENT constant and dispatches the bubbling event on user click. |
src/vs/workbench/contrib/chat/browser/widget/chatListWidget.ts |
Records the last toggle time and skips auto-scroll for height changes within the suppression window. |
src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatThinkingContentPart.test.ts |
Adds tests verifying the click dispatches one bubbling event and no-click dispatches none. |
| if (this._isWithinUserToggleWindow()) { | ||
| this._tree.updateElementHeight(element, height); | ||
| return; | ||
| } |
| ); | ||
| }); | ||
|
|
||
| test('no click means no USER_TOGGLE_EVENT (baseline: rendering and streaming updates do not falsely trigger scroll-anchor preservation)', () => { |
What — Manually expanding or collapsing a thinking/subagent/hook/markdown section in the chat list no longer snaps the viewport to the bottom. Previously-visible content stays visually put; the expand grows downward into unused space.
Why —
ChatListWidget._updateElementHeightauto-scrolls the list to the bottom whenever a row grows and the user was already pinned to the bottom. That's correct during streaming (content growing from below must stay visible) but wrong when the user clicks to expand a collapsed section: the expanded content appears above the anchor, so every added pixel shifts everything else upward (#274731).How — Distinguish user-driven from programmatic height changes.
ChatCollapsibleContentPartfires a bubblingCustomEvent('chatCollapsibleUserToggle')from its click handler;ChatListWidgetrecords the timestamp on receive and skips_withPersistedAutoScrollwithin a 250 ms window. ProgrammaticsetExpanded(used during streaming) doesn't dispatch the event, so auto-scroll during streaming is unchanged.Test plan — Manual (from the issue): agent-mode prompt "Look at the git changes in the last 24 hours…", wait for streaming, scroll to bottom, click to expand the thinking section. Pre-fix: viewport jumps upward. Post-fix: content above stays put. Also verified streaming auto-scroll still works. Added 2 unit tests in
chatThinkingContentPart.test.tsasserting the click dispatches exactly one bubbling event.Applies to every
ChatCollapsibleContentPartsubclass, not just thinking.Fixes #274731