From 066f89f8212eb4f1d048f40454149db0c585cac1 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 22 Nov 2024 14:54:19 +0200 Subject: [PATCH 1/4] fix(textbox): losing focus on PointerUp when inside a Popup on WASM --- .../Controls/ScrollViewer/ScrollViewer.MuxInternal.cs | 7 +++++++ .../UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs | 11 +++++++++++ src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs | 8 +++++++- src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs | 4 ++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.MuxInternal.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.MuxInternal.cs index da055cd0aa8a..a058f3c94505 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.MuxInternal.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.MuxInternal.cs @@ -179,6 +179,13 @@ protected override void OnPointerReleased(PointerRoutedEventArgs args) private static bool ScrollContentControl_SetFocusOnFlyoutLightDismissPopupByPointer(UIElement pScrollContentControl) { +#if __CROSSRUNTIME__ + if ((pScrollContentControl as ScrollViewer)?.DisableSetFocusOnPopupByPointer ?? false) + { + return false; + } +#endif + PopupRoot? pPopupRoot = null; Popup? pPopup = null; diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs index ff8b34c65ab3..a2174fa4151b 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs @@ -593,6 +593,17 @@ public double HorizontalOffset ); #endregion +#if __CROSSRUNTIME__ + /// + /// This is specifically added for ScrollViewers inside TextBoxes and specifically for WASM + /// On WASM, a click on a TextBox inside a popup shifts focus to the Popup instead of the TextBox + /// as a result of ScrollContentControl_SetFocusOnFlyoutLightDismissPopupByPointer. + /// This is only a problem on WASM because we don't capture the pointer when pressing inside a TextBox + /// . + /// + internal bool DisableSetFocusOnPopupByPointer { get; set; } +#endif + private readonly SerialDisposable _sizeChangedSubscription = new SerialDisposable(); #pragma warning disable 649 // unused member for Unit tests diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs index f2d61ea1b0f1..7276146f9438 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs @@ -28,6 +28,12 @@ public bool IsPointerCaptureRequired new FrameworkPropertyMetadata( // We should not capture the pointer on WASM by default because it would prevent the user from scrolling through text on selection. // See https://github.com/unoplatform/uno/pull/16982, https://issues.chromium.org/issues/344491566 - false)); + false, static (dO, args) => + { + if (((TextBox)dO)._contentElement is ScrollViewer sv) + { + sv.DisableSetFocusOnPopupByPointer = !(bool)args.NewValue; + } + })); #endif } diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs index a9b8d293542e..4a75b90b7dec 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs @@ -169,6 +169,10 @@ private protected override void OnLoaded() scrollViewer.HorizontalScrollBarVisibility = ScrollBarVisibility.Disabled; // The template sets this to Hidden scrollViewer.VerticalScrollBarVisibility = ScrollBarVisibility.Auto; // The template sets this to Hidden } + +#if __WASM__ + scrollViewer.DisableSetFocusOnPopupByPointer = !IsPointerCaptureRequired; +#endif #endif } } From b543dc75db899b0189e3dcc72db7502cb6e29d70 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Fri, 22 Nov 2024 15:13:40 +0200 Subject: [PATCH 2/4] test: add a manual test --- .../UITests.Shared/UITests.Shared.projitems | 7 ++++ .../AutoSuggestBox_PopupFocus.xaml | 38 +++++++++++++++++++ .../AutoSuggestBox_PopupFocus.xaml.cs | 13 +++++++ 3 files changed, 58 insertions(+) create mode 100644 src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml create mode 100644 src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml.cs diff --git a/src/SamplesApp/UITests.Shared/UITests.Shared.projitems b/src/SamplesApp/UITests.Shared/UITests.Shared.projitems index d59d12e62c59..176ea36b8b3f 100644 --- a/src/SamplesApp/UITests.Shared/UITests.Shared.projitems +++ b/src/SamplesApp/UITests.Shared/UITests.Shared.projitems @@ -1414,6 +1414,10 @@ Designer MSBuild:Compile + + Designer + MSBuild:Compile + Designer MSBuild:Compile @@ -6558,6 +6562,9 @@ AutoSuggestBox_Description.xaml + + AutoSuggestBox_PopupFocus.xaml + AutoSuggestBox_Icons.xaml diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml new file mode 100644 index 000000000000..e87af73d130d --- /dev/null +++ b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml @@ -0,0 +1,38 @@ + + + + + + diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml.cs b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml.cs new file mode 100644 index 000000000000..a15b251f5cb3 --- /dev/null +++ b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml.cs @@ -0,0 +1,13 @@ +using Uno.UI.Samples.Controls; +using Microsoft.UI.Xaml.Controls; + +namespace UITests.Windows_UI_Xaml_Controls.AutoSuggestBoxTests; + +[Sample("AutoSuggestBox", IsManualTest = true)] +public sealed partial class AutoSuggestBox_PopupFocus : Page +{ + public AutoSuggestBox_PopupFocus() + { + this.InitializeComponent(); + } +} From 183da4f8b522b924f50f1beee71ad93798132790 Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 25 Nov 2024 13:34:15 +0200 Subject: [PATCH 3/4] chore: make changes for WASM only --- .../Controls/ScrollViewer/ScrollViewer.MuxInternal.cs | 2 +- src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs | 2 +- src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs | 8 ++++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.MuxInternal.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.MuxInternal.cs index a058f3c94505..f46d339ba603 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.MuxInternal.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.MuxInternal.cs @@ -179,7 +179,7 @@ protected override void OnPointerReleased(PointerRoutedEventArgs args) private static bool ScrollContentControl_SetFocusOnFlyoutLightDismissPopupByPointer(UIElement pScrollContentControl) { -#if __CROSSRUNTIME__ +#if __WASM__ if ((pScrollContentControl as ScrollViewer)?.DisableSetFocusOnPopupByPointer ?? false) { return false; diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs index a2174fa4151b..d6c17d460f8b 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs @@ -593,7 +593,7 @@ public double HorizontalOffset ); #endregion -#if __CROSSRUNTIME__ +#if __WASM__ /// /// This is specifically added for ScrollViewers inside TextBoxes and specifically for WASM /// On WASM, a click on a TextBox inside a popup shifts focus to the Popup instead of the TextBox diff --git a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs index 7276146f9438..d4ae5cc37679 100644 --- a/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs +++ b/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.Uno.cs @@ -28,12 +28,16 @@ public bool IsPointerCaptureRequired new FrameworkPropertyMetadata( // We should not capture the pointer on WASM by default because it would prevent the user from scrolling through text on selection. // See https://github.com/unoplatform/uno/pull/16982, https://issues.chromium.org/issues/344491566 - false, static (dO, args) => + false +#if __WASM__ + , static (dO, args) => { if (((TextBox)dO)._contentElement is ScrollViewer sv) { sv.DisableSetFocusOnPopupByPointer = !(bool)args.NewValue; } - })); + } +#endif + )); #endif } From 9058d6d2444f705c76e6c634c5b872f290e5fe2d Mon Sep 17 00:00:00 2001 From: Ramez Ragaa Date: Mon, 25 Nov 2024 14:07:28 +0200 Subject: [PATCH 4/4] chore: remove the manual ui test in favor of a runtime test --- .../UITests.Shared/UITests.Shared.projitems | 7 ---- .../AutoSuggestBox_PopupFocus.xaml | 38 ------------------ .../AutoSuggestBox_PopupFocus.xaml.cs | 13 ------- .../Windows_UI_Xaml_Controls/Given_TextBox.cs | 39 +++++++++++++++++++ .../Controls/ScrollViewer/ScrollViewer.cs | 11 ------ .../ScrollViewer/ScrollViewer.wasm.cs | 9 +++++ 6 files changed, 48 insertions(+), 69 deletions(-) delete mode 100644 src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml delete mode 100644 src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml.cs diff --git a/src/SamplesApp/UITests.Shared/UITests.Shared.projitems b/src/SamplesApp/UITests.Shared/UITests.Shared.projitems index 176ea36b8b3f..d59d12e62c59 100644 --- a/src/SamplesApp/UITests.Shared/UITests.Shared.projitems +++ b/src/SamplesApp/UITests.Shared/UITests.Shared.projitems @@ -1414,10 +1414,6 @@ Designer MSBuild:Compile - - Designer - MSBuild:Compile - Designer MSBuild:Compile @@ -6562,9 +6558,6 @@ AutoSuggestBox_Description.xaml - - AutoSuggestBox_PopupFocus.xaml - AutoSuggestBox_Icons.xaml diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml deleted file mode 100644 index e87af73d130d..000000000000 --- a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml +++ /dev/null @@ -1,38 +0,0 @@ - - - - - - diff --git a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml.cs b/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml.cs deleted file mode 100644 index a15b251f5cb3..000000000000 --- a/src/SamplesApp/UITests.Shared/Windows_UI_Xaml_Controls/AutoSuggestBoxTests/AutoSuggestBox_PopupFocus.xaml.cs +++ /dev/null @@ -1,13 +0,0 @@ -using Uno.UI.Samples.Controls; -using Microsoft.UI.Xaml.Controls; - -namespace UITests.Windows_UI_Xaml_Controls.AutoSuggestBoxTests; - -[Sample("AutoSuggestBox", IsManualTest = true)] -public sealed partial class AutoSuggestBox_PopupFocus : Page -{ - public AutoSuggestBox_PopupFocus() - { - this.InitializeComponent(); - } -} diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_TextBox.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_TextBox.cs index a25f8b786c71..a34dd21378a8 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_TextBox.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_TextBox.cs @@ -28,6 +28,7 @@ using Windows.UI.Input.Preview.Injection; using Windows.Foundation; using System.Collections.Generic; +using Uno.Extensions; #if WINAPPSDK @@ -1124,6 +1125,44 @@ static FrameworkElement GetSCP(TextBox tb) } #endif +#if HAS_UNO + [TestMethod] + [UnoWorkItem("https://github.com/unoplatform/uno/issues/18790")] +#if !HAS_INPUT_INJECTOR + [Ignore("InputInjector is not supported on this platform.")] +#endif + public async Task When_Clicked_In_Popup() + { + TextBox tb; + var btn = new Button + { + Flyout = new Flyout + { + Content = tb = new TextBox() + } + }; + + await UITestHelper.Load(btn); + + var injector = InputInjector.TryCreate() ?? throw new InvalidOperationException("Failed to init the InputInjector"); + using var mouse = injector.GetMouse(); + + mouse.Press(btn.GetAbsoluteBoundsRect().GetCenter()); + await UITestHelper.WaitForIdle(); + mouse.Release(); + await UITestHelper.WaitForIdle(); + + Assert.IsTrue(btn.Flyout.IsOpen); + + mouse.Press(tb.GetAbsoluteBoundsRect().GetCenter()); + await UITestHelper.WaitForIdle(); + mouse.Release(); + await UITestHelper.WaitForIdle(); + + Assert.AreEqual(tb, FocusManager.GetFocusedElement(WindowHelper.XamlRoot)); + } +#endif + private static async Task LoadZeroSizeTextBoxAsync(Style style) { var loaded = false; diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs index d6c17d460f8b..ff8b34c65ab3 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.cs @@ -593,17 +593,6 @@ public double HorizontalOffset ); #endregion -#if __WASM__ - /// - /// This is specifically added for ScrollViewers inside TextBoxes and specifically for WASM - /// On WASM, a click on a TextBox inside a popup shifts focus to the Popup instead of the TextBox - /// as a result of ScrollContentControl_SetFocusOnFlyoutLightDismissPopupByPointer. - /// This is only a problem on WASM because we don't capture the pointer when pressing inside a TextBox - /// . - /// - internal bool DisableSetFocusOnPopupByPointer { get; set; } -#endif - private readonly SerialDisposable _sizeChangedSubscription = new SerialDisposable(); #pragma warning disable 649 // unused member for Unit tests diff --git a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.wasm.cs b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.wasm.cs index e709cb5ac605..185369d583cd 100644 --- a/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.wasm.cs +++ b/src/Uno.UI/UI/Xaml/Controls/ScrollViewer/ScrollViewer.wasm.cs @@ -11,6 +11,15 @@ namespace Microsoft.UI.Xaml.Controls { partial class ScrollViewer { + /// + /// This is specifically added for ScrollViewers inside TextBoxes and specifically for WASM + /// On WASM, a click on a TextBox inside a popup shifts focus to the Popup instead of the TextBox + /// as a result of ScrollContentControl_SetFocusOnFlyoutLightDismissPopupByPointer. + /// This is only a problem on WASM because we don't capture the pointer when pressing inside a TextBox + /// . + /// + internal bool DisableSetFocusOnPopupByPointer { get; set; } + internal bool CancelNextNativeScroll { get; private set; } internal Size ScrollBarSize => (_presenter as ScrollContentPresenter)?.ScrollBarSize ?? default;