Code Quality: Fixed keyboard shortcuts when refocusing window (#17502)

yaira2 · web-flow · commit a1b843b09485 · 2025-08-28T20:58:28.000-04:00
diff --git a/src/Files.App/Actions/Navigation/NextTabAction.cs b/src/Files.App/Actions/Navigation/NextTabAction.cs
@@ -9,6 +9,7 @@ namespace Files.App.Actions
 	internal sealed partial class NextTabAction : ObservableObject, IAction
 	{
 		private readonly IMultitaskingContext multitaskingContext;
+		private readonly IContentPageContext contentPageContext = Ioc.Default.GetRequiredService<IContentPageContext>();
 
 		public string Label
 			=> Strings.NextTab.GetLocalizedResource();
@@ -37,7 +38,7 @@ public async Task ExecuteAsync(object? parameter = null)
 			await Task.Delay(500);
 
 			// Focus the content of the selected tab item (needed for keyboard navigation)
-			(multitaskingContext.CurrentTabItem.TabItemContent as Control)?.Focus(FocusState.Programmatic);
+			contentPageContext.ShellPage!.PaneHolder.FocusActivePane();
 		}
 
 		private void MultitaskingContext_PropertyChanged(object? sender, PropertyChangedEventArgs e)
diff --git a/src/Files.App/Actions/Navigation/PreviousTabAction.cs b/src/Files.App/Actions/Navigation/PreviousTabAction.cs
@@ -9,6 +9,7 @@ namespace Files.App.Actions
 	internal sealed partial class PreviousTabAction : ObservableObject, IAction
 	{
 		private readonly IMultitaskingContext multitaskingContext;
+		private readonly IContentPageContext contentPageContext = Ioc.Default.GetRequiredService<IContentPageContext>();
 
 		public string Label
 			=> Strings.PreviousTab.GetLocalizedResource();
@@ -40,7 +41,7 @@ public async Task ExecuteAsync(object? parameter = null)
 			await Task.Delay(500);
 
 			// Focus the content of the selected tab item (needed for keyboard navigation)
-			(multitaskingContext.CurrentTabItem.TabItemContent as Control)?.Focus(FocusState.Programmatic);
+			contentPageContext.ShellPage!.PaneHolder.FocusActivePane();
 		}
 
 		private void MultitaskingContext_PropertyChanged(object? sender, PropertyChangedEventArgs e)
diff --git a/src/Files.App/Data/Contracts/IShellPanesPage.cs b/src/Files.App/Data/Contracts/IShellPanesPage.cs
@@ -69,6 +69,11 @@ public interface IShellPanesPage : IDisposable, INotifyPropertyChanged
 		/// </summary>
 		public void FocusActivePane();
 
+		/// <summary>
+		/// Locks the active pane.
+		/// </summary>
+		public void LockActivePane();
+
 		/// <summary>
 		/// Gets open panes.
 		/// </summary>
diff --git a/src/Files.App/UserControls/TabBar/TabBar.xaml b/src/Files.App/UserControls/TabBar/TabBar.xaml
@@ -145,7 +145,7 @@
 								x:Name="SplitPaneMenuItem"
 								x:Load="{x:Bind Commands.SplitPaneHorizontally.IsExecutable, Mode=OneWay}"
 								Text="{helpers:ResourceString Name=SplitPane}">
-								<MenuFlyoutSubItem.Items>									
+								<MenuFlyoutSubItem.Items>
 									<!--  Vertical  -->
 									<uc:MenuFlyoutItemWithThemedIcon
 										x:Name="AddVerticalPaneTabActionButton"
diff --git a/src/Files.App/UserControls/Toolbar.xaml b/src/Files.App/UserControls/Toolbar.xaml
@@ -687,7 +687,10 @@
 				<controls:ThemedIcon Style="{x:Bind ViewModel.LayoutThemedIcon, Mode=OneWay}" />
 
 				<AppBarButton.Flyout>
-					<Flyout contract8Present:ShouldConstrainToRootBounds="False" Placement="Bottom">
+					<Flyout
+						x:Name="LayoutFlyout"
+						contract8Present:ShouldConstrainToRootBounds="False"
+						Placement="Bottom">
 						<StackPanel Spacing="12">
 
 							<!--  Header  -->
@@ -699,6 +702,7 @@
 								<!--  Details  -->
 								<RadioButton
 									AutomationProperties.Name="{x:Bind Commands.LayoutDetails.AutomationName}"
+									Click="LayoutButton_Click"
 									Command="{x:Bind Commands.LayoutDetails}"
 									GroupName="LayoutRadio"
 									IsChecked="{x:Bind ViewModel.IsDetailsLayout, Mode=OneWay}"
@@ -722,6 +726,7 @@
 								<!--  List  -->
 								<RadioButton
 									AutomationProperties.Name="{x:Bind Commands.LayoutList.AutomationName}"
+									Click="LayoutButton_Click"
 									Command="{x:Bind Commands.LayoutList}"
 									GroupName="LayoutRadio"
 									IsChecked="{x:Bind ViewModel.IsListLayout, Mode=OneWay}"
@@ -746,6 +751,7 @@
 								<!--  Cards  -->
 								<RadioButton
 									AutomationProperties.Name="{x:Bind Commands.LayoutCards.AutomationName}"
+									Click="LayoutButton_Click"
 									Command="{x:Bind Commands.LayoutCards}"
 									GroupName="LayoutRadio"
 									IsChecked="{x:Bind ViewModel.IsCardsLayout, Mode=OneWay}"
@@ -770,6 +776,7 @@
 								<!--  Grid  -->
 								<RadioButton
 									AutomationProperties.Name="{x:Bind Commands.LayoutGrid.AutomationName}"
+									Click="LayoutButton_Click"
 									Command="{x:Bind Commands.LayoutGrid}"
 									GroupName="LayoutRadio"
 									IsChecked="{x:Bind ViewModel.IsGridLayout, Mode=OneWay}"
@@ -794,6 +801,7 @@
 								<!--  Columns  -->
 								<RadioButton
 									AutomationProperties.Name="{x:Bind Commands.LayoutColumns.AutomationName}"
+									Click="LayoutButton_Click"
 									Command="{x:Bind Commands.LayoutColumns}"
 									GroupName="LayoutRadio"
 									IsChecked="{x:Bind ViewModel.IsColumnLayout, Mode=OneWay}"
diff --git a/src/Files.App/UserControls/Toolbar.xaml.cs b/src/Files.App/UserControls/Toolbar.xaml.cs
@@ -108,5 +108,11 @@ private void AppBarButton_AccessKeyInvoked(UIElement sender, AccessKeyInvokedEve
 			if (VisualTreeHelper.GetOpenPopupsForXamlRoot(MainWindow.Instance.Content.XamlRoot).Any())
 				args.Handled = true;
 		}
+
+		private void LayoutButton_Click(object sender, RoutedEventArgs e)
+		{
+			// Hide flyout after choosing a layout
+			LayoutFlyout.Hide();
+		}
 	}
 }
