Refactor usage of FocusManager to avoid weak table usage

dotnet · Sep 10, 2024 · ab5551f · ab5551f
1 parent 35ee46b
commit ab5551f
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 41 deletions.
diff --git a/src/Core/src/Handlers/View/ViewHandler.Windows.cs b/src/Core/src/Handlers/View/ViewHandler.Windows.cs
@@ -1,7 +1,5 @@
 #nullable enable
 using System;
-using System.Collections.Generic;
-using System.Runtime.CompilerServices;
 using Microsoft.UI.Xaml;
 using Microsoft.UI.Xaml.Controls.Primitives;
 using Microsoft.UI.Xaml.Input;
@@ -11,30 +9,21 @@ namespace Microsoft.Maui.Handlers
 {
 	public partial class ViewHandler
 	{
-		static ConditionalWeakTable<PlatformView, ViewHandler>? FocusManagerMapping;
+		static ViewHandler()
+		{
+			FocusManager.GotFocus += FocusManager_GotFocus;
+			FocusManager.LostFocus += FocusManager_LostFocus;
+		}
 
 		partial void ConnectingHandler(PlatformView? platformView)
 		{
-			if (platformView is not null)
-			{
-				if (FocusManagerMapping is null)
-				{
-					FocusManagerMapping = [];
-
-					FocusManager.GotFocus += FocusManager_GotFocus;
-					FocusManager.LostFocus += FocusManager_LostFocus;
-				}
-
-				FocusManagerMapping.Add(platformView, this);
-			}
+			platformView?.SetMauiHandler(this);
 		}
 
 		partial void DisconnectingHandler(PlatformView platformView)
 		{
-			_ = FocusManagerMapping ?? throw new InvalidOperationException($"{nameof(FocusManagerMapping)} should have been set.");
-
+			platformView.SetMauiHandler(null);
 			UpdateIsFocused(false);
-			FocusManagerMapping.Remove(platformView);
 		}
 
 		static partial void MappingFrame(IViewHandler handler, IView view)
@@ -146,36 +135,32 @@ internal static void MapContextFlyout(IElementHandler handler, IContextFlyoutEle
 
 		static void FocusManager_GotFocus(object? sender, FocusManagerGotFocusEventArgs e)
 		{
-			_ = FocusManagerMapping ?? throw new InvalidOperationException($"{nameof(FocusManagerMapping)} should have been set.");
-
-			if (e.NewFocusedElement is PlatformView platformView && FocusManagerMapping.TryGetValue(platformView, out ViewHandler? viewHandler))
+			if (e.NewFocusedElement is PlatformView platformView && platformView.GetMauiHandler() is { } handler)
 			{
-				viewHandler.UpdateIsFocused(true);
+				handler.UpdateIsFocused(true);
 			}
 		}
 
 		static void FocusManager_LostFocus(object? sender, FocusManagerLostFocusEventArgs e)
 		{
-			_ = FocusManagerMapping ?? throw new InvalidOperationException($"{nameof(FocusManagerMapping)} should have been set.");
-
-			if (e.OldFocusedElement is PlatformView platformView && FocusManagerMapping.TryGetValue(platformView, out ViewHandler? viewHandler))
+			if (e.OldFocusedElement is PlatformView platformView && platformView.GetMauiHandler() is { } handler)
 			{
-				viewHandler.UpdateIsFocused(false);
+				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
@@ -19,6 +19,21 @@ namespace Microsoft.Maui.Platform
 {
 	public static partial class ViewExtensions
 	{
+		internal static readonly DependencyProperty MauiHandlerProperty = DependencyProperty.RegisterAttached("MauiHandler",
+			typeof(WeakReference<ViewHandler>), typeof(FrameworkElement), new PropertyMetadata(null));
+
+		internal static void SetMauiHandler(this FrameworkElement element, ViewHandler? handler)
+		{
+			var weakRef = handler != null ? new WeakReference<ViewHandler>(handler) : null;
+			element.SetValue(MauiHandlerProperty, weakRef);
+		}
+
+		internal static ViewHandler? GetMauiHandler(this FrameworkElement element)
+		{
+			var weakRef = (WeakReference<ViewHandler>?)element.GetValue(MauiHandlerProperty);
+			return weakRef?.TryGetTarget(out var viewHandler) == true ? viewHandler : null;
+		}
+
 		public static void TryMoveFocus(this FrameworkElement platformView, FocusNavigationDirection direction)
 		{
 			if (platformView?.XamlRoot?.Content is UIElement elem)
@@ -36,7 +51,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)
@@ -378,8 +395,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();
+				}
 			}
 		}