[Windows] Handle focus events using FocusManager (take 2) (#24695)

* WIPWIP * Feedback * Refactor usage of `FocusManager` to avoid weak table usage * Use ConditionalWeakTable --------- Co-authored-by: Alberto Aldegheri <[email protected]>
dotnet · Sep 30, 2024 · 0b84dad · 0b84dad
1 parent 9f502da
commit 0b84dad
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 28 deletions.
diff --git a/src/Core/src/Handlers/View/ViewHandler.Windows.cs b/src/Core/src/Handlers/View/ViewHandler.Windows.cs
@@ -1,31 +1,35 @@
 #nullable enable
 using System;
-using System.Collections.Generic;
-using System.ComponentModel;
+using System.Runtime.CompilerServices;
 using Microsoft.UI.Xaml;
-using Microsoft.UI.Xaml.Controls;
 using Microsoft.UI.Xaml.Controls.Primitives;
+using Microsoft.UI.Xaml.Input;
 using PlatformView = Microsoft.UI.Xaml.FrameworkElement;
 
 namespace Microsoft.Maui.Handlers
 {
 	public partial class ViewHandler
 	{
+		readonly static ConditionalWeakTable<PlatformView, ViewHandler> FocusManagerMapping = new();
+
+		static ViewHandler()
+		{
+			FocusManager.GotFocus += FocusManager_GotFocus;
+			FocusManager.LostFocus += FocusManager_LostFocus;
+		}
+
 		partial void ConnectingHandler(PlatformView? platformView)
 		{
-			if (platformView != null)
+			if (platformView is not null)
 			{
-				platformView.GotFocus += OnPlatformViewGotFocus;
-				platformView.LostFocus += OnPlatformViewLostFocus;
+				FocusManagerMapping.Add(platformView, this);
 			}
 		}
 
 		partial void DisconnectingHandler(PlatformView platformView)
 		{
+			FocusManagerMapping.Remove(platformView);
 			UpdateIsFocused(false);
-
-			platformView.GotFocus -= OnPlatformViewGotFocus;
-			platformView.LostFocus -= OnPlatformViewLostFocus;
 		}
 
 		static partial void MappingFrame(IViewHandler handler, IView view)
@@ -88,7 +92,9 @@ public static void MapAnchorY(IViewHandler handler, IView view)
 		public static void MapToolbar(IViewHandler handler, IView view)
 		{
 			if (view is IToolbarElement tb)
+			{
 				MapToolbar(handler, tb);
+			}
 		}
 
 		internal static void MapToolbar(IElementHandler handler, IToolbarElement toolbarElement)
@@ -133,25 +139,35 @@ internal static void MapContextFlyout(IElementHandler handler, IContextFlyoutEle
 			}
 		}
 
-		void OnPlatformViewGotFocus(object sender, RoutedEventArgs args)
+		static void FocusManager_GotFocus(object? sender, FocusManagerGotFocusEventArgs e)
 		{
-			UpdateIsFocused(true);
+			if (e.NewFocusedElement is PlatformView platformView && FocusManagerMapping.TryGetValue(platformView, out ViewHandler? handler))
+			{
+				handler.UpdateIsFocused(true);
+			}
 		}
 
-		void OnPlatformViewLostFocus(object sender, RoutedEventArgs args)
+		static void FocusManager_LostFocus(object? sender, FocusManagerLostFocusEventArgs e)
 		{
-			UpdateIsFocused(false);
+			if (e.OldFocusedElement is PlatformView platformView && FocusManagerMapping.TryGetValue(platformView, out ViewHandler? handler))
+			{
+				handler.UpdateIsFocused(false);
+			}
 		}
 
 		void UpdateIsFocused(bool isFocused)
 		{
-			if (VirtualView == null)
+			if (VirtualView is not { } virtualView)
+			{
 				return;
+			}
 
-			bool updateIsFocused = (isFocused && !VirtualView.IsFocused) || (!isFocused && VirtualView.IsFocused);
+			bool updateIsFocused = (isFocused && !virtualView.IsFocused) || (!isFocused && virtualView.IsFocused);
 
 			if (updateIsFocused)
-				VirtualView.IsFocused = isFocused;
+			{
+				virtualView.IsFocused = isFocused;
+			}
 		}
 	}
 }
