From d531658b81d41249e314ecaa16e0a208d501850a Mon Sep 17 00:00:00 2001 From: Maruthan G Date: Fri, 1 May 2026 16:57:12 +0530 Subject: [PATCH] Allow removing notification sources from the DnD picker Adds a per-row trash button to the "Notifications: Toggle Do Not Disturb Mode By Source..." quick-pick. Triggering the button calls the existing INotificationService.removeFilter API and rebuilds the picker items minus the removed source while preserving the remaining selection. The same picker is also reached from the status-bar "Configure Do Not Disturb..." entry. Fixes #248913 --- .../notifications/notificationsCommands.ts | 27 +++++- .../test/common/notificationService.test.ts | 90 +++++++++++++++++++ 2 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 src/vs/workbench/services/notification/test/common/notificationService.test.ts diff --git a/src/vs/workbench/browser/parts/notifications/notificationsCommands.ts b/src/vs/workbench/browser/parts/notifications/notificationsCommands.ts index 4c01c42d61bd1..9213b1aa4b163 100644 --- a/src/vs/workbench/browser/parts/notifications/notificationsCommands.ts +++ b/src/vs/workbench/browser/parts/notifications/notificationsCommands.ts @@ -16,10 +16,11 @@ import { NotificationFocusedContext, NotificationsCenterVisibleContext, Notifica import { INotificationService, INotificationSourceFilter, NotificationsFilter } from '../../../../platform/notification/common/notification.js'; import { IInstantiationService, ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; import { ActionRunner, IAction, WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification } from '../../../../base/common/actions.js'; -import { IQuickInputService, IQuickPickItem } from '../../../../platform/quickinput/common/quickInput.js'; +import { IQuickInputButton, IQuickInputService, IQuickPickItem } from '../../../../platform/quickinput/common/quickInput.js'; import { DisposableStore } from '../../../../base/common/lifecycle.js'; import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../platform/accessibilitySignal/browser/accessibilitySignalService.js'; import { Codicon } from '../../../../base/common/codicons.js'; +import { ThemeIcon } from '../../../../base/common/themables.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; // Center @@ -285,6 +286,12 @@ export function registerNotificationCommands(center: INotificationsCenterControl const sortedFilters = notificationService.getFilters().sort((a, b) => a.label.localeCompare(b.label)); + const removeSourceButton: IQuickInputButton = { + iconClass: ThemeIcon.asClassName(Codicon.removeClose), + tooltip: localize('removeSource', "Remove Source"), + alwaysVisible: true + }; + const disposables = new DisposableStore(); const picker = disposables.add(quickInputService.createQuickPick()); @@ -292,7 +299,8 @@ export function registerNotificationCommands(center: INotificationsCenterControl id: source.id, label: source.label, tooltip: `${source.label} (${source.id})`, - filter: source.filter + filter: source.filter, + buttons: [removeSourceButton] })); picker.canSelectMany = true; @@ -301,6 +309,21 @@ export function registerNotificationCommands(center: INotificationsCenterControl picker.show(); + disposables.add(picker.onDidTriggerItemButton(event => { + if (event.button !== removeSourceButton || !event.item.id) { + return; + } + + notificationService.removeFilter(event.item.id); + + // Rebuild items so the removed entry disappears immediately + // while preserving the user's current selection for the rest. + const previousSelectedIds = new Set(picker.selectedItems.map(item => item.id)); + const remaining = picker.items.filter(item => item.id !== event.item.id); + picker.items = remaining; + picker.selectedItems = remaining.filter(item => previousSelectedIds.has(item.id)); + })); + disposables.add(picker.onDidAccept(async () => { for (const item of picker.items) { notificationService.setFilter({ diff --git a/src/vs/workbench/services/notification/test/common/notificationService.test.ts b/src/vs/workbench/services/notification/test/common/notificationService.test.ts new file mode 100644 index 0000000000000..4cb6333464f42 --- /dev/null +++ b/src/vs/workbench/services/notification/test/common/notificationService.test.ts @@ -0,0 +1,90 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { NotificationsFilter } from '../../../../../platform/notification/common/notification.js'; +import { TestStorageService } from '../../../../test/common/workbenchTestServices.js'; +import { NotificationService } from '../../common/notificationService.js'; + +suite('NotificationService - filters', () => { + + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + function createService(): NotificationService { + const storageService = disposables.add(new TestStorageService()); + return disposables.add(new NotificationService(storageService)); + } + + test('getFilters returns sources registered via setFilter', () => { + const service = createService(); + + service.setFilter({ id: 'ext.a', label: 'Extension A', filter: NotificationsFilter.OFF }); + service.setFilter({ id: 'ext.b', label: 'Extension B', filter: NotificationsFilter.ERROR }); + + const filters = service.getFilters().sort((a, b) => a.id.localeCompare(b.id)); + assert.strictEqual(filters.length, 2); + assert.strictEqual(filters[0].id, 'ext.a'); + assert.strictEqual(filters[0].filter, NotificationsFilter.OFF); + assert.strictEqual(filters[1].id, 'ext.b'); + assert.strictEqual(filters[1].filter, NotificationsFilter.ERROR); + }); + + test('removeFilter drops a previously tracked source', () => { + const service = createService(); + + service.setFilter({ id: 'ext.a', label: 'Extension A', filter: NotificationsFilter.OFF }); + service.setFilter({ id: 'ext.b', label: 'Extension B', filter: NotificationsFilter.ERROR }); + assert.strictEqual(service.getFilters().length, 2); + + service.removeFilter('ext.a'); + + const filters = service.getFilters(); + assert.strictEqual(filters.length, 1); + assert.strictEqual(filters[0].id, 'ext.b'); + + // After removal, the per-source filter falls back to the default (OFF). + assert.strictEqual(service.getFilter({ id: 'ext.a', label: 'Extension A' }), NotificationsFilter.OFF); + }); + + test('removeFilter is a no-op for unknown source ids', () => { + const service = createService(); + + service.setFilter({ id: 'ext.a', label: 'Extension A', filter: NotificationsFilter.ERROR }); + + service.removeFilter('ext.unknown'); + + const filters = service.getFilters(); + assert.strictEqual(filters.length, 1); + assert.strictEqual(filters[0].id, 'ext.a'); + assert.strictEqual(filters[0].filter, NotificationsFilter.ERROR); + }); + + test('removeFilter persists across service instances using the same storage', () => { + const storageService = disposables.add(new TestStorageService()); + + const first = disposables.add(new NotificationService(storageService)); + first.setFilter({ id: 'ext.a', label: 'Extension A', filter: NotificationsFilter.ERROR }); + first.setFilter({ id: 'ext.b', label: 'Extension B', filter: NotificationsFilter.OFF }); + first.removeFilter('ext.a'); + + // A fresh service backed by the same storage should not see the removed source. + const second = disposables.add(new NotificationService(storageService)); + const filters = second.getFilters(); + assert.strictEqual(filters.length, 1); + assert.strictEqual(filters[0].id, 'ext.b'); + }); + + test('removing the last filter leaves the picker source list empty', () => { + const service = createService(); + + service.setFilter({ id: 'ext.only', label: 'Only Extension', filter: NotificationsFilter.OFF }); + assert.strictEqual(service.getFilters().length, 1); + + service.removeFilter('ext.only'); + + assert.strictEqual(service.getFilters().length, 0); + }); +});