From f81802430c9f83466c5ee28d987fb98dbc2bd0f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Mon, 28 Jul 2025 23:40:29 +0200 Subject: [PATCH] CmdPal: Handle CommandItem Title changes properly and raise notification every time it changes (#40513) ## Summary of the Pull Request ## PR Checklist - [x] **Closes:** #39167 - [ ] **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 | 12 +++ .../CommandItem.cs | 33 +++++++- .../WeakEventListener`3.cs | 80 +++++++++++++++++++ 3 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/WeakEventListener`3.cs diff --git a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/CommandItemViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/CommandItemViewModel.cs index 75a8eb9a56..247814e5e6 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/CommandItemViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/CommandItemViewModel.cs @@ -313,6 +313,10 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa Command = new(model.Command, PageContext); Command.InitializeProperties(); + + // Extensions based on Command Palette SDK < 0.3 CommandItem class won't notify when Title changes because Command + // 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; UpdateProperty(nameof(Name)); UpdateProperty(nameof(Title)); UpdateProperty(nameof(Icon)); @@ -385,6 +389,14 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel, ICommandBa switch (propertyName) { case nameof(Command.Name): + // Extensions based on Command Palette SDK < 0.3 CommandItem class won't notify when Title changes because Command + // or Command.Name change. This is a workaround to ensure that the Title is always up-to-date for extensions with old SDK. + var model = _commandItemModel.Unsafe; + if (model != null) + { + _itemTitle = model.Title; + } + UpdateProperty(nameof(Title)); UpdateProperty(nameof(Name)); break; diff --git a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/CommandItem.cs b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/CommandItem.cs index f421622f94..153c4cda4f 100644 --- a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/CommandItem.cs +++ b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/CommandItem.cs @@ -7,6 +7,8 @@ namespace Microsoft.CommandPalette.Extensions.Toolkit; public partial class CommandItem : BaseObservable, ICommandItem { private ICommand? _command; + private WeakEventListener? _commandListener; + private string _title = string.Empty; public virtual IIconInfo? Icon { @@ -20,17 +22,15 @@ public partial class CommandItem : BaseObservable, ICommandItem public virtual string Title { - get => !string.IsNullOrEmpty(field) ? field : _command?.Name ?? string.Empty; + get => !string.IsNullOrEmpty(_title) ? _title : _command?.Name ?? string.Empty; set { - field = value; + _title = value; OnPropertyChanged(nameof(Title)); } } -= string.Empty; - public virtual string Subtitle { get; @@ -48,8 +48,33 @@ public partial class CommandItem : BaseObservable, ICommandItem get => _command; set { + if (_commandListener != null) + { + _commandListener.Detach(); + _commandListener = null; + } + _command = value; + + if (value != null) + { + _commandListener = new(this, OnCommandPropertyChanged, listener => value.PropChanged -= listener.OnEvent); + value.PropChanged += _commandListener.OnEvent; + } + OnPropertyChanged(nameof(Command)); + if (string.IsNullOrWhiteSpace(_title)) + { + OnPropertyChanged(nameof(Title)); + } + } + } + + private static void OnCommandPropertyChanged(CommandItem instance, object source, IPropChangedEventArgs args) + { + if (args.PropertyName == nameof(ICommand.Name)) + { + instance.OnPropertyChanged(nameof(Title)); } } diff --git a/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/WeakEventListener`3.cs b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/WeakEventListener`3.cs new file mode 100644 index 0000000000..cd3a62a079 --- /dev/null +++ b/src/modules/cmdpal/extensionsdk/Microsoft.CommandPalette.Extensions.Toolkit/WeakEventListener`3.cs @@ -0,0 +1,80 @@ +// Copyright (c) Microsoft Corporation +// The Microsoft Corporation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.ComponentModel; + +namespace Microsoft.CommandPalette.Extensions.Toolkit; + +/// +/// Implements a weak event listener that allows the owner to be garbage +/// collected if its only remaining link is an event handler. +/// +/// Type of instance listening for the event. +/// Type of source for the event. +/// Type of event arguments for the event. +[EditorBrowsable(EditorBrowsableState.Never)] +internal sealed class WeakEventListener + where TInstance : class +{ + /// + /// WeakReference to the instance listening for the event. + /// + private readonly WeakReference _weakInstance; + + /// + /// Initializes a new instance of the class. + /// + /// Instance subscribing to the event. + /// Event handler executed when event is raised. + /// Action to execute when instance was collected. + public WeakEventListener( + TInstance instance, + Action? onEventAction = null, + Action>? onDetachAction = null) + { + ArgumentNullException.ThrowIfNull(instance); + + _weakInstance = new(instance); + OnEventAction = onEventAction; + OnDetachAction = onDetachAction; + } + + /// + /// Gets or sets the method to call when the event fires. + /// + public Action? OnEventAction { get; set; } + + /// + /// Gets or sets the method to call when detaching from the event. + /// + public Action>? OnDetachAction { get; set; } + + /// + /// Handler for the subscribed event calls OnEventAction to handle it. + /// + /// Event source. + /// Event arguments. + public void OnEvent(TSource source, TEventArgs eventArgs) + { + if (_weakInstance.TryGetTarget(out var target)) + { + // Call registered action + OnEventAction?.Invoke(target, source, eventArgs); + } + else + { + // Detach from event + Detach(); + } + } + + /// + /// Detaches from the subscribed event. + /// + public void Detach() + { + OnDetachAction?.Invoke(this); + OnDetachAction = null; + } +}