Skip to content

Commit de00cbf

Browse files
[CmdPal] Fix filters visibility on non-ListPage (#42828)
<!-- 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 This PR aims to fix the issue where filters from a ListPage remain visible when navigating to other pages. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #42827 - [ ] **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 ### Before: ![FiltersIssue](https://github.com/user-attachments/assets/b0ad6059-9a11-4e12-821d-7202358e25bb) ### After: ![FiltersFix](https://github.com/user-attachments/assets/b9ee71ee-cb5d-4ef9-b9fc-bc2e2a710b5c) <!-- 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 c4e96c7 commit de00cbf

File tree

6 files changed

+133
-3
lines changed

6 files changed

+133
-3
lines changed

src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/ListViewModel.cs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,17 @@ public ListViewModel(IListPage model, TaskScheduler scheduler, AppExtensionHost
9797
EmptyContent = new(new(null), PageContext);
9898
}
9999

100+
private void FiltersPropertyChanged(object? sender, System.ComponentModel.PropertyChangedEventArgs e)
101+
{
102+
if (e.PropertyName == nameof(FiltersViewModel.Filters))
103+
{
104+
var filtersViewModel = sender as FiltersViewModel;
105+
var hasFilters = filtersViewModel?.Filters.Length > 0;
106+
HasFilters = hasFilters;
107+
UpdateProperty(nameof(HasFilters));
108+
}
109+
}
110+
100111
// TODO: Does this need to hop to a _different_ thread, so that we don't block the extension while we're fetching?
101112
private void Model_ItemsChanged(object sender, IItemsChangedEventArgs args) => FetchItems();
102113

@@ -586,8 +597,11 @@ public override void InitializeProperties()
586597
EmptyContent = new(new(model.EmptyContent), PageContext);
587598
EmptyContent.SlowInitializeProperties();
588599

600+
Filters?.PropertyChanged -= FiltersPropertyChanged;
589601
Filters = new(new(model.Filters), PageContext);
590-
Filters.InitializeProperties();
602+
Filters?.PropertyChanged += FiltersPropertyChanged;
603+
604+
Filters?.InitializeProperties();
591605
UpdateProperty(nameof(Filters));
592606

593607
FetchItems();
@@ -686,8 +700,10 @@ protected override void FetchProperty(string propertyName)
686700
EmptyContent.SlowInitializeProperties();
687701
break;
688702
case nameof(Filters):
703+
Filters?.PropertyChanged -= FiltersPropertyChanged;
689704
Filters = new(new(model.Filters), PageContext);
690-
Filters.InitializeProperties();
705+
Filters?.PropertyChanged += FiltersPropertyChanged;
706+
Filters?.InitializeProperties();
691707
break;
692708
case nameof(IsLoading):
693709
UpdateEmptyContent();
@@ -757,6 +773,7 @@ protected override void UnsafeCleanup()
757773
FilteredItems.Clear();
758774
}
759775

776+
Filters?.PropertyChanged -= FiltersPropertyChanged;
760777
Filters?.SafeCleanup();
761778

762779
var model = _model.Unsafe;

src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/PageViewModel.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ public partial class PageViewModel : ExtensionObjectViewModel, IPageContext
7070

7171
public bool HasSearchBox { get; protected set; } = true;
7272

73+
public bool HasFilters { get; protected set; }
74+
7375
public IconInfoViewModel Icon { get; protected set; }
7476

7577
public PageViewModel(IPage? model, TaskScheduler scheduler, AppExtensionHost extensionHost)

