From 4d3c223402bb037851b76f77dc143793eda88981 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Mon, 1 Dec 2025 02:32:30 +0100 Subject: [PATCH] CmdPal: Fix grid views (#43991) ## Summary of the Pull Request This PR fixes the crash due to binding to a trimmed property. For this it converts runtime bindings on GridView to use `{x:Bind}` so this issue can't happen in the future. - Fixes a crash related to the `Visibility` property in gallery/grid views when trimmed during AOT builds. - Fixes ShowTitle and ShowSubtitle properties, they are now taken into account in a view. - Improves UI layout, removes some margins and maches the corner radius of the item contaienr with the item content in the gallery view. - Refactores gallery and grid views to move logic from the view to the view model so we can x:Bind to them. - Replaces `{Binding}` with `{x:Bind}` to improve performance and enable compile-time binding validation. - Properties related to grids are splatted on to the common `IGridPropertiesViewModel` interface. Subclassing would add extra overhead without substential benefit. - Adds new samples to showcase various grid view configurations. ## Pictures? Pictures! A) Gallery view (with title and subtitle) image B) Gallery view (only title) image C) Gallery view (no title or subtitle) image D) Small icons image E) Medium icons (with labels) image F) Medium icons (no labels) image ## PR Checklist - [x] Closes: #43973 - [ ] **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 --- .../GalleryGridPropertiesViewModel.cs | 8 +- .../IGridPropertiesViewModel.cs | 4 + .../ListItemViewModel.cs | 96 ++++++-- .../ListViewModel.cs | 45 ++-- .../MediumGridPropertiesViewModel.cs | 6 +- .../SmallGridPropertiesViewModel.cs | 4 + .../GridItemContainerStyleSelector.cs | 31 +++ .../Converters/GridItemTemplateSelector.cs | 21 +- .../ExtViews/ListPage.xaml | 231 ++++++++++++++---- .../Helpers/BindTransformers.cs | 3 + .../Pages/SampleGalleryListPage.cs | 7 - .../Pages/SampleGridsListPage.cs | 59 +++++ .../SamplePagesExtension/SamplesListPage.cs | 4 +- 13 files changed, 404 insertions(+), 115 deletions(-) create mode 100644 src/modules/cmdpal/Microsoft.CmdPal.UI/Converters/GridItemContainerStyleSelector.cs create mode 100644 src/modules/cmdpal/ext/SamplePagesExtension/Pages/SampleGridsListPage.cs diff --git a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/GalleryGridPropertiesViewModel.cs b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/GalleryGridPropertiesViewModel.cs index 9d360109dc..85d85838ac 100644 --- a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/GalleryGridPropertiesViewModel.cs +++ b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/GalleryGridPropertiesViewModel.cs @@ -11,15 +11,15 @@ public class GalleryGridPropertiesViewModel : IGridPropertiesViewModel { private readonly ExtensionObject _model; + public bool ShowTitle { get; private set; } + + public bool ShowSubtitle { get; private set; } + public GalleryGridPropertiesViewModel(IGalleryGridLayout galleryGridLayout) { _model = new(galleryGridLayout); } - public bool ShowTitle { get; set; } - - public bool ShowSubtitle { get; set; } - public void InitializeProperties() { var model = _model.Unsafe; diff --git a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/IGridPropertiesViewModel.cs b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/IGridPropertiesViewModel.cs index ea3d6027d3..ec14bbdde3 100644 --- a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/IGridPropertiesViewModel.cs +++ b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/IGridPropertiesViewModel.cs @@ -6,5 +6,9 @@ namespace Microsoft.CmdPal.Core.ViewModels; public interface IGridPropertiesViewModel { + bool ShowTitle { get; } + + bool ShowSubtitle { get; } + void InitializeProperties(); } diff --git a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/ListItemViewModel.cs b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/ListItemViewModel.cs index 8850d5778b..a400374e3c 100644 --- a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/ListItemViewModel.cs +++ b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/ListItemViewModel.cs @@ -10,10 +10,9 @@ using Microsoft.CommandPalette.Extensions.Toolkit; namespace Microsoft.CmdPal.Core.ViewModels; -public partial class ListItemViewModel(IListItem model, WeakReference context) - : CommandItemViewModel(new(model), context) +public partial class ListItemViewModel : CommandItemViewModel { - public new ExtensionObject Model { get; } = new(model); + public new ExtensionObject Model { get; } public List? Tags { get; set; } @@ -32,6 +31,40 @@ public partial class ListItemViewModel(IListItem model, WeakReference context) + : base(new(model), context) + { + Model = new ExtensionObject(model); + } + public override void InitializeProperties() { if (IsInitialized) @@ -93,16 +126,18 @@ public partial class ListItemViewModel(IListItem model, WeakReference FilteredItems { get; set; } = []; + public ObservableCollection FilteredItems { get; } = []; public FiltersViewModel? Filters { get; set; } @@ -224,6 +223,8 @@ public partial class ListViewModel : PageViewModel, IDisposable // TODO we can probably further optimize this by also keeping a // HashSet of every ExtensionObject we currently have, and only // building new viewmodels for the ones we haven't already built. + var showsTitle = GridProperties?.ShowTitle ?? true; + var showsSubtitle = GridProperties?.ShowSubtitle ?? true; foreach (var item in newItems) { // Check for cancellation during item processing @@ -237,6 +238,8 @@ public partial class ListViewModel : PageViewModel, IDisposable // If an item fails to load, silently ignore it. if (viewModel.SafeFastInit()) { + viewModel.LayoutShowsTitle = showsTitle; + viewModel.LayoutShowsSubtitle = showsSubtitle; newViewModels.Add(viewModel); } } @@ -583,6 +586,7 @@ public partial class ListViewModel : PageViewModel, IDisposable GridProperties = LoadGridPropertiesViewModel(model.GridProperties); GridProperties?.InitializeProperties(); UpdateProperty(nameof(GridProperties)); + ApplyLayoutToItems(); ShowDetails = model.ShowDetails; UpdateProperty(nameof(ShowDetails)); @@ -608,22 +612,15 @@ public partial class ListViewModel : PageViewModel, IDisposable model.ItemsChanged += Model_ItemsChanged; } - private IGridPropertiesViewModel? LoadGridPropertiesViewModel(IGridProperties? gridProperties) + private static IGridPropertiesViewModel? LoadGridPropertiesViewModel(IGridProperties? gridProperties) { - if (gridProperties is IMediumGridLayout mediumGridLayout) + return gridProperties switch { - return new MediumGridPropertiesViewModel(mediumGridLayout); - } - else if (gridProperties is IGalleryGridLayout galleryGridLayout) - { - return new GalleryGridPropertiesViewModel(galleryGridLayout); - } - else if (gridProperties is ISmallGridLayout smallGridLayout) - { - return new SmallGridPropertiesViewModel(smallGridLayout); - } - - return null; + IMediumGridLayout mediumGridLayout => new MediumGridPropertiesViewModel(mediumGridLayout), + IGalleryGridLayout galleryGridLayout => new GalleryGridPropertiesViewModel(galleryGridLayout), + ISmallGridLayout smallGridLayout => new SmallGridPropertiesViewModel(smallGridLayout), + _ => null, + }; } public void LoadMoreIfNeeded() @@ -685,6 +682,7 @@ public partial class ListViewModel : PageViewModel, IDisposable GridProperties = LoadGridPropertiesViewModel(model.GridProperties); GridProperties?.InitializeProperties(); UpdateProperty(nameof(IsGridView)); + ApplyLayoutToItems(); break; case nameof(ShowDetails): ShowDetails = model.ShowDetails; @@ -730,6 +728,21 @@ public partial class ListViewModel : PageViewModel, IDisposable }); } + private void ApplyLayoutToItems() + { + lock (_listLock) + { + var showsTitle = GridProperties?.ShowTitle ?? true; + var showsSubtitle = GridProperties?.ShowSubtitle ?? true; + + foreach (var item in Items) + { + item.LayoutShowsTitle = showsTitle; + item.LayoutShowsSubtitle = showsSubtitle; + } + } + } + public void Dispose() { GC.SuppressFinalize(this); diff --git a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/MediumGridPropertiesViewModel.cs b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/MediumGridPropertiesViewModel.cs index 57150bbd0d..2059e1547b 100644 --- a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/MediumGridPropertiesViewModel.cs +++ b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/MediumGridPropertiesViewModel.cs @@ -11,13 +11,15 @@ public class MediumGridPropertiesViewModel : IGridPropertiesViewModel { private readonly ExtensionObject _model; + public bool ShowTitle { get; private set; } + + public bool ShowSubtitle => false; + public MediumGridPropertiesViewModel(IMediumGridLayout mediumGridLayout) { _model = new(mediumGridLayout); } - public bool ShowTitle { get; set; } - public void InitializeProperties() { var model = _model.Unsafe; diff --git a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/SmallGridPropertiesViewModel.cs b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/SmallGridPropertiesViewModel.cs index 03f43fe8e5..3cc51d780e 100644 --- a/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/SmallGridPropertiesViewModel.cs +++ b/src/modules/cmdpal/Core/Microsoft.CmdPal.Core.ViewModels/SmallGridPropertiesViewModel.cs @@ -11,6 +11,10 @@ public class SmallGridPropertiesViewModel : IGridPropertiesViewModel { private readonly ExtensionObject _model; + public bool ShowTitle => false; + + public bool ShowSubtitle => false; + public SmallGridPropertiesViewModel(ISmallGridLayout smallGridLayout) { _model = new(smallGridLayout); diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Converters/GridItemContainerStyleSelector.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Converters/GridItemContainerStyleSelector.cs new file mode 100644 index 0000000000..5d45592ef1 --- /dev/null +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Converters/GridItemContainerStyleSelector.cs @@ -0,0 +1,31 @@ +// 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 Microsoft.CmdPal.Core.ViewModels; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; + +namespace Microsoft.CmdPal.UI; + +internal sealed partial class GridItemContainerStyleSelector : StyleSelector +{ + public IGridPropertiesViewModel? GridProperties { get; set; } + + public Style? Small { get; set; } + + public Style? Medium { get; set; } + + public Style? Gallery { get; set; } + + protected override Style? SelectStyleCore(object item, DependencyObject container) + { + return GridProperties switch + { + SmallGridPropertiesViewModel => Small, + MediumGridPropertiesViewModel => Medium, + GalleryGridPropertiesViewModel => Gallery, + _ => Medium, + }; + } +} diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Converters/GridItemTemplateSelector.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Converters/GridItemTemplateSelector.cs index df20e70b02..c93470e3e3 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Converters/GridItemTemplateSelector.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Converters/GridItemTemplateSelector.cs @@ -20,21 +20,12 @@ internal sealed partial class GridItemTemplateSelector : DataTemplateSelector protected override DataTemplate? SelectTemplateCore(object item, DependencyObject dependencyObject) { - DataTemplate? dataTemplate = Medium; - - if (GridProperties is SmallGridPropertiesViewModel) + return GridProperties switch { - dataTemplate = Small; - } - else if (GridProperties is MediumGridPropertiesViewModel) - { - dataTemplate = Medium; - } - else if (GridProperties is GalleryGridPropertiesViewModel) - { - dataTemplate = Gallery; - } - - return dataTemplate; + SmallGridPropertiesViewModel => Small, + MediumGridPropertiesViewModel => Medium, + GalleryGridPropertiesViewModel => Gallery, + _ => Medium, + }; } } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml index 676a676f95..7cf720198a 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml @@ -5,33 +5,151 @@ xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:cmdpalUI="using:Microsoft.CmdPal.UI" xmlns:controls="using:CommunityToolkit.WinUI.Controls" - xmlns:converters="using:CommunityToolkit.WinUI.Converters" xmlns:coreViewModels="using:Microsoft.CmdPal.Core.ViewModels" xmlns:cpcontrols="using:Microsoft.CmdPal.UI.Controls" xmlns:d="http://schemas.microsoft.com/expression/blend/2008" xmlns:help="using:Microsoft.CmdPal.UI.Helpers" - xmlns:local="using:Microsoft.CmdPal.UI" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" - xmlns:ui="using:CommunityToolkit.WinUI" - xmlns:viewModels="using:Microsoft.CmdPal.UI.ViewModels" x:Name="PageRoot" Background="Transparent" DataContext="{x:Bind ViewModel, Mode=OneWay}" mc:Ignorable="d"> - - - - - + + 6 + 4 + 4 + 8 + 8 + + + + + + + Visibility="{x:Bind ShowSubtitle, Mode=OneWay}" /> + ToolTipService.ToolTip="{x:Bind Title, Mode=OneWay}"> - - + Padding="8" + AutomationProperties.Name="{x:Bind Title, Mode=OneWay}" + CornerRadius="{StaticResource MediumGridViewItemCornerRadius}" + ToolTipService.ToolTip="{x:Bind Title, Mode=OneWay}"> + + + + - - + Visibility="{x:Bind LayoutShowsTitle, Mode=OneWay}" /> + @@ -193,11 +316,11 @@ Padding="0" HorizontalAlignment="Center" VerticalAlignment="Center" - AutomationProperties.Name="{x:Bind Title}" + AutomationProperties.Name="{x:Bind Title, Mode=OneWay}" BorderThickness="0" - CornerRadius="4" + CornerRadius="{StaticResource GalleryGridViewItemRadius}" Orientation="Vertical" - ToolTipService.ToolTip="{x:Bind Title}"> + ToolTipService.ToolTip="{x:Bind Title, Mode=OneWay}"> - - - + CornerRadius="{StaticResource GalleryGridViewItemRadius}"> @@ -222,35 +341,39 @@ - + + TextWrapping="NoWrap" + Visibility="{x:Bind ShowTitle, Mode=OneWay}" /> + TextWrapping="NoWrap" + Visibility="{x:Bind ShowSubtitle, Mode=OneWay}" /> @@ -295,6 +418,7 @@ IsDoubleTapEnabled="True" IsItemClickEnabled="True" ItemClick="Items_ItemClick" + ItemContainerStyleSelector="{StaticResource GridItemContainerStyleSelector}" ItemTemplateSelector="{StaticResource GridItemTemplateSelector}" ItemsSource="{x:Bind ViewModel.FilteredItems, Mode=OneWay}" RightTapped="Items_RightTapped" @@ -302,6 +426,7 @@ + diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/BindTransformers.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/BindTransformers.cs index 24d2ef47a6..012e8dc789 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/BindTransformers.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Helpers/BindTransformers.cs @@ -15,4 +15,7 @@ internal static class BindTransformers public static Visibility EmptyOrWhitespaceToCollapsed(string? input) => string.IsNullOrWhiteSpace(input) ? Visibility.Collapsed : Visibility.Visible; + + public static Visibility VisibleWhenAny(bool value1, bool value2) + => (value1 || value2) ? Visibility.Visible : Visibility.Collapsed; } diff --git a/src/modules/cmdpal/ext/SamplePagesExtension/Pages/SampleGalleryListPage.cs b/src/modules/cmdpal/ext/SamplePagesExtension/Pages/SampleGalleryListPage.cs index 3a80d180e0..2f6fba7089 100644 --- a/src/modules/cmdpal/ext/SamplePagesExtension/Pages/SampleGalleryListPage.cs +++ b/src/modules/cmdpal/ext/SamplePagesExtension/Pages/SampleGalleryListPage.cs @@ -9,13 +9,6 @@ namespace SamplePagesExtension; internal sealed partial class SampleGalleryListPage : ListPage { - public SampleGalleryListPage() - { - Icon = new IconInfo("\uE7C5"); - Name = "Sample Gallery List Page"; - GridProperties = new GalleryGridLayout(); - } - public override IListItem[] GetItems() { return [ diff --git a/src/modules/cmdpal/ext/SamplePagesExtension/Pages/SampleGridsListPage.cs b/src/modules/cmdpal/ext/SamplePagesExtension/Pages/SampleGridsListPage.cs new file mode 100644 index 0000000000..05b604c912 --- /dev/null +++ b/src/modules/cmdpal/ext/SamplePagesExtension/Pages/SampleGridsListPage.cs @@ -0,0 +1,59 @@ +// 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 Microsoft.CommandPalette.Extensions; +using Microsoft.CommandPalette.Extensions.Toolkit; + +namespace SamplePagesExtension; + +internal sealed partial class SampleGridsListPage : ListPage +{ + private readonly IListItem[] _items = + [ + new ListItem(new SampleGalleryListPage { GridProperties = new GalleryGridLayout { ShowTitle = true, ShowSubtitle = true } }) + { + Title = "Gallery list page (title and subtitle)", + Subtitle = "A sample gallery list page with images", + Icon = IconHelpers.FromRelativePath("Assets/Images/Swirls.png"), + }, + new ListItem(new SampleGalleryListPage { GridProperties = new GalleryGridLayout { ShowTitle = true, ShowSubtitle = false } }) + { + Title = "Gallery list page (title, no subtitle)", + Subtitle = "A sample gallery list page with images", + Icon = IconHelpers.FromRelativePath("Assets/Images/Swirls.png"), + }, + new ListItem(new SampleGalleryListPage { GridProperties = new GalleryGridLayout { ShowTitle = false, ShowSubtitle = false } }) + { + Title = "Gallery list page (no title, no subtitle)", + Subtitle = "A sample gallery list page with images", + Icon = IconHelpers.FromRelativePath("Assets/Images/Swirls.png"), + }, + new ListItem(new SampleGalleryListPage { GridProperties = new SmallGridLayout() }) + { + Title = "Small grid list page", + Subtitle = "A sample grid list page with text items", + Icon = IconHelpers.FromRelativePath("Assets/Images/Win-Digital.png"), + }, + new ListItem(new SampleGalleryListPage { GridProperties = new MediumGridLayout { ShowTitle = true } }) + { + Title = "Medium grid (with title)", + Subtitle = "A sample grid list page with text items", + Icon = IconHelpers.FromRelativePath("Assets/Images/Win-Digital.png"), + }, + new ListItem(new SampleGalleryListPage { GridProperties = new MediumGridLayout { ShowTitle = false } }) + { + Title = "Medium grid (hidden title)", + Subtitle = "A sample grid list page with text items", + Icon = IconHelpers.FromRelativePath("Assets/Images/Win-Digital.png"), + } + ]; + + public SampleGridsListPage() + { + Icon = new IconInfo("\uE7C5"); + Name = "Grid and gallery lists"; + } + + public override IListItem[] GetItems() => _items; +} diff --git a/src/modules/cmdpal/ext/SamplePagesExtension/SamplesListPage.cs b/src/modules/cmdpal/ext/SamplePagesExtension/SamplesListPage.cs index 254dbf3eb9..73ef1815d4 100644 --- a/src/modules/cmdpal/ext/SamplePagesExtension/SamplesListPage.cs +++ b/src/modules/cmdpal/ext/SamplePagesExtension/SamplesListPage.cs @@ -34,9 +34,9 @@ public partial class SamplesListPage : ListPage Title = "Dynamic List Page Command", Subtitle = "Changes the list of items in response to the typed query", }, - new ListItem(new SampleGalleryListPage()) + new ListItem(new SampleGridsListPage()) { - Title = "Gallery List Page Command", + Title = "Grid views and galleries", Subtitle = "Displays items as a gallery", }, new ListItem(new OnLoadPage())