Skip to content

Allow removing notification sources from the DnD picker#313708

Open
maruthang wants to merge 1 commit intomicrosoft:mainfrom
maruthang:fix/issue-248913-remove-notification-filter-source
Open

Allow removing notification sources from the DnD picker#313708
maruthang wants to merge 1 commit intomicrosoft:mainfrom
maruthang:fix/issue-248913-remove-notification-filter-source

Conversation

@maruthang
Copy link
Copy Markdown
Contributor

Summary

Adds a per-row "Remove Source" trash button to the "Notifications: Toggle Do Not Disturb Mode By Source..." quick-pick (and the status-bar "Configure Do Not Disturb..." entry, which opens the same picker), implementing the action @bpasero suggested in #248913.

Fixes #248913

Implementation

  • A new removeSourceButton: IQuickInputButton is added to every quick-pick item, using Codicon.removeClose, a localized "Remove Source" tooltip, and alwaysVisible: true.
  • A picker.onDidTriggerItemButton handler:
    1. Calls the existing INotificationService.removeFilter(item.id) API (no new service surface — removeFilter is already wired through to storage and is what extension-uninstall flows already use internally).
    2. Rebuilds picker.items minus the removed entry.
    3. Preserves the user's existing checkbox selection for the surviving rows by id.
  • The existing onDidAccept handler is unchanged. Because it iterates picker.items, the removed source is not re-added on accept — it simply disappears from the list. This matches the user's expectation that "Remove" means "forget this source entirely."
  • Removal is triggered by explicit user action only; existing automatic behavior (sources being re-added when an extension that posts a notification reappears) is unchanged.

Test plan

  • New unit tests in notificationService.test.ts cover the removeFilter/getFilters API the picker depends on:
    • getFilters returns sources registered via setFilter.
    • removeFilter drops a tracked source and reverts its per-source filter to the default.
    • removeFilter is a no-op for unknown ids.
    • Removal persists across NotificationService instances backed by the same storage.
    • Removing the last filter leaves the source list empty.
  • Manual: install an extension that posts a notification, run "Notifications: Toggle Do Not Disturb Mode By Source...", click the trash icon on the row — row disappears, picker stays open with surviving rows selectable.
  • Manual: uninstall the extension, re-open the picker — the source no longer appears in the list (even though it was previously stored).
  • Manual: removing the last source leaves the picker empty but accepts cleanly.

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 microsoft#248913
Copilot AI review requested due to automatic review settings May 1, 2026 11:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow to remove notification sources from the filter list

4 participants