diff --git a/src/Files.App/ViewModels/MainPageViewModel.cs b/src/Files.App/ViewModels/MainPageViewModel.cs
@@ -391,7 +391,7 @@ private async void ExecuteNavigateToNumberedTabKeyboardAcceleratorCommand(Keyboa
 				await Task.Delay(500);
 
 				// Focus the content of the selected tab item (needed for keyboard navigation)
-				(SelectedTabItem?.TabItemContent as Control)?.Focus(FocusState.Programmatic);
+				context.ShellPage!.PaneHolder.FocusActivePane();
 			}
 
 			e.Handled = true;
diff --git a/src/Files.App/ViewModels/ShellViewModel.cs b/src/Files.App/ViewModels/ShellViewModel.cs
@@ -584,6 +584,13 @@ public ShellViewModel(LayoutPreferencesManager folderSettingsViewModel)
 
 		private async void LayoutModeChangeRequested(object? sender, LayoutModeEventArgs e)
 		{
+			// Layout changes can cause the active pane to lose focus. To prevent this,
+			// the pane is locked here and focus is restored when file loading completes
+			// in the RefreshItem() method in BaseLayoutPage.cs.
+			// See https://github.com/files-community/Files/issues/15397
+			// See https://github.com/files-community/Files/issues/16530
+			ContentPageContext.ShellPage!.PaneHolder.LockActivePane();
+
 			await dispatcherQueue.EnqueueOrInvokeAsync(CheckForBackgroundImage, Microsoft.UI.Dispatching.DispatcherQueuePriority.Low);
 		}
 
diff --git a/src/Files.App/Views/Layouts/BaseLayoutPage.cs b/src/Files.App/Views/Layouts/BaseLayoutPage.cs
@@ -1240,8 +1240,14 @@ private void RefreshItem(SelectorItem container, object item, bool inRecycleQueu
 						if (ParentShellPageInstance.ShellViewModel.EnabledGitProperties is not GitProperties.None && listedItem is IGitItem gitItem)
 							await ParentShellPageInstance.ShellViewModel.LoadGitPropertiesAsync(gitItem);
 
-						// Focus file list when items finish loading (#16530)
-						ItemManipulationModel.FocusFileList();
+						// Layout changes can cause the active pane to lose focus. To prevent this,
+						// the pane is locked in LayoutModeChangeRequested() and focus is restored here
+						// when file loading completes.
+						// See https://github.com/files-community/Files/issues/15397
+						// See https://github.com/files-community/Files/issues/16530
+
+						if (ParentShellPageInstance.IsCurrentPane && !ParentShellPageInstance.IsColumnView)
+							ItemManipulationModel.FocusFileList();
 					});
 				}
 			}
