From 3f35b11cee91644e482469b5cfb410d331650a1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Tue, 24 Mar 2026 23:24:04 +0100 Subject: [PATCH] CmdPal: Fix missing primary context command for late-bound items (#46131) This PR does fix a bug where an item that starts with a null or empty primary command never adds that primary action to the context menu after the extension later provides a real command. - Creates the default primary context-menu item lazily when `Command` or `Command.Name` becomes available after `SlowInitializeProperties()` - Refreshes `AllCommands`, `SecondaryCommand`, and `HasMoreCommands` notifications for late command materialization and Show Details updates. - Adds unit tests to cover the fixed issue. ## Summary of the Pull Request ## PR Checklist - [x] Closes: #46129 - [ ] **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 ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed --- .../CommandItemViewModel.cs | 81 ++++++++++++++----- .../ListItemViewModel.cs | 2 + .../CommandItemViewModelTests.cs | 61 ++++++++++++++ 3 files changed, 124 insertions(+), 20 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs index 068748d585..e85d3b1fb1 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs @@ -213,22 +213,7 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa BuildAndInitMoreCommands(); - if (!string.IsNullOrEmpty(model.Command?.Name)) - { - _defaultCommandContextItemViewModel = new CommandContextItemViewModel(new CommandContextItem(model.Command!), PageContext) - { - _itemTitle = Name, - Subtitle = Subtitle, - Command = Command, - - // TODO this probably should just be a CommandContextItemViewModel(CommandItemViewModel) ctor, or a copy ctor or whatever - // Anything we set manually here must stay in sync with the corresponding properties on CommandItemViewModel. - }; - - // Only set the icon on the context item for us if our command didn't - // have its own icon - UpdateDefaultContextItemIcon(); - } + TryCreateDefaultCommandContextItem(model); lock (_moreCommandsLock) { @@ -238,6 +223,7 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa Initialized |= InitializedState.SelectionInitialized; UpdateProperty(nameof(MoreCommands)); UpdateProperty(nameof(AllCommands)); + UpdateProperty(nameof(SecondaryCommand), nameof(SecondaryCommandName), nameof(HasMoreCommands)); UpdateProperty(nameof(IsSelectedInitialized)); } @@ -335,9 +321,16 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa // or Command.Name change. This is a workaround to ensure that the Title is always up-to-date for extensions with old SDK. _itemTitle = model.Title; - _defaultCommandContextItemViewModel?.Command = Command; - _defaultCommandContextItemViewModel?.UpdateTitle(_itemTitle); - UpdateDefaultContextItemIcon(); + if (_defaultCommandContextItemViewModel is not null) + { + _defaultCommandContextItemViewModel.Command = Command; + _defaultCommandContextItemViewModel.UpdateTitle(_itemTitle); + UpdateDefaultContextItemIcon(); + } + else + { + TryCreateDefaultCommandContextItem(model); + } UpdateProperty(nameof(Name)); UpdateProperty(nameof(Title)); @@ -403,7 +396,15 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa _titleCache.Invalidate(); UpdateProperty(nameof(Title), nameof(Name)); - _defaultCommandContextItemViewModel?.UpdateTitle(model.Command.Name); + if (_defaultCommandContextItemViewModel is not null) + { + _defaultCommandContextItemViewModel.UpdateTitle(model.Command.Name); + } + else + { + TryCreateDefaultCommandContextItem(model); + } + break; case nameof(Command.Icon): @@ -413,6 +414,46 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa } } + /// + /// Creates when it does not exist + /// yet and the current command has a non-empty name. This covers the case + /// where an extension initially exposes a NoOpCommand (empty name) + /// and later switches to a concrete command after has already run. + /// When a new instance is created, the snapshot is refreshed and + /// is notified. + /// + private void TryCreateDefaultCommandContextItem(ICommandItem model) + { + if (_defaultCommandContextItemViewModel is not null) + { + return; + } + + if (string.IsNullOrEmpty(model.Command?.Name)) + { + return; + } + + _defaultCommandContextItemViewModel = new CommandContextItemViewModel(new CommandContextItem(model.Command!), PageContext) + { + _itemTitle = Name, + Subtitle = Subtitle, + Command = Command, + + // TODO this probably should just be a CommandContextItemViewModel(CommandItemViewModel) ctor, or a copy ctor or whatever + // Anything we set manually here must stay in sync with the corresponding properties on CommandItemViewModel. + }; + + UpdateDefaultContextItemIcon(); + + lock (_moreCommandsLock) + { + RefreshMoreCommandStateUnsafe(); + } + + UpdateProperty(nameof(AllCommands)); + } + private void UpdateDefaultContextItemIcon() => // Command icon takes precedence over our icon on the primary command diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListItemViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListItemViewModel.cs index 0cda382191..518dd0bc63 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListItemViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/ListItemViewModel.cs @@ -214,6 +214,7 @@ public partial class ListItemViewModel : CommandItemViewModel if (addedCommand) { UpdateProperty(nameof(MoreCommands), nameof(AllCommands)); + UpdateProperty(nameof(SecondaryCommand), nameof(SecondaryCommandName), nameof(HasMoreCommands)); } } } @@ -252,6 +253,7 @@ public partial class ListItemViewModel : CommandItemViewModel oldCommand?.SafeCleanup(); UpdateProperty(nameof(MoreCommands), nameof(AllCommands)); + UpdateProperty(nameof(SecondaryCommand), nameof(SecondaryCommandName), nameof(HasMoreCommands)); } } diff --git a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/CommandItemViewModelTests.cs b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/CommandItemViewModelTests.cs index e4c65ecfaf..882352f01a 100644 --- a/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/CommandItemViewModelTests.cs +++ b/src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/CommandItemViewModelTests.cs @@ -4,6 +4,7 @@ using System; using System.Threading.Tasks; +using Microsoft.CmdPal.Common.Text; using Microsoft.CmdPal.UI.ViewModels.Models; using Microsoft.CommandPalette.Extensions.Toolkit; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -76,4 +77,64 @@ public class CommandItemViewModelTests Assert.IsNotNull(viewModel.SecondaryCommand); Assert.AreEqual("Secondary", viewModel.SecondaryCommand.Name); } + + [TestMethod] + public void LatePrimaryCommandCreation_AddsPrimaryToAllCommands() + { + // Reproduces issue where SlowInitializeProperties runs before a real primary command exists. + // The late-arriving command should still create the synthetic primary context item and prepend it to AllCommands. + var pageContext = new TestPageContext(); + var item = new CommandItem() + { + Command = null, + MoreCommands = + [ + new CommandContextItem(new NoOpCommand { Name = "Secondary" }), + ], + }; + + var viewModel = new CommandItemViewModel(new(item), new(pageContext), DefaultContextMenuFactory.Instance); + viewModel.SlowInitializeProperties(); + + Assert.AreEqual(1, viewModel.AllCommands.Count); + Assert.AreEqual("Secondary", ((CommandContextItemViewModel)viewModel.AllCommands[0]).Name); + + item.Command = new NoOpCommand { Name = "Primary" }; + + Assert.AreEqual(2, viewModel.AllCommands.Count); + Assert.AreEqual("Primary", ((CommandContextItemViewModel)viewModel.AllCommands[0]).Name); + Assert.AreEqual("Secondary", ((CommandContextItemViewModel)viewModel.AllCommands[1]).Name); + Assert.IsTrue(viewModel.HasMoreCommands); + Assert.AreEqual("Secondary", viewModel.SecondaryCommand?.Name); + } + + [TestMethod] + public void SyntheticPrimaryContextItem_UpdatesSubtitleAndCachedSubtitleTarget() + { + // The synthetic primary context item copies subtitle state from the parent CommandItemViewModel. + // When subtitle changes later, both the exposed subtitle and its cached fuzzy-search target must refresh. + var pageContext = new TestPageContext(); + var item = new CommandItem(new NoOpCommand { Name = "Primary" }) + { + Subtitle = "before", + MoreCommands = + [ + new CommandContextItem(new NoOpCommand { Name = "Secondary" }), + ], + }; + + var viewModel = new CommandItemViewModel(new(item), new(pageContext), DefaultContextMenuFactory.Instance); + viewModel.SlowInitializeProperties(); + + var primaryContextItem = (CommandContextItemViewModel)viewModel.AllCommands[0]; + var matcher = new PrecomputedFuzzyMatcher(new PrecomputedFuzzyMatcherOptions()); + + Assert.AreEqual("before", primaryContextItem.Subtitle); + Assert.AreEqual("before", primaryContextItem.GetSubtitleTarget(matcher).Original); + + item.Subtitle = "after unique"; + + Assert.AreEqual("after unique", primaryContextItem.Subtitle); + Assert.AreEqual("after unique", primaryContextItem.GetSubtitleTarget(matcher).Original); + } }