From feae285c400b9d4d8834959abf73afad60fc6b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Tue, 31 Mar 2026 04:28:36 +0200 Subject: [PATCH] CmdPal: Revert focus restoration on Extensions settings page (#46642) ## Summary of the Pull Request This PR reverts focus restoration to the previously selected item in the list on the Extensions page in the Settings window, as it unintentionally caused the wrong item to open on click. This reverts commit cb9d54317ab15b3bfc45d6cae11ec24ce42e47f6. ## PR Checklist - [x] Closes: #46641 - [ ] **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 --- .../Settings/ExtensionsPage.xaml | 2 - .../Settings/ExtensionsPage.xaml.cs | 154 ------------------ 2 files changed, 156 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml index b471b67d5d..3ac3519cb4 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml @@ -216,7 +216,6 @@ @@ -225,7 +224,6 @@ Click="SettingsCard_Click" DataContext="{x:Bind}" Description="{x:Bind ExtensionSubtext, Mode=OneWay}" - GotFocus="SettingsCard_GotFocus" Header="{x:Bind DisplayName, Mode=OneWay}" IsClickEnabled="True"> diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml.cs index bfa73c8a20..952b9ffee5 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI/Settings/ExtensionsPage.xaml.cs @@ -19,7 +19,6 @@ public sealed partial class ExtensionsPage : Page private readonly TaskScheduler _mainTaskScheduler = TaskScheduler.FromCurrentSynchronizationContext(); private readonly SettingsViewModel? viewModel; - private int _lastFocusedIndex; public ExtensionsPage() { @@ -59,157 +58,4 @@ public sealed partial class ExtensionsPage : Page Logger.LogError("Error when showing FallbackRankerDialog", ex); } } - - private void SettingsCard_GotFocus(object sender, RoutedEventArgs e) - { - // Track focus whenever any part of the card gets focus (including children like ToggleSwitch) - if (sender is SettingsCard card && viewModel is not null) - { - var dataContext = card.DataContext as ProviderSettingsViewModel; - if (dataContext is not null) - { - var filteredProviders = viewModel.Extensions.FilteredProviders; - var index = filteredProviders.IndexOf(dataContext); - if (index >= 0) - { - _lastFocusedIndex = index; - } - } - } - } - - private void ProvidersRepeater_GettingFocus(UIElement sender, GettingFocusEventArgs args) - { - if (viewModel is null || ProvidersRepeater is null) - { - return; - } - - // Only intervene when focus is coming into the ItemsRepeater from outside - if (args.OldFocusedElement != null && IsElementInsideRepeater(args.OldFocusedElement)) - { - return; - } - - var filteredProviders = viewModel.Extensions.FilteredProviders; - - if (filteredProviders.Count == 0) - { - return; - } - - // Get the last focused index, defaulting to 0 - var index = _lastFocusedIndex; - if (index < 0 || index >= filteredProviders.Count) - { - index = 0; - } - - // Check if WinUI is trying to focus something other than our target - var shouldIntervene = false; - - // If direction is Previous (Shift+Tab), we need to intervene - if (args.Direction == FocusNavigationDirection.Previous || args.Direction == FocusNavigationDirection.Up) - { - shouldIntervene = true; - } - - // Also intervene if the NewFocusedElement is not at our target index - else if (args.NewFocusedElement is DependencyObject newFocus) - { - // Check if the new focus element is inside our target card - var targetCard = ProvidersRepeater.TryGetElement(index) as SettingsCard; - if (targetCard != null && !IsElementInsideCard(newFocus, targetCard)) - { - shouldIntervene = true; - } - } - - if (shouldIntervene) - { - // Ensure the target element is realized before trying to focus it - ProvidersRepeater.GetOrCreateElement(index); - - // Get the target card - var targetCard = ProvidersRepeater.TryGetElement(index) as SettingsCard; - - if (targetCard != null) - { - // For shift-tab or wrong target, cancel and manually set focus - args.TryCancel(); - args.Handled = true; - - // Set focus asynchronously to the target card and scroll it into view - _ = targetCard.DispatcherQueue.TryEnqueue(Microsoft.UI.Dispatching.DispatcherQueuePriority.Normal, () => - { - targetCard.Focus(FocusState.Keyboard); - BringCardIntoView(targetCard); - }); - } - } - else - { - // For normal Tab forward, just redirect - ProvidersRepeater.GetOrCreateElement(index); - var targetCard = ProvidersRepeater.TryGetElement(index) as SettingsCard; - - if (targetCard != null) - { - args.TrySetNewFocusedElement(targetCard); - args.Handled = true; - - // Set focus asynchronously to the target card and scroll it into view - _ = targetCard.DispatcherQueue.TryEnqueue(Microsoft.UI.Dispatching.DispatcherQueuePriority.Low, () => - { - BringCardIntoView(targetCard); - }); - } - } - } - - private void BringCardIntoView(SettingsCard card) - { - card.StartBringIntoView(new BringIntoViewOptions - { - AnimationDesired = true, - VerticalAlignmentRatio = 0.5, // Center vertically - }); - } - - private bool IsElementInsideCard(DependencyObject element, SettingsCard card) - { - var parent = element; - while (parent != null) - { - if (ReferenceEquals(parent, card)) - { - return true; - } - - parent = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetParent(parent); - } - - return false; - } - - private bool IsElementInsideRepeater(object element) - { - if (element is not DependencyObject depObj) - { - return false; - } - - var parent = depObj; - while (parent != null) - { - if (ReferenceEquals(parent, ProvidersRepeater)) - { - return true; - } - - parent = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetParent(parent); - } - - return false; - } }