diff --git a/src/Files.App/Views/MainPage.xaml b/src/Files.App/Views/MainPage.xaml
@@ -22,6 +22,7 @@
 	KeyboardAcceleratorPlacementMode="Hidden"
 	Loaded="Page_Loaded"
 	NavigationCacheMode="Required"
+	PointerReleased="Page_PointerReleased"
 	SizeChanged="Page_SizeChanged"
 	mc:Ignorable="d">
 
diff --git a/src/Files.App/Views/MainPage.xaml.cs b/src/Files.App/Views/MainPage.xaml.cs
@@ -162,7 +162,6 @@ public async void MultitaskingControl_CurrentInstanceChanged(object? sender, Cur
 			// Focus the content of the selected tab item (this also avoids an issue where the Omnibar sometimes steals the focus)
 			await Task.Delay(100);
 			ContentPageContext.ShellPage!.PaneHolder.FocusActivePane();
-
 		}
 
 		private void PaneHolder_PropertyChanged(object? sender, PropertyChangedEventArgs e)
@@ -477,5 +476,12 @@ private void SettingsButton_AccessKeyInvoked(UIElement sender, AccessKeyInvokedE
 			if (VisualTreeHelper.GetOpenPopupsForXamlRoot(MainWindow.Instance.Content.XamlRoot).Any())
 				args.Handled = true;
 		}
+
+		private void Page_PointerReleased(object sender, PointerRoutedEventArgs e)
+		{
+			// Workaround for issue where clicking an empty area in the window (toolbar, title bar etc) prevents keyboard
+			// shortcuts from working properly, see https://github.com/microsoft/microsoft-ui-xaml/issues/6467
+			DispatcherQueue.TryEnqueue(() => ContentPageContext.ShellPage?.PaneHolder.FocusActivePane());
+		}
 	}
 }
diff --git a/src/Files.App/Views/ShellPanesPage.xaml.cs b/src/Files.App/Views/ShellPanesPage.xaml.cs
@@ -20,6 +20,7 @@ public sealed partial class ShellPanesPage : Page, IShellPanesPage, ITabBarItemC
 		// Dependency injections
 
 		private IGeneralSettingsService GeneralSettingsService { get; } = Ioc.Default.GetRequiredService<IGeneralSettingsService>();
+		private IContentPageContext ContentPageContext { get; } = Ioc.Default.GetRequiredService<IContentPageContext>();
 		private AppModel AppModel { get; } = Ioc.Default.GetRequiredService<AppModel>();
 
 		// Constants
@@ -228,6 +229,17 @@ public bool IsCurrentInstance
 			}
 		}
 
+		private bool _IsActivePaneLocked;
+		public bool IsActivePaneLocked
+		{
+			get => _IsActivePaneLocked;
+			set
+			{
+				if (_IsActivePaneLocked != value)
+					_IsActivePaneLocked = value;
+			}
+		}
+
 		// Events
 
 		public static event EventHandler<ShellPanesPage>? CurrentInstanceChanged;
@@ -376,6 +388,16 @@ public void FocusActivePane()
 				GetPane(0)?.Focus(FocusState.Programmatic);
 			else
 				GetPane(1)?.Focus(FocusState.Programmatic);
+
+			// Focus file list
+			if (ActivePane is BaseShellPage baseShellPage)
+				baseShellPage.ContentPage?.ItemManipulationModel.FocusFileList();
+		}
+
+		/// <inheritdoc/>
+		public void LockActivePane()
+		{
+			IsActivePaneLocked = true;
 		}
 
 		/// <inheritdoc/>
