From ae39c27b406fbc8f5fef8cad9f58f8f0c01256be Mon Sep 17 00:00:00 2001 From: MartyIX <203266+MartyIX@users.noreply.github.com> Date: Wed, 21 Aug 2024 16:06:42 +0200 Subject: [PATCH 1/3] WIPWIP --- .../src/Handlers/View/ViewHandler.Windows.cs | 48 ++++++++++++++----- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/src/Core/src/Handlers/View/ViewHandler.Windows.cs b/src/Core/src/Handlers/View/ViewHandler.Windows.cs index f7b36aa79dae..5ee087a3123b 100644 --- a/src/Core/src/Handlers/View/ViewHandler.Windows.cs +++ b/src/Core/src/Handlers/View/ViewHandler.Windows.cs @@ -1,31 +1,39 @@ #nullable enable using System; using System.Collections.Generic; -using System.ComponentModel; 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 { + static Dictionary? FocusManagerMapping; + partial void ConnectingHandler(PlatformView? platformView) { - if (platformView != null) + if (platformView is not null) { - platformView.GotFocus += OnPlatformViewGotFocus; - platformView.LostFocus += OnPlatformViewLostFocus; + if (FocusManagerMapping is null) + { + FocusManagerMapping = []; + + FocusManager.GotFocus += FocusManager_GotFocus; + FocusManager.LostFocus += FocusManager_LostFocus; + } + + FocusManagerMapping.Add(platformView, this); } } partial void DisconnectingHandler(PlatformView platformView) { - UpdateIsFocused(false); + _ = FocusManagerMapping ?? throw new InvalidOperationException($"{nameof(FocusManagerMapping)} should have been set."); - platformView.GotFocus -= OnPlatformViewGotFocus; - platformView.LostFocus -= OnPlatformViewLostFocus; + UpdateIsFocused(false); + FocusManagerMapping.Remove(platformView); } static partial void MappingFrame(IViewHandler handler, IView view) @@ -88,7 +96,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 +143,39 @@ internal static void MapContextFlyout(IElementHandler handler, IContextFlyoutEle } } - void OnPlatformViewGotFocus(object sender, RoutedEventArgs args) + static void FocusManager_GotFocus(object? sender, FocusManagerGotFocusEventArgs e) { - UpdateIsFocused(true); + _ = FocusManagerMapping ?? throw new InvalidOperationException($"{nameof(FocusManagerMapping)} should have been set."); + + if (e.NewFocusedElement is PlatformView platformView && FocusManagerMapping.TryGetValue(platformView, out ViewHandler? viewHandler)) + { + viewHandler.UpdateIsFocused(true); + } } - void OnPlatformViewLostFocus(object sender, RoutedEventArgs args) + static void FocusManager_LostFocus(object? sender, FocusManagerLostFocusEventArgs e) { - UpdateIsFocused(false); + _ = FocusManagerMapping ?? throw new InvalidOperationException($"{nameof(FocusManagerMapping)} should have been set."); + + if (e.OldFocusedElement is PlatformView platformView && FocusManagerMapping.TryGetValue(platformView, out ViewHandler? viewHandler)) + { + viewHandler.UpdateIsFocused(false); + } } void UpdateIsFocused(bool isFocused) { if (VirtualView == null) + { return; + } bool updateIsFocused = (isFocused && !VirtualView.IsFocused) || (!isFocused && VirtualView.IsFocused); if (updateIsFocused) + { VirtualView.IsFocused = isFocused; + } } } } \ No newline at end of file From 68155a32a21673871595f359390d6ed9a644013c Mon Sep 17 00:00:00 2001 From: MartyIX <203266+MartyIX@users.noreply.github.com> Date: Wed, 21 Aug 2024 20:27:52 +0200 Subject: [PATCH 2/3] Feedback --- src/Core/src/Handlers/View/ViewHandler.Windows.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Core/src/Handlers/View/ViewHandler.Windows.cs b/src/Core/src/Handlers/View/ViewHandler.Windows.cs index 5ee087a3123b..2be4405af890 100644 --- a/src/Core/src/Handlers/View/ViewHandler.Windows.cs +++ b/src/Core/src/Handlers/View/ViewHandler.Windows.cs @@ -1,6 +1,7 @@ #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; @@ -10,7 +11,7 @@ namespace Microsoft.Maui.Handlers { public partial class ViewHandler { - static Dictionary? FocusManagerMapping; + static ConditionalWeakTable? FocusManagerMapping; partial void ConnectingHandler(PlatformView? platformView) { From 14fd6af427003560c373e96203aee74ce97e3770 Mon Sep 17 00:00:00 2001 From: Alberto Aldegheri Date: Mon, 9 Sep 2024 15:20:05 +0200 Subject: [PATCH 3/3] Refactor usage of `FocusManager` to avoid weak table usage --- .../src/Handlers/View/ViewHandler.Windows.cs | 43 ++++++------------- .../src/Platform/Windows/ViewExtensions.cs | 21 ++++++++- .../HandlerTests/Focus/FocusHandlerTests.cs | 2 +- .../AssertionExtensions.Windows.cs | 29 ++++++++----- 4 files changed, 54 insertions(+), 41 deletions(-) diff --git a/src/Core/src/Handlers/View/ViewHandler.Windows.cs b/src/Core/src/Handlers/View/ViewHandler.Windows.cs index 2be4405af890..24bafb60a10e 100644 --- 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? 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 index c275fb1c378e..0a3144a0f3b9 100644 --- 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), typeof(FrameworkElement), new PropertyMetadata(null)); + + internal static void SetMauiHandler(this FrameworkElement element, ViewHandler? handler) + { + var weakRef = handler != null ? new WeakReference(handler) : null; + element.SetValue(MauiHandlerProperty, weakRef); + } + + internal static ViewHandler? GetMauiHandler(this FrameworkElement element) + { + var weakRef = (WeakReference?)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) @@ -370,8 +387,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 index 9baba69af963..8e20e9784cdc 100644 --- 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(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 index 045ca092eda5..a7c83e7f9be8 100644 --- 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,7 +50,8 @@ 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 { @@ -57,20 +59,24 @@ public static async Task WaitForFocused(this FrameworkElement view, int 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 { @@ -78,13 +84,16 @@ public static async Task WaitForUnFocused(this FrameworkElement view, int timeou } 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(); + } } }