From e2f611a7fc4f12023147c2ffffe20b46e145353b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Tue, 24 Mar 2026 20:15:44 +0100 Subject: [PATCH] CmdPal: Prevent PgUp/PgDown from selecting non-internactive items (#46439) ## Summary of the Pull Request This PR prevents paging (PgUp/PgDown) in the item list from selecting non-interactive items (such as separators or section headers). It adds `FindSelectableIndex` and `FindSelectableIndexForPageNavigation` helper methods, which locate the next interactive item in the given direction. These methods are used to guard paging navigation and to consolidate related logic elsewhere. ## PR Checklist - [x] Closes: #46283 - [ ] **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 --- .../ExtViews/ListPage.xaml.cs | 152 ++++++++---------- 1 file changed, 66 insertions(+), 86 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml.cs index a3c2dcab11..9622782d68 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/ExtViews/ListPage.xaml.cs @@ -156,21 +156,7 @@ public sealed partial class ListPage : Page, /// private int GetFirstSelectableIndex() { - var items = ItemView.Items; - if (items is null || items.Count == 0) - { - return -1; - } - - for (var i = 0; i < items.Count; i++) - { - if (!IsSeparator(items[i])) - { - return i; - } - } - - return -1; + return FindSelectableIndex(0, 1); } [System.Diagnostics.CodeAnalysis.SuppressMessage("CodeQuality", "IDE0051:Remove unused private members", Justification = "VS is too aggressive at pruning methods bound in XAML")] @@ -604,9 +590,44 @@ public sealed partial class ListPage : Page, ? Math.Min(itemCount - 1, currentIndex + Math.Max(1, itemsPerPage)) : Math.Max(0, currentIndex - Math.Max(1, itemsPerPage)); + targetIndex = FindSelectableIndexForPageNavigation(targetIndex, isPageDown); + if (targetIndex == -1) + { + return null; + } + return (currentIndex, targetIndex); } + private int FindSelectableIndex(int startIndex, int step) + { + var items = ItemView.Items; + var count = items.Count; + if (count == 0 || step == 0) + { + return -1; + } + + startIndex = Math.Clamp(startIndex, 0, count - 1); + + for (var i = startIndex; i >= 0 && i < count; i += step) + { + if (!IsSeparator(items[i])) + { + return i; + } + } + + return -1; + } + + private int FindSelectableIndexForPageNavigation(int targetIndex, bool isPageDown) + { + var step = isPageDown ? 1 : -1; + var index = FindSelectableIndex(targetIndex, step); + return index != -1 ? index : FindSelectableIndex(targetIndex - step, -step); + } + private static void OnViewModelChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) { if (d is ListPage @this) @@ -1045,39 +1066,24 @@ public sealed partial class ListPage : Page, /// private void NavigateUp() { - var newIndex = ItemView.SelectedIndex; - - if (ItemView.SelectedIndex > 0) + if (ItemView.Items.Count == 0) { - newIndex--; - - while ( - newIndex >= 0 && - IsSeparator(ItemView.Items[newIndex]) && - newIndex != ItemView.SelectedIndex) - { - newIndex--; - } - - if (newIndex < 0) - { - newIndex = ItemView.Items.Count - 1; - - while ( - newIndex >= 0 && - IsSeparator(ItemView.Items[newIndex]) && - newIndex != ItemView.SelectedIndex) - { - newIndex--; - } - } - } - else - { - newIndex = ItemView.Items.Count - 1; + return; } - ItemView.SelectedIndex = newIndex; + var newIndex = ItemView.SelectedIndex > 0 + ? FindSelectableIndex(ItemView.SelectedIndex - 1, -1) + : -1; + + if (newIndex == -1) + { + newIndex = FindSelectableIndex(ItemView.Items.Count - 1, -1); + } + + if (newIndex != -1) + { + ItemView.SelectedIndex = newIndex; + } } private void Items_DragItemsStarting(object sender, DragItemsStartingEventArgs e) @@ -1168,50 +1174,24 @@ public sealed partial class ListPage : Page, /// private void NavigateDown() { - var newIndex = ItemView.SelectedIndex; - - if (ItemView.SelectedIndex == ItemView.Items.Count - 1) + if (ItemView.Items.Count == 0) { - newIndex = 0; - while ( - newIndex < ItemView.Items.Count && - IsSeparator(ItemView.Items[newIndex])) - { - newIndex++; - } - - if (newIndex >= ItemView.Items.Count) - { - return; - } - } - else - { - newIndex++; - - while ( - newIndex < ItemView.Items.Count && - IsSeparator(ItemView.Items[newIndex]) && - newIndex != ItemView.SelectedIndex) - { - newIndex++; - } - - if (newIndex >= ItemView.Items.Count) - { - newIndex = 0; - - while ( - newIndex < ItemView.Items.Count && - IsSeparator(ItemView.Items[newIndex]) && - newIndex != ItemView.SelectedIndex) - { - newIndex++; - } - } + return; } - ItemView.SelectedIndex = newIndex; + var newIndex = ItemView.SelectedIndex < ItemView.Items.Count - 1 + ? FindSelectableIndex(ItemView.SelectedIndex + 1, 1) + : -1; + + if (newIndex == -1) + { + newIndex = FindSelectableIndex(0, 1); + } + + if (newIndex != -1) + { + ItemView.SelectedIndex = newIndex; + } } private bool IsSeparator(object? item) => item is ListItemViewModel li && !li.IsInteractive;