Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2026-04-02 08:15:56 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #302589 from microsoft/bryanchen-d/fix-listener-leak-show-checkmarks

Browse Source 
fix: reduce listener leak from per-part show-checkmarks subscriptions
This commit is contained in:
Bryan Chen
2026-03-18 07:36:52 -07:00
committed by GitHub
parent 4f23fabf8a e0e770689f
commit fa4879a84e
6 changed files with 15 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
.github/prompts/fix-error.prompt.md vendored
View File

@@ -30,8 +30,8 @@ After the fix is validated (compilation clean, tests pass):
- Steps a user can follow to manually validate the fix.
- How the fix addresses the issue, with a brief note per changed file.
5. **Monitor the PR** for Copilot review comments. Wait 1-2 minutes after each push for Copilot to leave its review, then check for new comments. Evaluate each comment:
- If valid, apply the fix, amend the commit, and force-push.
- If valid, apply the fix in a new commit, push, and **resolve the comment thread**.
- If not applicable, leave a reply explaining why.
- After addressing comments, update the PR description if the changes affect the summary, code flow explanation, or per-file notes.
6. **Repeat monitoring** after each force-push: wait 1-2 minutes, check for new Copilot comments, and address them. Continue this loop until no new comments appear.
6. **Repeat monitoring** after each push: wait 1-2 minutes, check for new Copilot comments, and address them. Continue this loop until no new comments appear.
7. **Re-run tests** after addressing review comments to confirm nothing regressed.

9
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatMarkdownContentPart.ts
View File

@@ -526,15 +526,6 @@ export class CollapsedCodeBlock extends Disposable {
this.element.appendChild(this.statusIndicatorContainer);
this.element.appendChild(this.pillElement);
// Toggle show-checkmarks class for the accessibility setting
const updateCheckmarks = () => this.element.classList.toggle('show-checkmarks', !!this.configurationService.getValue<boolean>(AccessibilityWorkbenchSettingId.ShowChatCheckmarks));
updateCheckmarks();
this._register(this.configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration(AccessibilityWorkbenchSettingId.ShowChatCheckmarks)) {
updateCheckmarks();
}
}));
this.registerListeners();
}

10
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolProgressPart.ts
View File

@@ -45,7 +45,7 @@ import { URI } from '../../../../../../../base/common/uri.js';
import { stripIcons } from '../../../../../../../base/common/iconLabels.js';
import { IAccessibleViewService } from '../../../../../../../platform/accessibility/browser/accessibleView.js';
import { IContextKey, IContextKeyService } from '../../../../../../../platform/contextkey/common/contextkey.js';
import { AccessibilityVerbositySettingId, AccessibilityWorkbenchSettingId } from '../../../../../accessibility/browser/accessibilityConfiguration.js';
import { AccessibilityVerbositySettingId } from '../../../../../accessibility/browser/accessibilityConfiguration.js';
import { ChatContextKeys } from '../../../../common/actions/chatContextKeys.js';
import { EditorPool } from '../chatContentCodePools.js';
import { IKeybindingService } from '../../../../../../../platform/keybinding/common/keybinding.js';
@@ -422,14 +422,6 @@ export class ChatTerminalToolProgressPart extends BaseChatToolInvocationSubPart
this.domNode = this._createCollapsibleWrapper(progressPart.domNode, displayCommand, toolInvocation, context);
} else {
this.domNode = progressPart.domNode;
// Toggle show-checkmarks class on the progress container for accessibility setting
const updateCheckmarks = () => this.domNode.classList.toggle('show-checkmarks', !!this._configurationService.getValue<boolean>(AccessibilityWorkbenchSettingId.ShowChatCheckmarks));
updateCheckmarks();
this._register(this._configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration(AccessibilityWorkbenchSettingId.ShowChatCheckmarks)) {
updateCheckmarks();
}
}));
}
this._renderImagePills(toolInvocation, context, elements.container);

9
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolProgressPart.ts
View File

@@ -36,15 +36,6 @@ export class ChatToolProgressSubPart extends BaseChatToolInvocationSubPart {
super(toolInvocation);
this.domNode = this.createProgressPart();
// Toggle show-checkmarks class for the accessibility setting
const updateCheckmarks = () => this.domNode.classList.toggle('show-checkmarks', !!this.configurationService.getValue<boolean>(AccessibilityWorkbenchSettingId.ShowChatCheckmarks));
updateCheckmarks();
this._register(this.configurationService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration(AccessibilityWorkbenchSettingId.ShowChatCheckmarks)) {
updateCheckmarks();
}
}));
}
private createProgressPart(): HTMLElement {

19
src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts
View File

@@ -745,6 +745,17 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
templateData.rowContainer.classList.toggle('interactive-response', isResponseVM(element));
const progressMessageAtBottomOfResponse = checkModeOption(this.delegate.currentChatMode(), this.rendererOptions.progressMessageAtBottomOfResponse);
templateData.rowContainer.classList.toggle('show-detail-progress', isResponseVM(element) && !element.isComplete && !element.progressMessages.length && !progressMessageAtBottomOfResponse);
// Toggle show-checkmarks class at the container level for the accessibility setting,
// so child content parts can use CSS descendant selectors instead of each subscribing individually.
const updateContainerCheckmarks = () => templateData.rowContainer.classList.toggle('show-checkmarks', !!this.configService.getValue<boolean>(AccessibilityWorkbenchSettingId.ShowChatCheckmarks));
updateContainerCheckmarks();
templateData.elementDisposables.add(this.configService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration(AccessibilityWorkbenchSettingId.ShowChatCheckmarks)) {
updateContainerCheckmarks();
}
}));
if (!this.rendererOptions.noHeader) {
this.renderAvatar(element, templateData);
}
@@ -929,14 +940,6 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
dom.append(templateData.detail, $('span.confirmation-text', undefined, localize('chatConfirmationAction', 'Selected "{0}"', element.confirmation)));
templateData.header?.classList.remove('header-disabled');
templateData.header?.classList.add('partially-disabled');
const updateCheckmarks = () => templateData.detail.classList.toggle('show-checkmarks', !!this.configService.getValue<boolean>(AccessibilityWorkbenchSettingId.ShowChatCheckmarks));
updateCheckmarks();
templateData.elementDisposables.add(this.configService.onDidChangeConfiguration(e => {
if (e.affectsConfiguration(AccessibilityWorkbenchSettingId.ShowChatCheckmarks)) {
updateCheckmarks();
}
}));
}
}

2
src/vs/workbench/contrib/chat/browser/widget/media/chat.css
View File

@@ -2900,7 +2900,7 @@ have to be updated for changes to the rules above, or to support more deeply nes
display: none;
}
.interactive-item-container .header.partially-disabled .detail.show-checkmarks {
.interactive-item-container.show-checkmarks .header.partially-disabled .detail {
margin-left: 4px;
.codicon-check {