From 2196b8e5a931e73a03a5032cb5aa7bcdc5229ee3 Mon Sep 17 00:00:00 2001 From: justschen Date: Mon, 20 Jul 2026 09:22:27 -0700 Subject: [PATCH 1/2] sessions: reduce hidden animation work - Pauses continuous session-title shimmer and pixel-spinner animations when their elements are outside the viewport or their document is hidden, avoiding rendering work that cannot be seen. - Centralizes visibility-aware CSS animation handling so animated components share window lifecycle cleanup and resume synchronization instead of maintaining separate observers. - Caps the shimmer's visual update cadence while preserving its existing three-second appearance, reducing unnecessary paint pressure in large session lists. - Documents the sessions-list animation behavior for future maintenance. (Commit message generated by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/base/browser/animationSync.ts | 124 ++++++++++++++++++ .../browser/ui/pixelSpinner/pixelSpinner.ts | 67 +--------- src/vs/sessions/SESSIONS_LIST.md | 2 + .../sessions/browser/media/sessionsList.css | 6 +- .../sessions/browser/views/sessionsList.ts | 7 +- 5 files changed, 144 insertions(+), 62 deletions(-) diff --git a/src/vs/base/browser/animationSync.ts b/src/vs/base/browser/animationSync.ts index e962a5595aec22..67a5c44b35cada 100644 --- a/src/vs/base/browser/animationSync.ts +++ b/src/vs/base/browser/animationSync.ts @@ -3,6 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { addDisposableListener, getWindow, onDidUnregisterWindow } from './dom.js'; +import { CodeWindow } from './window.js'; +import { Disposable, IDisposable, toDisposable } from '../common/lifecycle.js'; + export interface ISynchronizeAnimationsOptions { /** * Also synchronize animations running on descendant elements (e.g. the dots @@ -64,3 +68,123 @@ export function synchronizeCSSAnimations(element: HTMLElement, options?: ISynchr } } } + +export interface IPauseCSSAnimationsWhenHiddenOptions extends ISynchronizeAnimationsOptions { + readonly pausedClass: string; +} + +interface ITrackedAnimation { + readonly options: IPauseCSSAnimationsWhenHiddenOptions; +} + +interface IAnimationVisibilityObserver { + readonly observer: IntersectionObserver; + readonly trackedAnimations: Map; + readonly intersectingElements: Set; + readonly visibilityListener: IDisposable; +} + +const animationVisibilityObservers = new Map(); +let unregisterWindowListener: IDisposable | undefined; + +/** + * Pauses CSS animations while their element is outside the viewport or its document is hidden. + */ +export function pauseCSSAnimationsWhenHidden(element: HTMLElement, options: IPauseCSSAnimationsWhenHiddenOptions): IDisposable { + const targetWindow = getWindow(element); + if (typeof targetWindow.IntersectionObserver !== 'function') { + return Disposable.None; + } + + let state = animationVisibilityObservers.get(targetWindow); + if (!state) { + const trackedAnimations = new Map(); + const intersectingElements = new Set(); + const observer = new targetWindow.IntersectionObserver(entries => { + const toResync: Array<[HTMLElement, IPauseCSSAnimationsWhenHiddenOptions]> = []; + for (const entry of entries) { + const target = entry.target as HTMLElement; + const trackedAnimation = trackedAnimations.get(target); + if (!trackedAnimation) { + continue; + } + if (!target.isConnected) { + observer.unobserve(target); + trackedAnimations.delete(target); + intersectingElements.delete(target); + continue; + } + if (entry.isIntersecting) { + intersectingElements.add(target); + } else { + intersectingElements.delete(target); + } + const paused = targetWindow.document.hidden || !entry.isIntersecting; + target.classList.toggle(trackedAnimation.options.pausedClass, paused); + if (!paused) { + toResync.push([target, trackedAnimation.options]); + } + } + + for (const [target, trackedOptions] of toResync) { + synchronizeCSSAnimations(target, trackedOptions); + } + disposeVisibilityObserverIfEmpty(targetWindow, animationVisibilityObservers.get(targetWindow)); + }); + const visibilityListener = addDisposableListener(targetWindow.document, 'visibilitychange', () => { + const documentHidden = targetWindow.document.hidden; + const toResync: Array<[HTMLElement, IPauseCSSAnimationsWhenHiddenOptions]> = []; + for (const [target, trackedAnimation] of trackedAnimations) { + if (!target.isConnected) { + observer.unobserve(target); + trackedAnimations.delete(target); + intersectingElements.delete(target); + continue; + } + const paused = documentHidden || !intersectingElements.has(target); + target.classList.toggle(trackedAnimation.options.pausedClass, paused); + if (!paused) { + toResync.push([target, trackedAnimation.options]); + } + } + for (const [target, trackedOptions] of toResync) { + synchronizeCSSAnimations(target, trackedOptions); + } + disposeVisibilityObserverIfEmpty(targetWindow, animationVisibilityObservers.get(targetWindow)); + }); + state = { observer, trackedAnimations, intersectingElements, visibilityListener }; + animationVisibilityObservers.set(targetWindow, state); + + if (!unregisterWindowListener) { + unregisterWindowListener = onDidUnregisterWindow(window => { + const state = animationVisibilityObservers.get(window); + if (state) { + state.observer.disconnect(); + state.visibilityListener.dispose(); + animationVisibilityObservers.delete(window); + } + }); + } + } + + element.classList.add(options.pausedClass); + state.trackedAnimations.set(element, { options }); + state.observer.observe(element); + + return toDisposable(() => { + state.observer.unobserve(element); + state.trackedAnimations.delete(element); + state.intersectingElements.delete(element); + element.classList.remove(options.pausedClass); + disposeVisibilityObserverIfEmpty(targetWindow, state); + }); +} + +function disposeVisibilityObserverIfEmpty(targetWindow: CodeWindow, state: IAnimationVisibilityObserver | undefined): void { + if (!state || state.trackedAnimations.size !== 0 || animationVisibilityObservers.get(targetWindow) !== state) { + return; + } + state.observer.disconnect(); + state.visibilityListener.dispose(); + animationVisibilityObservers.delete(targetWindow); +} diff --git a/src/vs/base/browser/ui/pixelSpinner/pixelSpinner.ts b/src/vs/base/browser/ui/pixelSpinner/pixelSpinner.ts index 14399960b8a9a2..38cf85f787b2d3 100644 --- a/src/vs/base/browser/ui/pixelSpinner/pixelSpinner.ts +++ b/src/vs/base/browser/ui/pixelSpinner/pixelSpinner.ts @@ -3,10 +3,8 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { getWindow, h, onDidUnregisterWindow } from '../../dom.js'; -import { synchronizeCSSAnimations } from '../../animationSync.js'; -import { CodeWindow } from '../../window.js'; -import { IDisposable } from '../../../common/lifecycle.js'; +import { h } from '../../dom.js'; +import { pauseCSSAnimationsWhenHidden } from '../../animationSync.js'; import './pixelSpinner.css'; export interface IPixelSpinnerOptions { @@ -67,62 +65,11 @@ const SPINNER_ANIMATION_NAMES = new Set([ 'monaco-pixel-spinner-dot-cycle-short', 'monaco-pixel-spinner-ring-pulse', ]); -const observersByWindow = new Map(); -let unregisterWindowListener: IDisposable | undefined; - -function getObserverFor(targetWindow: CodeWindow): IntersectionObserver | undefined { - if (typeof targetWindow.IntersectionObserver !== 'function') { - return undefined; - } - let observer = observersByWindow.get(targetWindow); - if (!observer) { - observer = new targetWindow.IntersectionObserver(entries => { - // Two passes so all style writes happen before any style read: the - // pause-class toggles below dirty style, and `getAnimations()` in the - // sync pass flushes it. Interleaving them would force a style recalc - // per entry instead of one for the whole batch. - const toResync: HTMLElement[] = []; - for (const entry of entries) { - const target = entry.target as HTMLElement; - if (!target.isConnected) { - observer!.unobserve(target); - continue; - } - target.classList.toggle(PAUSED_CLASS, !entry.isIntersecting); - if (entry.isIntersecting) { - toResync.push(target); - } - } - // Re-sync resumed spinners to the shared timeline: while paused - // offscreen the animation froze and its startTime drifted from - // spinners that kept running. Anchor it back (now that it is running - // again) so all visible spinners display the same frame. - for (const target of toResync) { - synchronizeCSSAnimations(target, { subtree: true, animationNames: SPINNER_ANIMATION_NAMES }); - } - }); - observersByWindow.set(targetWindow, observer); - - if (!unregisterWindowListener) { - unregisterWindowListener = onDidUnregisterWindow(window => { - const obs = observersByWindow.get(window); - if (obs) { - obs.disconnect(); - observersByWindow.delete(window); - } - }); - } - } - return observer; -} function trackSpinner(root: HTMLElement): void { - const observer = getObserverFor(getWindow(root)); - if (!observer) { - return; - } - // Start paused; the observer delivers an initial notification that resumes - // the spinner if it is actually on screen. - root.classList.add(PAUSED_CLASS); - observer.observe(root); + pauseCSSAnimationsWhenHidden(root, { + pausedClass: PAUSED_CLASS, + subtree: true, + animationNames: SPINNER_ANIMATION_NAMES, + }); } diff --git a/src/vs/sessions/SESSIONS_LIST.md b/src/vs/sessions/SESSIONS_LIST.md index bc073740ce7ab9..803b73342410d3 100644 --- a/src/vs/sessions/SESSIONS_LIST.md +++ b/src/vs/sessions/SESSIONS_LIST.md @@ -37,6 +37,8 @@ Each session row displays: Quick-chat rows (`.session-item.quick-chat`, driven by the reactive `ISession.isQuickChat` observable) are single-line entries: the details (second) row is hidden entirely and its content is never built — smaller icon, one line of title only, tighter row height (see `SessionsTreeDelegate.ITEM_HEIGHT_QUICK_CHAT`). Regular sessions keep the standard two-line row (title + details row). +Continuous row animations preserve their existing appearance while limiting rendering work: the title shimmer follows the same three-second path with at most 60 visual updates per second, and both it and the shared pixel spinner pause outside the viewport and whenever their document is hidden. + `SessionsFlatList` reuses the same session row renderer for sectionless surfaces, including the approval row and dynamic row height updates. Consumers that size their own container listen for content-height changes and relayout the list. When embedded inside another hover, consumers disable row hovers so moving over the list does not replace the parent hover. ### Grouping diff --git a/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css b/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css index f984cdf1e59d60..19c97f2fefb9ae 100644 --- a/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css +++ b/src/vs/sessions/contrib/sessions/browser/media/sessionsList.css @@ -642,7 +642,11 @@ background-clip: text; -webkit-background-clip: text; -webkit-text-fill-color: transparent; - animation: session-title-shimmer 3s linear infinite; + animation: session-title-shimmer 3s steps(180, jump-none) infinite; + } + + .monaco-list-row:not(.selected) .session-item.in-progress .session-title.session-title-shimmer-paused { + animation-play-state: paused; } .vs-dark .monaco-list-row:not(.selected) .session-item.in-progress .session-title, diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts index 624e647345f913..7445d67206830b 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts @@ -5,7 +5,7 @@ import '../media/sessionsList.css'; import * as DOM from '../../../../../base/browser/dom.js'; -import { synchronizeCSSAnimations } from '../../../../../base/browser/animationSync.js'; +import { pauseCSSAnimationsWhenHidden, synchronizeCSSAnimations } from '../../../../../base/browser/animationSync.js'; import { Gesture } from '../../../../../base/browser/touch.js'; import { IListVirtualDelegate, ListDragOverEffectPosition, ListDragOverEffectType, NotSelectableGroupId } from '../../../../../base/browser/ui/list/list.js'; import { IListStyles } from '../../../../../base/browser/ui/list/listWidget.js'; @@ -295,6 +295,7 @@ class SessionItemActionRunner extends ActionRunner { // in sessionsList.css). Used to phase-align the shimmer across rows. const SESSION_TITLE_SHIMMER_ANIMATION_NAME = 'session-title-shimmer'; const SESSION_TITLE_SHIMMER_ANIMATION_NAMES = new Set([SESSION_TITLE_SHIMMER_ANIMATION_NAME]); +const SESSION_TITLE_SHIMMER_PAUSED_CLASS = 'session-title-shimmer-paused'; interface ISessionItemTemplate { readonly container: HTMLElement; @@ -410,6 +411,10 @@ class SessionItemRenderer implements ITreeRenderer Date: Tue, 21 Jul 2026 00:52:45 -0700 Subject: [PATCH 2/2] address comments --- src/vs/base/browser/animationSync.ts | 9 +++++ .../browser/ui/pixelSpinner/pixelSpinner.ts | 20 +++++++---- src/vs/sessions/browser/sessionStatusIcon.ts | 33 ++++++++++++++----- .../agentSessions/agentSessionsViewer.ts | 11 +++++-- .../chatMcpServersStartingContentPart.ts | 6 ++-- .../chatTerminalToolProgressPart.ts | 2 +- .../chatMcpServersStartingContentPart.test.ts | 14 ++++---- 7 files changed, 68 insertions(+), 27 deletions(-) diff --git a/src/vs/base/browser/animationSync.ts b/src/vs/base/browser/animationSync.ts index 67a5c44b35cada..1aac420b7f450c 100644 --- a/src/vs/base/browser/animationSync.ts +++ b/src/vs/base/browser/animationSync.ts @@ -162,6 +162,7 @@ export function pauseCSSAnimationsWhenHidden(element: HTMLElement, options: IPau state.observer.disconnect(); state.visibilityListener.dispose(); animationVisibilityObservers.delete(window); + disposeUnregisterWindowListenerIfUnused(); } }); } @@ -187,4 +188,12 @@ function disposeVisibilityObserverIfEmpty(targetWindow: CodeWindow, state: IAnim state.observer.disconnect(); state.visibilityListener.dispose(); animationVisibilityObservers.delete(targetWindow); + disposeUnregisterWindowListenerIfUnused(); +} + +function disposeUnregisterWindowListenerIfUnused(): void { + if (animationVisibilityObservers.size === 0) { + unregisterWindowListener?.dispose(); + unregisterWindowListener = undefined; + } } diff --git a/src/vs/base/browser/ui/pixelSpinner/pixelSpinner.ts b/src/vs/base/browser/ui/pixelSpinner/pixelSpinner.ts index 38cf85f787b2d3..27ce3847cfb85c 100644 --- a/src/vs/base/browser/ui/pixelSpinner/pixelSpinner.ts +++ b/src/vs/base/browser/ui/pixelSpinner/pixelSpinner.ts @@ -5,6 +5,7 @@ import { h } from '../../dom.js'; import { pauseCSSAnimationsWhenHidden } from '../../animationSync.js'; +import { IDisposable } from '../../../common/lifecycle.js'; import './pixelSpinner.css'; export interface IPixelSpinnerOptions { @@ -25,6 +26,10 @@ export interface IPixelSpinnerOptions { readonly variant?: 'grid' | 'ring'; } +export interface IPixelSpinner extends IDisposable { + readonly element: HTMLElement; +} + /** * Creates a small pixel-art style spinner. Color is driven by `currentColor`, * so consumers can control the visual color via the parent element's `color` @@ -34,9 +39,9 @@ export interface IPixelSpinnerOptions { * * @param parent Optional parent to append the spinner to. * @param options Optional spinner configuration. - * @returns The spinner root element. + * @returns The spinner and its root element. */ -export function createPixelSpinner(parent?: HTMLElement, options?: IPixelSpinnerOptions): HTMLElement { +export function createPixelSpinner(parent?: HTMLElement, options?: IPixelSpinnerOptions): IPixelSpinner { const variant = options?.variant ?? 'grid'; const rootClass = variant === 'ring' ? 'span.monaco-pixel-spinner.monaco-pixel-spinner-ring' : 'span.monaco-pixel-spinner'; const root = h(rootClass).root; @@ -50,8 +55,11 @@ export function createPixelSpinner(parent?: HTMLElement, options?: IPixelSpinner root.appendChild(h('span.monaco-pixel-spinner-dot').root); } parent?.appendChild(root); - trackSpinner(root); - return root; + const animationTracking = trackSpinner(root); + return { + element: root, + dispose: () => animationTracking.dispose(), + }; } @@ -66,8 +74,8 @@ const SPINNER_ANIMATION_NAMES = new Set([ 'monaco-pixel-spinner-ring-pulse', ]); -function trackSpinner(root: HTMLElement): void { - pauseCSSAnimationsWhenHidden(root, { +function trackSpinner(root: HTMLElement): IDisposable { + return pauseCSSAnimationsWhenHidden(root, { pausedClass: PAUSED_CLASS, subtree: true, animationNames: SPINNER_ANIMATION_NAMES, diff --git a/src/vs/sessions/browser/sessionStatusIcon.ts b/src/vs/sessions/browser/sessionStatusIcon.ts index 8afe862fcf7720..a6c27fbe96d5f8 100644 --- a/src/vs/sessions/browser/sessionStatusIcon.ts +++ b/src/vs/sessions/browser/sessionStatusIcon.ts @@ -5,7 +5,7 @@ import * as DOM from '../../base/browser/dom.js'; import { disposableTimeout } from '../../base/common/async.js'; -import { Disposable, DisposableStore } from '../../base/common/lifecycle.js'; +import { Disposable, DisposableMap, DisposableStore, IDisposable } from '../../base/common/lifecycle.js'; import { ThemeIcon } from '../../base/common/themables.js'; import { createPixelSpinner } from '../../base/browser/ui/pixelSpinner/pixelSpinner.js'; import { asCssVariable } from '../../platform/theme/common/colorUtils.js'; @@ -60,6 +60,7 @@ export class SessionStatusIcon extends Disposable { /** Owns the removal timers for outgoing icons mid cross-fade. */ private readonly _swapStore = this._register(new DisposableStore()); + private readonly _iconDisposables = this._register(new DisposableMap()); constructor( private readonly _container: HTMLElement, @@ -98,6 +99,7 @@ export class SessionStatusIcon extends Disposable { this._currentCacheKey = undefined; this._lastInputs = undefined; this._swapStore.clear(); + this._iconDisposables.clearAndDisposeAll(); DOM.clearNode(this._container); } @@ -107,18 +109,21 @@ export class SessionStatusIcon extends Disposable { let cacheKey: string; let color: string; - let createIcon: () => HTMLElement; + let createIcon: () => { element: HTMLElement; disposable?: IDisposable }; if (isSpinner) { const isNeedsInput = status === SessionStatus.NeedsInput; const variant: 'grid' | 'ring' = isNeedsInput ? 'ring' : 'grid'; cacheKey = isNeedsInput ? PIXEL_SPINNER_RING_KEY : PIXEL_SPINNER_GRID_KEY; color = isNeedsInput ? asCssVariable('list.warningForeground') : asCssVariable('textLink.foreground'); - createIcon = () => createPixelSpinner(undefined, { variant }); + createIcon = () => { + const spinner = createPixelSpinner(undefined, { variant }); + return { element: spinner.element, disposable: spinner }; + }; } else { const icon = this._sessionsListModelService.getStatusIcon(status, isRead, isArchived, completedStateIcon); cacheKey = ThemeIcon.asCSSSelector(icon); color = icon.color ? asCssVariable(icon.color.id) : ''; - createIcon = () => $(`span${cacheKey}`); + createIcon = () => ({ element: $(`span${cacheKey}`) }); } // Reduced-motion fallback for needs-input pulses the codicon; harmless when a spinner is shown. @@ -131,9 +136,9 @@ export class SessionStatusIcon extends Disposable { const animate = this._currentCacheKey !== undefined; this._currentCacheKey = cacheKey; - const iconEl = createIcon(); - iconEl.style.color = color; - this._swapIcon(iconEl, animate); + const { element: iconElement, disposable: iconDisposable } = createIcon(); + iconElement.style.color = color; + this._swapIcon(iconElement, animate, iconDisposable); } /** Updates the color of the current (non fading-out) icon without rebuilding it. */ @@ -152,10 +157,14 @@ export class SessionStatusIcon extends Disposable { * new child can settle into its slot during the fade. Safe to call repeatedly: * each outgoing element is marked so a follow-up swap never re-processes it. */ - private _swapIcon(newChild: HTMLElement, animate: boolean): void { + private _swapIcon(newChild: HTMLElement, animate: boolean, disposable: IDisposable | undefined): void { if (!animate) { + this._iconDisposables.clearAndDisposeAll(); DOM.clearNode(this._container); this._container.appendChild(newChild); + if (disposable) { + this._iconDisposables.set(newChild, disposable); + } return; } for (const existing of Array.from(this._container.children) as HTMLElement[]) { @@ -168,11 +177,17 @@ export class SessionStatusIcon extends Disposable { existing.style.left = '0'; existing.style.transition = `opacity ${ICON_SWAP_FADE_MS}ms ease`; DOM.scheduleAtNextAnimationFrame(DOM.getWindow(existing), () => { existing.style.opacity = '0'; }); - disposableTimeout(() => existing.remove(), ICON_SWAP_FADE_MS + 40, this._swapStore); + disposableTimeout(() => { + existing.remove(); + this._iconDisposables.deleteAndDispose(existing); + }, ICON_SWAP_FADE_MS + 40, this._swapStore); } newChild.style.opacity = '0'; newChild.style.transition = `opacity ${ICON_SWAP_FADE_MS}ms ease`; this._container.appendChild(newChild); + if (disposable) { + this._iconDisposables.set(newChild, disposable); + } DOM.scheduleAtNextAnimationFrame(DOM.getWindow(newChild), () => { newChild.style.opacity = '1'; }); } } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsViewer.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsViewer.ts index 2fe47448843245..2a5cca08c8c12d 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsViewer.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsViewer.ts @@ -54,7 +54,7 @@ import { compareIgnoreCase } from '../../../../../base/common/strings.js'; import { CancellationTokenSource } from '../../../../../base/common/cancellation.js'; import { IChatSessionsService } from '../../common/chatSessionsService.js'; import { IVoicePlaybackService } from '../../common/voicePlaybackService.js'; -import { createPixelSpinner } from '../../../../../base/browser/ui/pixelSpinner/pixelSpinner.js'; +import { createPixelSpinner, IPixelSpinner } from '../../../../../base/browser/ui/pixelSpinner/pixelSpinner.js'; import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js'; export type AgentSessionListItem = IAgentSession | IAgentSessionSection | IAgentSessionShowMore | IAgentSessionShowLess; @@ -103,6 +103,7 @@ class AgentSessionStatusIcon extends Disposable { private _currentCacheKey: string | undefined; private _lastSession: IAgentSession | undefined; + private readonly spinner = this._register(new MutableDisposable()); constructor( private readonly container: HTMLElement, @@ -126,6 +127,7 @@ class AgentSessionStatusIcon extends Disposable { reset(): void { this._currentCacheKey = undefined; this._lastSession = undefined; + this.spinner.clear(); clearNode(this.container); } @@ -143,10 +145,12 @@ class AgentSessionStatusIcon extends Disposable { } this._currentCacheKey = cacheKey; + this.spinner.clear(); clearNode(this.container); const spinner = createPixelSpinner(undefined, { variant: isNeedsInput ? 'ring' : 'grid' }); - spinner.style.color = color; - this.container.appendChild(spinner); + this.spinner.value = spinner; + spinner.element.style.color = color; + this.container.appendChild(spinner.element); return; } @@ -159,6 +163,7 @@ class AgentSessionStatusIcon extends Disposable { } this._currentCacheKey = cacheKey; + this.spinner.clear(); clearNode(this.container); const iconElement = h(`span${cacheKey}`).root; iconElement.style.color = color; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMcpServersStartingContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMcpServersStartingContentPart.ts index 089c3f82b4a631..0414a8a420b8f8 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMcpServersStartingContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMcpServersStartingContentPart.ts @@ -5,7 +5,7 @@ import * as dom from '../../../../../../base/browser/dom.js'; import { IRenderedMarkdown } from '../../../../../../base/browser/markdownRenderer.js'; -import { createPixelSpinner } from '../../../../../../base/browser/ui/pixelSpinner/pixelSpinner.js'; +import { createPixelSpinner, IPixelSpinner } from '../../../../../../base/browser/ui/pixelSpinner/pixelSpinner.js'; import { escapeMarkdownSyntaxTokens, MarkdownString } from '../../../../../../base/common/htmlContent.js'; import { Disposable, IDisposable, MutableDisposable } from '../../../../../../base/common/lifecycle.js'; import { autorun } from '../../../../../../base/common/observable.js'; @@ -28,6 +28,7 @@ export class ChatMcpServersStartingContentPart extends Disposable implements ICh public readonly domNode: HTMLElement; private readonly rendered = this._register(new MutableDisposable()); + private readonly spinner = this._register(new MutableDisposable()); private hadStartingServers = false; private didNotifyFinished = false; @@ -49,6 +50,7 @@ export class ChatMcpServersStartingContentPart extends Disposable implements ICh private render(servers: readonly IChatMcpStartingServer[]): void { dom.clearNode(this.domNode); this.rendered.clear(); + this.spinner.clear(); if (!servers.length) { this.domNode.style.display = 'none'; @@ -73,7 +75,7 @@ export class ChatMcpServersStartingContentPart extends Disposable implements ICh const container = dom.$('.chat-mcp-servers-interaction-hint'); const messageContainer = dom.$('.chat-mcp-servers-message'); const iconElement = dom.$('.chat-mcp-servers-icon'); - (this.options?.createSpinner ?? createPixelSpinner)(iconElement); + this.spinner.value = (this.options?.createSpinner ?? createPixelSpinner)(iconElement); const rendered = this.rendered.value = this.markdownRendererService.render(new MarkdownString(content)); messageContainer.appendChild(iconElement); diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts index 2ee92dd99c2798..9e74cd79b46eac 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts @@ -158,7 +158,7 @@ class TerminalCommandDecoration extends Disposable { super(); const decorationElements = h('span.chat-terminal-command-decoration@decoration', { role: 'img', tabIndex: 0 }); this._element = decorationElements.decoration; - createPixelSpinner(this._element); + this._register(createPixelSpinner(this._element)); this._attachElementToContainer(); } diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatMcpServersStartingContentPart.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatMcpServersStartingContentPart.test.ts index 1d910e8876d863..ecd31d0e1e5f9c 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatMcpServersStartingContentPart.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatMcpServersStartingContentPart.test.ts @@ -32,27 +32,29 @@ suite('ChatMcpServersStartingContentPart', () => { sessionResource: URI.parse('chat-session://test/session1'), servers: servers$, }; + let disposedSpinners = 0; const createSpinner = (parent?: HTMLElement) => { const spinner = document.createElement('span'); spinner.classList.add('monaco-pixel-spinner'); parent?.appendChild(spinner); - return spinner; + return { element: spinner, dispose: () => disposedSpinners++ }; }; let finishedCount = 0; const part = disposables.add(instantiationService.createInstance(ChatMcpServersStartingContentPart, data, { createSpinner, onDidFinishStarting: () => finishedCount++, })); - return { part, servers$, getFinishedCount: () => finishedCount }; + return { part, servers$, getFinishedCount: () => finishedCount, getDisposedSpinners: () => disposedSpinners }; } test('reflects the starting servers and hides when empty as the observable updates', () => { - const { part, servers$, getFinishedCount } = createPart([{ id: 'a', name: 'alpha' }, { id: 'b', name: 'beta' }]); + const { part, servers$, getFinishedCount, getDisposedSpinners } = createPart([{ id: 'a', name: 'alpha' }, { id: 'b', name: 'beta' }]); const snapshot = () => ({ hidden: part.domNode.style.display === 'none', text: part.domNode.textContent ?? '', hasPixelSpinner: !!part.domNode.querySelector('.monaco-pixel-spinner'), + disposedSpinners: getDisposedSpinners(), finishedCount: getFinishedCount(), }); @@ -65,9 +67,9 @@ suite('ChatMcpServersStartingContentPart', () => { const afterAllFinished = snapshot(); assert.deepStrictEqual({ initial, afterOneFinished, afterAllFinished }, { - initial: { hidden: false, text: 'Starting MCP servers alpha, beta...', hasPixelSpinner: true, finishedCount: 0 }, - afterOneFinished: { hidden: false, text: 'Starting MCP servers alpha...', hasPixelSpinner: true, finishedCount: 0 }, - afterAllFinished: { hidden: true, text: '', hasPixelSpinner: false, finishedCount: 1 }, + initial: { hidden: false, text: 'Starting MCP servers alpha, beta...', hasPixelSpinner: true, disposedSpinners: 0, finishedCount: 0 }, + afterOneFinished: { hidden: false, text: 'Starting MCP servers alpha...', hasPixelSpinner: true, disposedSpinners: 1, finishedCount: 0 }, + afterAllFinished: { hidden: true, text: '', hasPixelSpinner: false, disposedSpinners: 2, finishedCount: 1 }, }); });