Skip to content

Commit 2cf7d0f

Browse files
Fix for duplicated dockbands (#46438)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request CommandProviderWrapper.PinDockBand had no duplicate check and it blindly called .Add() on the settings list every time. This allowed the same extension to be pinned multiple times to the dock. Once duplicates existed in settings, LoadTopLevelCommands would faithfully re-create all of them on every CommandsChanged reload, making edit-mode unpin appear broken (unpin removes one, reload brings them all back). **Fix** - CommandProviderWrapper.PinDockBand: Check all three band lists (Start/Center/End) for an existing (ProviderId, CommandId) match before adding; early-return if already pinned. - CommandProviderWrapper.LoadTopLevelCommands: Track seen command IDs in a HashSet when iterating AllPinnedCommands; skip duplicates so they never materialize in the UI even if present in settings. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [ ] Closes: #xxx <!-- - [ ] Closes: #yyy (add separate lines for additional resolved issues) --> - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed --------- Co-authored-by: Jiří Polášek <me@jiripolasek.com>
1 parent 7cb0f38 commit 2cf7d0f

File tree

2 files changed

+38
-1
lines changed

2 files changed

+38
-1
lines changed

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,19 @@ private void InitializeCommands(
301301
var dockSettings = settings.DockSettings;
302302
var allPinnedCommands = dockSettings.AllPinnedCommands;
303303
var pinnedBandsForThisProvider = allPinnedCommands.Where(c => c.ProviderId == ProviderId);
304+
305+
// Track which command IDs we've already added to avoid duplicates
306+
// from settings that were pinned multiple times.
307+
HashSet<string> seenCommandIds = new(bands.Select(b => b.Id));
308+
304309
foreach (var (providerId, commandId) in pinnedBandsForThisProvider)
305310
{
311+
if (!seenCommandIds.Add(commandId))
312+
{
313+
Logger.LogWarning($"Skipping duplicate pinned dock band command {commandId} for provider {providerId}");
314+
continue;
315+
}
316+
306317
Logger.LogDebug($"Looking for pinned dock band command {commandId} for provider {providerId}");
307318

308319
// First, try to lookup the command as one of this provider's
@@ -440,6 +451,17 @@ public void PinDockBand(string commandId, IServiceProvider serviceProvider, Dock
440451
{
441452
var settingsService = serviceProvider.GetRequiredService<ISettingsService>();
442453
var settings = settingsService.Settings;
454+
var dockSettings = settings.DockSettings;
455+
456+
// Prevent duplicate pins — check all sections
457+
if (dockSettings.StartBands.Any(b => b.CommandId == commandId && b.ProviderId == this.ProviderId) ||
458+
dockSettings.CenterBands.Any(b => b.CommandId == commandId && b.ProviderId == this.ProviderId) ||
459+
dockSettings.EndBands.Any(b => b.CommandId == commandId && b.ProviderId == this.ProviderId))
460+
{
461+
Logger.LogDebug($"Dock band '{commandId}' from provider '{this.ProviderId}' is already pinned; skipping.");
462+
return;
463+
}
464+
443465
var bandSettings = new DockBandSettings
444466
{
445467
CommandId = commandId,

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Dock/DockViewModel.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public sealed partial class DockViewModel
2121
private readonly IContextMenuFactory _contextMenuFactory;
2222

2323
private DockSettings _settings;
24+
private bool _isEditing;
2425

2526
public TaskScheduler Scheduler { get; }
2627

@@ -52,6 +53,12 @@ public DockViewModel(
5253

5354
private void DockBands_CollectionChanged(object? sender, System.Collections.Specialized.NotifyCollectionChangedEventArgs e)
5455
{
56+
if (_isEditing)
57+
{
58+
Logger.LogDebug("Skipping DockBands_CollectionChanged during edit mode");
59+
return;
60+
}
61+
5562
Logger.LogDebug("Starting DockBands_CollectionChanged");
5663
SetupBands();
5764
Logger.LogDebug("Ended DockBands_CollectionChanged");
@@ -302,7 +309,12 @@ public void SaveBandOrder()
302309
_snapshotCenterBands = null;
303310
_snapshotEndBands = null;
304311
_snapshotBandViewModels = null;
305-
_settingsService.Save();
312+
313+
// Save without hotReload to avoid triggering SettingsChanged → SetupBands,
314+
// which could race with stale DockBands_CollectionChanged work items and
315+
// re-add bands that were just unpinned.
316+
_settingsService.Save(hotReload: false);
317+
_isEditing = false;
306318
Logger.LogDebug("Saved band order to settings");
307319
}
308320

@@ -317,6 +329,8 @@ public void SaveBandOrder()
317329
/// </summary>
318330
public void SnapshotBandOrder()
319331
{
332+
_isEditing = true;
333+
320334
var dockSettings = _settingsService.Settings.DockSettings;
321335
_snapshotStartBands = dockSettings.StartBands.Select(b => b.Clone()).ToList();
322336
_snapshotCenterBands = dockSettings.CenterBands.Select(b => b.Clone()).ToList();
@@ -391,6 +405,7 @@ public void RestoreBandOrder()
391405
_snapshotCenterBands = null;
392406
_snapshotEndBands = null;
393407
_snapshotBandViewModels = null;
408+
_isEditing = false;
394409
Logger.LogDebug("Restored band order from snapshot");
395410
}
396411

0 commit comments

Comments
 (0)