CmdPal: Fix FiltersViewModel binding #42467
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Removes runtime binding from FiltersDropDown control.
zadjii-msft
added
Hot Fix
Member
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
zadjii-msft
approved these changes
Oct 17, 2025
michaeljolley
approved these changes
Oct 17, 2025
khmyznikov
pushed a commit
that referenced
this pull request
Oct 20, 2025
## Summary of the Pull Request This PR resolves crashes on pages with filters, such as Windows Terminal profiles or Windows Services, when compiled with trimming/AOT. It removes runtime binding from the FiltersDropDown control, effectively preventing crashes caused by trimming/AOT dropping binding metadata for FilterItemViewModel. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #42428 - [x] Closes: #42482 - [x] Related to: #42458 - [ ] **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: Niels Laute <[email protected]>
Merged
khmyznikov
added a commit
that referenced
this pull request
Oct 21, 2025
Hotfixes #42467 #42434 #42405 #42399 --------- Co-authored-by: Jiří Polášek <[email protected]> Co-authored-by: Niels Laute <[email protected]> Co-authored-by: Jaylyn Barbee <[email protected]> Co-authored-by: Gordon Lam <[email protected]> Co-authored-by: Dustin L. Howett <[email protected]>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the filter selection mechanism in the Command Palette to bind directly to filter objects instead of using IDs. The change simplifies the XAML binding by removing the SelectedValuePath property and binding SelectedValue to the new CurrentFilter property instead of CurrentFilterId.
- Replaced XAML binding from ID-based to object-based selection in FiltersDropDown
- Added
CurrentFilterproperty toFiltersViewModelto track the selected filter object - Refactored
BuildFiltersmethod to return both filter items and the selected filter object in a tuple
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml | Simplified ComboBox binding to use CurrentFilter object directly instead of CurrentFilterId with SelectedValuePath |
| src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/FiltersViewModel.cs | Added CurrentFilter property and refactored BuildFilters to identify and return the selected filter object |
| })]; | ||
| } | ||
|
|
||
| return (items.ToArray(), selectedFilterItem ?? firstFilterItem); |
There was a problem hiding this comment.
When separators are present in the filters list, firstFilterItem might be null if the first item in the filters array is a separator (not an IFilter). This could return null as the selected filter even when there are valid filter items later in the list. Consider finding the first FilterItemViewModel in the items list instead of tracking it during iteration, or ensure firstFilterItem is only set once a valid filter is found.
| private (IFilterItemViewModel[] Items, IFilterItemViewModel? Selected) BuildFilters(IFilterItem[] filters, string currentFilterId) | ||
| { | ||
| return [..filters.Select<IFilterItem, IFilterItemViewModel>(filter => | ||
| if (filters is null || filters.Length == 0) |
There was a problem hiding this comment.
The null check for filters is redundant since the parameter is already guarded with the null-coalescing operator (filters ?? []) at the call site on line 39. Consider removing the null check and only checking for length.
Suggested change
| if (filters is null || filters.Length == 0) | |
| if (filters.Length == 0) |
mirmirmirr
pushed a commit
to mirmirmirr/PowerToys
that referenced
this pull request
Nov 9, 2025
## Summary of the Pull Request This PR resolves crashes on pages with filters, such as Windows Terminal profiles or Windows Services, when compiled with trimming/AOT. It removes runtime binding from the FiltersDropDown control, effectively preventing crashes caused by trimming/AOT dropping binding metadata for FilterItemViewModel. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: microsoft#42428 - [x] Closes: microsoft#42482 - [x] Related to: microsoft#42458 - [ ] **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: Niels Laute <[email protected]>
jiripolasek
deleted the
hotfix/cmdpal-filteritemviewmodel-not-bindable
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
This PR resolves crashes on pages with filters, such as Windows Terminal profiles or Windows Services, when compiled with trimming/AOT.
It removes runtime binding from the FiltersDropDown control, effectively preventing crashes caused by trimming/AOT dropping binding metadata for FilterItemViewModel.
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed