From 76fb46483248bf057d496104a7ac134505ce682e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Thu, 18 Sep 2025 05:59:44 +0200 Subject: [PATCH] CmdPal: Bind FilterDropDown selection to the current filter and ensure notifications are raised on UI thread (#41808) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request This PR declaratively binds FilterDropDown.SelectedValue to CurrentFilterId (one-way only; updates in the opposite direction are handled within the drop-down’s code). It also removes observable properties and reverts to the UpdateProperty style to ensure property change notifications are raised on the UI thread, aligning the handling style with other classes. ## Impact - Fixed a crash that could occur on pages with filters - The filter drop-down now correctly syncs with the initially selected filter when loading a page ## PR Checklist - [x] Closes: #41578 - [x] Closes: #41649 - [ ] **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 --- .../ExtensionObjectViewModel.cs | 30 +++++++++++ .../FiltersViewModel.cs | 54 ++++++++++--------- .../Controls/FiltersDropDown.xaml | 2 + 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ExtensionObjectViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ExtensionObjectViewModel.cs index f0bb9de118..1c056c4806 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ExtensionObjectViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/ExtensionObjectViewModel.cs @@ -51,6 +51,36 @@ public abstract partial class ExtensionObjectViewModel : ObservableObject DoOnUiThread(() => OnPropertyChanged(propertyName)); } + protected void UpdateProperty(string propertyName1, string propertyName2) + { + DoOnUiThread(() => + { + OnPropertyChanged(propertyName1); + OnPropertyChanged(propertyName2); + }); + } + + protected void UpdateProperty(string propertyName1, string propertyName2, string propertyName3) + { + DoOnUiThread(() => + { + OnPropertyChanged(propertyName1); + OnPropertyChanged(propertyName2); + OnPropertyChanged(propertyName3); + }); + } + + protected void UpdateProperty(params string[] propertyNames) + { + DoOnUiThread(() => + { + foreach (var propertyName in propertyNames) + { + OnPropertyChanged(propertyName); + } + }); + } + protected void ShowException(Exception ex, string? extensionHint = null) { if (PageContext.TryGetTarget(out var pageContext)) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/FiltersViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/FiltersViewModel.cs index 511581558c..191b0a5ec5 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/FiltersViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.Core.ViewModels/FiltersViewModel.cs @@ -2,7 +2,6 @@ // The Microsoft Corporation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using CommunityToolkit.Mvvm.ComponentModel; using Microsoft.CmdPal.Core.ViewModels.Models; using Microsoft.CommandPalette.Extensions; @@ -10,14 +9,11 @@ namespace Microsoft.CmdPal.Core.ViewModels; public partial class FiltersViewModel : ExtensionObjectViewModel { - private readonly ExtensionObject _filtersModel = new(null); + private readonly ExtensionObject _filtersModel; - [ObservableProperty] - public partial string CurrentFilterId { get; set; } = string.Empty; + public string CurrentFilterId { get; private set; } = string.Empty; - [ObservableProperty] - [NotifyPropertyChangedFor(nameof(ShouldShowFilters))] - public partial IFilterItemViewModel[] Filters { get; set; } = []; + public IFilterItemViewModel[] Filters { get; private set; } = []; public bool ShouldShowFilters => Filters.Length > 0; @@ -34,23 +30,11 @@ public partial class FiltersViewModel : ExtensionObjectViewModel if (_filtersModel.Unsafe is not null) { var filters = _filtersModel.Unsafe.GetFilters(); - Filters = filters.Select(filter => - { - var filterItem = filter as IFilter; - if (filterItem != null) - { - var filterVM = new FilterItemViewModel(filterItem!, PageContext); - filterVM.InitializeProperties(); + Filters = BuildFilters(filters ?? []); + UpdateProperty(nameof(Filters), nameof(ShouldShowFilters)); - return filterVM; - } - else - { - return new SeparatorViewModel(); - } - }).ToArray(); - - CurrentFilterId = _filtersModel.Unsafe.CurrentFilterId; + CurrentFilterId = _filtersModel.Unsafe.CurrentFilterId ?? string.Empty; + UpdateProperty(nameof(CurrentFilterId)); return; } @@ -61,7 +45,27 @@ public partial class FiltersViewModel : ExtensionObjectViewModel } Filters = []; + UpdateProperty(nameof(Filters), nameof(ShouldShowFilters)); + CurrentFilterId = string.Empty; + UpdateProperty(nameof(CurrentFilterId)); + } + + private IFilterItemViewModel[] BuildFilters(IFilterItem[] filters) + { + return [..filters.Select(filter => + { + if (filter is IFilter filterItem) + { + var filterItemViewModel = new FilterItemViewModel(filterItem!, PageContext); + filterItemViewModel.InitializeProperties(); + return filterItemViewModel; + } + else + { + return new SeparatorViewModel(); + } + })]; } public override void SafeCleanup() @@ -70,9 +74,9 @@ public partial class FiltersViewModel : ExtensionObjectViewModel foreach (var filter in Filters) { - if (filter is FilterItemViewModel filterVM) + if (filter is FilterItemViewModel filterItemViewModel) { - filterVM.SafeCleanup(); + filterItemViewModel.SafeCleanup(); } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml index dec0920235..ee05ea61e8 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml @@ -74,6 +74,8 @@ ItemsSource="{x:Bind ViewModel.Filters, Mode=OneWay}" PlaceholderText="Filters" PreviewKeyDown="FiltersComboBox_PreviewKeyDown" + SelectedValue="{x:Bind ViewModel.CurrentFilterId, Mode=OneWay}" + SelectedValuePath="Id" SelectionChanged="FiltersComboBox_SelectionChanged" Style="{StaticResource ComboBoxStyle}" Visibility="{x:Bind ViewModel.ShouldShowFilters, Mode=OneWay, Converter={StaticResource BoolToVisibilityConverter}}">