src/modules/cmdpal/Microsoft.CmdPal.UI/Controls/FiltersDropDown.xaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@
7777
SelectedValue="{x:Bind ViewModel.CurrentFilter, Mode=OneWay}"
7878
SelectionChanged="FiltersComboBox_SelectionChanged"
7979
Style="{StaticResource ComboBoxStyle}"
80-
Visibility="{x:Bind ViewModel.ShouldShowFilters, Mode=OneWay, Converter={StaticResource BoolToVisibilityConverter}}">
80+
Visibility="{x:Bind ViewModel.ShouldShowFilters, Mode=OneWay, Converter={StaticResource BoolToVisibilityConverter}, FallbackValue=Collapsed}">
8181
<ComboBox.ItemContainerStyle>
8282
<Style BasedOn="{StaticResource DefaultComboBoxItemStyle}" TargetType="ComboBoxItem">
8383
<Setter Property="MinHeight" Value="0" />
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright (c) Microsoft Corporation
2+
// The Microsoft Corporation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using Microsoft.CommandPalette.Extensions;
6+
using Microsoft.CommandPalette.Extensions.Toolkit;
7+
8+
namespace SamplePagesExtension.Pages.IssueSpecificPages;
9+
10+
internal sealed partial class AllIssueSamplesIndexPage : ListPage
11+
{
12+
public AllIssueSamplesIndexPage()
13+
{
14+
Icon = new IconInfo("🐛");
15+
Name = "All Issue Samples Index Page";
16+
}
17+
18+
public override IListItem[] GetItems()
19+
{
20+
return new IListItem[]
21+
{
22+
new ListItem(new SamplePageForIssue42827_FilterDropDownStaysVisibleAfterSwitchingFromListToContentPage())
23+
{
24+
Title = "Issue 42827 - Filter Drop Down Stays Visible After Switching From List To Content Page",
25+
Subtitle = "Repro steps: Open this page, open the filter dropdown, select a filter, navigate to a content page, navigate back to this page. The filter dropdown should be closed but it remains open.",
26+
},
27+
};
28+
}
29+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// Copyright (c) Microsoft Corporation
2+
// The Microsoft Corporation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Linq;
6+
using Microsoft.CommandPalette.Extensions;
7+
using Microsoft.CommandPalette.Extensions.Toolkit;
8+
9+
namespace SamplePagesExtension.Pages.IssueSpecificPages;
10+
11+
internal sealed partial class SamplePageForIssue42827_FilterDropDownStaysVisibleAfterSwitchingFromListToContentPage : DynamicListPage
12+
{
13+
public SamplePageForIssue42827_FilterDropDownStaysVisibleAfterSwitchingFromListToContentPage()
14+
{
15+
Icon = new IconInfo(string.Empty);
16+
Name = "Issue 42827 - Filters not hiding when navigating between pages";
17+
IsLoading = true;
18+
var filters = new SampleFilters();
19+
filters.PropChanged += Filters_PropChanged;
20+
Filters = filters;
21+
}
22+
23+
private void Filters_PropChanged(object sender, IPropChangedEventArgs args) => RaiseItemsChanged();
24+
25+
public override void UpdateSearchText(string oldSearch, string newSearch) => RaiseItemsChanged();
26+
27+
public override IListItem[] GetItems()
28+
{
29+
var items = SearchText.ToCharArray().Select(ch => new ListItem(new SampleContentPage()) { Title = ch.ToString() }).ToArray();
30+
if (items.Length == 0)
31+
{
32+
items = [
33+
new ListItem(new SampleContentPage()) { Title = "This List item will open a content page" },
34+
new ListItem(new SampleContentPage()) { Title = "This List item will open a content page too" },
35+
new ListItem(new SampleContentPage()) { Title = "Guess what this one will do?" },
36+
];
37+
}
38+
39+
if (!string.IsNullOrEmpty(Filters.CurrentFilterId))
40+
{
41+
switch (Filters.CurrentFilterId)
42+
{
43+
case "mod2":
44+
items = items.Where((item, index) => (index + 1) % 2 == 0).ToArray();
45+
break;
46+
case "mod3":
47+
items = items.Where((item, index) => (index + 1) % 3 == 0).ToArray();
48+
break;
49+
case "all":
50+
default:
51+
// No filtering
52+
break;
53+
}
54+
}
55+
56+
foreach (var item in items)
57+
{
58+
item.Subtitle = "Filter drop-down should be hidden when navigating to a content page";
59+
}
60+
61+
return items;
62+
}
63+
64+
internal sealed partial class SampleFilters : Filters
65+
{
66+
public override IFilterItem[] GetFilters()
67+
{
68+
return
69+
[
70+
new Filter() { Id = "all", Name = "All" },
71+
new Filter() { Id = "mod2", Name = "Every 2nd", Icon = new IconInfo("2") },
72+
new Filter() { Id = "mod3", Name = "Every 3rd (and long name)", Icon = new IconInfo("3") },
73+
];
74+
}
75+
}
76+
}

src/modules/cmdpal/ext/SamplePagesExtension/SamplesListPage.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.CommandPalette.Extensions;
66
using Microsoft.CommandPalette.Extensions.Toolkit;
77
using SamplePagesExtension.Pages;
8+
using SamplePagesExtension.Pages.IssueSpecificPages;
89

910
namespace SamplePagesExtension;
1011

@@ -106,6 +107,11 @@ public partial class SamplesListPage : ListPage
106107
{
107108
Title = "Evil samples",
108109
Subtitle = "Samples designed to break the palette in many different evil ways",
110+
},
111+
new ListItem(new AllIssueSamplesIndexPage())
112+
{
113+
Title = "Issue-specific samples",
114+
Subtitle = "Samples designed to reproduce specific issues",
109115
}
110116
];
111117

0 commit comments

Comments
 (0)