@@ -656,9 +678,16 @@ private void Pane_Loaded(object sender, RoutedEventArgs e)
 
 		private void Pane_GettingFocus(UIElement sender, GettingFocusEventArgs args)
 		{
-			// Workaround for https://github.com/files-community/Files/issues/15397
-			if (args?.NewFocusedElement is not null && args.NewFocusedElement is not (ListViewItem or GridViewItem or ListView or GridView or TextBox))
+			// Cancel focus attempts while the active pane is locked during layout changes.
+			// Pane locking occurs in LayoutModeChangeRequested() in ShellViewModel.cs.
+			// Focus is restored in RefreshItem() in BaseLayoutPage.cs when file loading completes.
+			// See https://github.com/files-community/Files/issues/15397
+			// See https://github.com/files-community/Files/issues/16530
+			if (IsActivePaneLocked)
+			{
+				IsActivePaneLocked = false;
 				args.TryCancel();
+			}
 		}
 
 		private void Pane_ContentChanged(object? sender, TabBarItemParameter e)

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@ namespace Files.App.Actions`
`9`	`9`	`internal sealed partial class NextTabAction : ObservableObject, IAction`
`10`	`10`	`{`
`11`	`11`	`private readonly IMultitaskingContext multitaskingContext;`
	`12`	`+ private readonly IContentPageContext contentPageContext = Ioc.Default.GetRequiredService<IContentPageContext>();`
`12`	`13`
`13`	`14`	`public string Label`
`14`	`15`	`=> Strings.NextTab.GetLocalizedResource();`
`@@ -37,7 +38,7 @@ public async Task ExecuteAsync(object? parameter = null)`
`37`	`38`	`await Task.Delay(500);`
`38`	`39`
`39`	`40`	`// Focus the content of the selected tab item (needed for keyboard navigation)`
`40`		`- (multitaskingContext.CurrentTabItem.TabItemContent as Control)?.Focus(FocusState.Programmatic);`
	`41`	`+ contentPageContext.ShellPage!.PaneHolder.FocusActivePane();`
`41`	`42`	`}`
`42`	`43`
`43`	`44`	`private void MultitaskingContext_PropertyChanged(object? sender, PropertyChangedEventArgs e)`
Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@ namespace Files.App.Actions`
`9`	`9`	`internal sealed partial class PreviousTabAction : ObservableObject, IAction`
`10`	`10`	`{`
`11`	`11`	`private readonly IMultitaskingContext multitaskingContext;`
	`12`	`+ private readonly IContentPageContext contentPageContext = Ioc.Default.GetRequiredService<IContentPageContext>();`
`12`	`13`
`13`	`14`	`public string Label`
`14`	`15`	`=> Strings.PreviousTab.GetLocalizedResource();`
`@@ -40,7 +41,7 @@ public async Task ExecuteAsync(object? parameter = null)`
`40`	`41`	`await Task.Delay(500);`
`41`	`42`
`42`	`43`	`// Focus the content of the selected tab item (needed for keyboard navigation)`
`43`		`- (multitaskingContext.CurrentTabItem.TabItemContent as Control)?.Focus(FocusState.Programmatic);`
	`44`	`+ contentPageContext.ShellPage!.PaneHolder.FocusActivePane();`
`44`	`45`	`}`
`45`	`46`
`46`	`47`	`private void MultitaskingContext_PropertyChanged(object? sender, PropertyChangedEventArgs e)`
Original file line number	Diff line number	Diff line change
`@@ -108,5 +108,11 @@ private void AppBarButton_AccessKeyInvoked(UIElement sender, AccessKeyInvokedEve`
`108`	`108`	`if (VisualTreeHelper.GetOpenPopupsForXamlRoot(MainWindow.Instance.Content.XamlRoot).Any())`
`109`	`109`	`args.Handled = true;`
`110`	`110`	`}`
	`111`	`+`
	`112`	`+ private void LayoutButton_Click(object sender, RoutedEventArgs e)`
	`113`	`+ {`
	`114`	`+ // Hide flyout after choosing a layout`
	`115`	`+ LayoutFlyout.Hide();`
	`116`	`+ }`
`111`	`117`	`}`
`112`	`118`	`}`
Original file line number	Diff line number	Diff line change
`@@ -391,7 +391,7 @@ private async void ExecuteNavigateToNumberedTabKeyboardAcceleratorCommand(Keyboa`
`391`	`391`	`await Task.Delay(500);`
`392`	`392`
`393`	`393`	`// Focus the content of the selected tab item (needed for keyboard navigation)`
`394`		`- (SelectedTabItem?.TabItemContent as Control)?.Focus(FocusState.Programmatic);`
	`394`	`+ context.ShellPage!.PaneHolder.FocusActivePane();`
`395`	`395`	`}`
`396`	`396`
`397`	`397`	`e.Handled = true;`
Original file line number	Diff line number	Diff line change
`@@ -584,6 +584,13 @@ public ShellViewModel(LayoutPreferencesManager folderSettingsViewModel)`
`584`	`584`
`585`	`585`	`private async void LayoutModeChangeRequested(object? sender, LayoutModeEventArgs e)`
`586`	`586`	`{`
	`587`	`+ // Layout changes can cause the active pane to lose focus. To prevent this,`
	`588`	`+ // the pane is locked here and focus is restored when file loading completes`
	`589`	`+ // in the RefreshItem() method in BaseLayoutPage.cs.`
	`590`	`+ // See https://github.com/files-community/Files/issues/15397`
	`591`	`+ // See https://github.com/files-community/Files/issues/16530`
	`592`	`+ ContentPageContext.ShellPage!.PaneHolder.LockActivePane();`
	`593`	`+`
`587`	`594`	`await dispatcherQueue.EnqueueOrInvokeAsync(CheckForBackgroundImage, Microsoft.UI.Dispatching.DispatcherQueuePriority.Low);`
`588`	`595`	`}`
`589`	`596`