diff --git a/src/Core/src/Platform/Windows/ViewExtensions.cs b/src/Core/src/Platform/Windows/ViewExtensions.cs
@@ -36,7 +36,9 @@ public static void Focus(this FrameworkElement platformView, FocusRequest reques
 		public static void Unfocus(this FrameworkElement platformView, IView view)
 		{
 			if (platformView is Control control)
+			{
 				UnfocusControl(control);
+			}
 		}
 
 		public static void UpdateVisibility(this FrameworkElement platformView, IView view)
@@ -379,8 +381,10 @@ internal static Graphics.Rect GetBoundingBox(this FrameworkElement? platformView
 
 		internal static void UnfocusControl(Control control)
 		{
-			if (control == null || !control.IsEnabled)
+			if (!control.IsEnabled)
+			{
 				return;
+			}
 
 			var isTabStop = control.IsTabStop;
 			control.IsTabStop = false;

diff --git a/src/Core/tests/DeviceTests.Shared/HandlerTests/Focus/FocusHandlerTests.cs b/src/Core/tests/DeviceTests.Shared/HandlerTests/Focus/FocusHandlerTests.cs
@@ -97,7 +97,7 @@ await AttachAndRun<LayoutHandler>(layout, async (contentViewHandler) =>
 				Assert.True(inputControl1.IsFocused);
 				Assert.False(inputControl2.IsFocused);
 
-				// UNfocus the first control (revert the focus)
+				// Unfocus the first control (revert the focus)
 				inputControl1.Handler.Invoke(nameof(IView.Unfocus));
 
 				// assert

diff --git a/src/TestUtils/src/DeviceTests/AssertionExtensions.Windows.cs b/src/TestUtils/src/DeviceTests/AssertionExtensions.Windows.cs
@@ -13,6 +13,7 @@
 using Microsoft.UI.Xaml.Media.Imaging;
 using Windows.Graphics.DirectX;
 using Windows.Storage.Streams;
+using Microsoft.UI.Xaml.Input;
 using Xunit;
 using Xunit.Sdk;
 using WColor = Windows.UI.Color;
@@ -49,42 +50,50 @@ public static Task SendKeyboardReturnType(this FrameworkElement view, ReturnType
 		public static async Task WaitForFocused(this FrameworkElement view, int timeout = 1000)
 		{
 			TaskCompletionSource focusSource = new TaskCompletionSource();
-			view.GotFocus += OnFocused;
+
+			FocusManager.GotFocus += OnFocused;
 
 			try
 			{
 				await focusSource.Task.WaitAsync(TimeSpan.FromMilliseconds(timeout));
 			}
 			finally
 			{
-				view.GotFocus -= OnFocused;
+				FocusManager.GotFocus -= OnFocused;
 			}
 
-			void OnFocused(object? sender, RoutedEventArgs e)
+			void OnFocused(object? sender, FocusManagerGotFocusEventArgs e)
 			{
-				view.GotFocus -= OnFocused;
-				focusSource.SetResult();
+				if (e.NewFocusedElement == view)
+				{
+					FocusManager.GotFocus -= OnFocused;
+					focusSource.SetResult();
+				}
 			}
 		}
 
 		public static async Task WaitForUnFocused(this FrameworkElement view, int timeout = 1000)
 		{
 			TaskCompletionSource focusSource = new TaskCompletionSource();
-			view.LostFocus += OnUnFocused;
+
+			FocusManager.LostFocus += OnUnFocused;
 
 			try
 			{
 				await focusSource.Task.WaitAsync(TimeSpan.FromMilliseconds(timeout));
 			}
 			finally
 			{
-				view.LostFocus -= OnUnFocused;
+				FocusManager.LostFocus -= OnUnFocused;
 			}
 
-			void OnUnFocused(object? sender, RoutedEventArgs e)
+			void OnUnFocused(object? sender, FocusManagerLostFocusEventArgs e)
 			{
-				view.LostFocus -= OnUnFocused;
-				focusSource.SetResult();
+				if (e.OldFocusedElement == view)
+				{
+					FocusManager.LostFocus -= OnUnFocused;
+					focusSource.SetResult();
+				}
 			}
 		}