From 6c815ea6aabd883167831607dfb03811b47e6aab Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 18:46:05 +0100 Subject: [PATCH 1/9] test: Ensure local style flows to a popup in visual tree --- .../Controls/StyleFlowToPopup.xaml | 24 ++++++++++++++ .../Controls/StyleFlowToPopup.xaml.cs | 30 +++++++++++++++++ .../Tests/Windows_UI_Xaml/Given_Style.cs | 33 ++++++++++++++----- 3 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/StyleFlowToPopup.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/StyleFlowToPopup.xaml.cs diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/StyleFlowToPopup.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/StyleFlowToPopup.xaml new file mode 100644 index 000000000000..7ce45c2e4ff9 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/StyleFlowToPopup.xaml @@ -0,0 +1,24 @@ + + + + + + + + + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/StyleFlowToPopup.xaml.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/StyleFlowToPopup.xaml.cs new file mode 100644 index 000000000000..785a59c2e583 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/StyleFlowToPopup.xaml.cs @@ -0,0 +1,30 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices.WindowsRuntime; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Controls.Primitives; +using Microsoft.UI.Xaml.Data; +using Microsoft.UI.Xaml.Input; +using Microsoft.UI.Xaml.Media; +using Microsoft.UI.Xaml.Navigation; +using Windows.Foundation; +using Windows.Foundation.Collections; + +namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls; + +public sealed partial class StyleFlowToPopup : Page +{ + public StyleFlowToPopup() + { + this.InitializeComponent(); + } + + public void ShowPopup() => TestPopup.IsOpen = true; + + public TextBlock GridTextBlock => InnerTextBlock; + + public TextBlock PopupTextBlock => PopupContentTextBlock; +} diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs index 11747ce677ae..abaef51b6a2e 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs @@ -1,17 +1,15 @@ using System; -using System.Collections.Generic; -using System.Text; using System.Threading.Tasks; -using Uno.UI.Extensions; +using Microsoft.UI; +using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Data; using Microsoft.UI.Xaml.Markup; -using Microsoft.VisualStudio.TestTools.UnitTesting; +using Microsoft.UI.Xaml.Media; using Private.Infrastructure; -using Microsoft.UI.Xaml; -using Microsoft.UI.Xaml.Data; -using Uno.UI.RuntimeTests.Helpers; using SamplesApp.UITests; -using Microsoft.UI.Xaml.Media; +using Uno.UI.RuntimeTests.Helpers; +using Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls; namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml { @@ -88,5 +86,24 @@ public async Task When_ImplicitStyle() Assert.AreEqual(HorizontalAlignment.Left, cc.HorizontalContentAlignment); } + + [TestMethod] + [RunsOnUIThread] + public async Task When_Style_Flows_To_Popup() + { + var page = new StyleFlowToPopup(); + TestServices.WindowHelper.WindowContent = page; + await UITestHelper.Load(page); + + var foreground = (SolidColorBrush)page.GridTextBlock.Foreground; + Assert.AreEqual(Colors.Red, foreground.Color); + + page.ShowPopup(); + + await TestServices.WindowHelper.WaitFor(() => VisualTreeHelper.GetOpenPopupsForXamlRoot(TestServices.WindowHelper.XamlRoot).Count > 0); + + var popupForeground = (SolidColorBrush)page.PopupTextBlock.Foreground; + Assert.AreEqual(Colors.Red, popupForeground.Color); + } } } From 6f486dfb90f81428dcb72a08abc2e89567c606e6 Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 19:14:37 +0100 Subject: [PATCH 2/9] fix: Do not output PopupPanel from VisualTreeHelper --- src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs index 7fabfc05fa15..4c73029f4e64 100644 --- a/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs +++ b/src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs @@ -270,6 +270,12 @@ private static IReadOnlyList GetOpenPopups(VisualTree visualTree) return uiElement.GetVisualTreeParent() as DependencyObject; } + if (realParent is PopupPanel) + { + // Skip the popup panel and go to PopupRoot instead. + realParent = GetParent(realParent); + } + return realParent; } From 7d558a747f3110a73378fde545e1136fa02a0434 Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 19:15:00 +0100 Subject: [PATCH 3/9] fix: Make the Popup logical parent of Child --- src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs | 9 +++++++++ src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs | 3 --- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs b/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs index aeb6cd78ee3c..b709914f35f4 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs @@ -138,6 +138,15 @@ partial void OnIsOpenChangedPartial(bool oldIsOpen, bool newIsOpen) partial void OnChildChangedPartial(UIElement oldChild, UIElement newChild) { + if (oldChild is FrameworkElement oldChildFe && oldChildFe.LogicalParentOverride == this) + { + oldChildFe.LogicalParentOverride = null; + } + if (newChild is FrameworkElement newChildFe) + { + newChildFe.LogicalParentOverride = this; + } + if (oldChild is IDependencyObjectStoreProvider provider && provider.Store.ReadLocalValue(provider.Store.DataContextProperty) != DependencyProperty.UnsetValue) { diff --git a/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs b/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs index 08c78e23372b..f7e198c4359e 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs @@ -231,9 +231,6 @@ Popup.PlacementTarget is not null private protected override void OnLoaded() { base.OnLoaded(); - // Set Parent to the Popup, to obtain the same behavior as UWP that the Popup (and therefore the rest of the main visual tree) - // is reachable by scaling the combined Parent/GetVisualParent() hierarchy. - this.SetLogicalParent(Popup); this.XamlRoot.Changed += XamlRootChanged; } From 8c11884f744ef684ac155f0f9b47f1c5cc9d26f4 Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 19:19:26 +0100 Subject: [PATCH 4/9] test: Popup child parent hierarchy --- .../Given_Popup.cs | 109 +++++++++++++----- 1 file changed, 77 insertions(+), 32 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls_Primitives/Given_Popup.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls_Primitives/Given_Popup.cs index 313c35c5c8cd..afbe8bcd388e 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls_Primitives/Given_Popup.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls_Primitives/Given_Popup.cs @@ -78,29 +78,10 @@ public void When_Closed_Immediately() } [TestMethod] - public async Task When_Child_Visual_Parents_Does_Not_Include_Popup() + public async Task When_Child_Visual_Parents_Do_Not_Include_Popup() { - var popup = new Popup(); - popup.XamlRoot = TestServices.WindowHelper.XamlRoot; - var button = new Button() - { - Content = "test" - }; - popup.Child = button; - popup.IsOpen = true; - await WindowHelper.WaitForLoaded(button); - bool found = false; - DependencyObject current = popup.Child; - while (current != null) - { - if (current == popup) - { - found = true; - break; - } - - current = VisualTreeHelper.GetParent(current); - } + var popup = await LoadAndOpenPopupWithButtonAsync(); + bool found = SearchPopupChildAscendants(popup, element => element == popup, element => VisualTreeHelper.GetParent(element)); Assert.IsFalse(found); @@ -110,6 +91,70 @@ public async Task When_Child_Visual_Parents_Does_Not_Include_Popup() [TestMethod] public async Task When_Child_Logical_Parents_Include_Popup() + { + var popup = await LoadAndOpenPopupWithButtonAsync(); + bool found = SearchPopupChildAscendants(popup, element => element == popup, element => (element as FrameworkElement)?.Parent); + + Assert.IsTrue(found); + + // Should not throw + popup.IsOpen = false; + } + + [TestMethod] + public async Task When_Child_Visual_Parent_Is_Canvas() + { + var popup = await LoadAndOpenPopupWithButtonAsync(); + var child = (FrameworkElement)popup.Child; + var parent = VisualTreeHelper.GetParent(child); + Assert.IsInstanceOfType(parent, typeof(Canvas)); +#if HAS_UNO // It is actually a PopupRoot, but it is internal in WinUI + Assert.IsInstanceOfType(parent, typeof(PopupRoot)); +#endif + + // Should not throw + popup.IsOpen = false; + } + + [TestMethod] + public async Task When_Child_Logical_Parent_Is_Popup() + { + var popup = await LoadAndOpenPopupWithButtonAsync(); + var child = (FrameworkElement)popup.Child; + + Assert.AreEqual(popup, child.Parent); + + // Should not throw + popup.IsOpen = false; + } + +#if HAS_UNO // PopupPanel is Uno-specific + [TestMethod] + public async Task When_Child_Visual_Parents_Do_Not_Include_PopupPanel() + { + var popup = await LoadAndOpenPopupWithButtonAsync(); + bool found = SearchPopupChildAscendants(popup, element => element is PopupPanel, VisualTreeHelper.GetParent); + + Assert.IsFalse(found); + + // Should not throw + popup.IsOpen = false; + } + + [TestMethod] + public async Task When_Child_Logical_Parents_Do_Not_Include_PopupPanel() + { + var popup = await LoadAndOpenPopupWithButtonAsync(); + bool found = SearchPopupChildAscendants(popup, element => element is PopupPanel, element => (element as FrameworkElement)?.Parent); + + Assert.IsFalse(found); + + // Should not throw + popup.IsOpen = false; + } +#endif + + private async Task LoadAndOpenPopupWithButtonAsync() { var popup = new Popup(); popup.XamlRoot = TestServices.WindowHelper.XamlRoot; @@ -120,23 +165,23 @@ public async Task When_Child_Logical_Parents_Include_Popup() popup.Child = button; popup.IsOpen = true; await WindowHelper.WaitForLoaded(button); - bool found = false; - DependencyObject current = button; + return popup; + } + + private bool SearchPopupChildAscendants(Popup popup, Predicate predicate, Func getParent) + { + DependencyObject current = popup.Child; while (current != null) { - if (current == popup) + if (predicate(current)) { - found = true; - break; + return true; } - current = (current as FrameworkElement)?.Parent; + current = getParent(current); } - Assert.IsTrue(found); - - // Should not throw - popup.IsOpen = false; + return false; } [TestMethod] From 8b15fb050781d4528f525109824e6975ed4b4a9c Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 19:19:38 +0100 Subject: [PATCH 5/9] fix: PopupRoot is a Canvas --- src/Uno.UI/UI/Xaml/Controls/Popup/PopupRoot.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/Popup/PopupRoot.cs b/src/Uno.UI/UI/Xaml/Controls/Popup/PopupRoot.cs index 83f7fc9d572e..f0fabfeb3c9c 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Popup/PopupRoot.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Popup/PopupRoot.cs @@ -17,7 +17,7 @@ namespace Microsoft.UI.Xaml.Controls.Primitives; -internal partial class PopupRoot : Panel +internal partial class PopupRoot : Canvas { private readonly List _openPopups = new(); From 5a56ca68e36ed5209c3b5f442c0f6de12292efd9 Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 19:21:07 +0100 Subject: [PATCH 6/9] fix: Prefer logical parent over visual for ResourceDictionary enumeration --- src/Uno.UI/UI/Xaml/DependencyObjectStore.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs index 54e44e504678..ab4423b7eabc 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs @@ -19,6 +19,7 @@ using Windows.ApplicationModel.Calls; using Microsoft.UI.Xaml.Controls; using System.Diagnostics.CodeAnalysis; +using Microsoft.UI.Xaml.Media; @@ -1637,9 +1638,13 @@ internal IEnumerable GetResourceDictionaries( { yield return fe.Resources; } - } - candidate = candidate.GetParent() as DependencyObject; + candidate = fe.Parent as FrameworkElement; + } + else + { + candidate = VisualTreeHelper.GetParent(candidate) as DependencyObject; + } } if (includeAppResources && Application.Current != null) From 54040fb4c5d5e790d2353666dd42786f3424e606 Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 19:26:56 +0100 Subject: [PATCH 7/9] chore: Use method --- src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs | 6 +++--- src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs b/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs index b709914f35f4..2c40841c924a 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs @@ -138,13 +138,13 @@ partial void OnIsOpenChangedPartial(bool oldIsOpen, bool newIsOpen) partial void OnChildChangedPartial(UIElement oldChild, UIElement newChild) { - if (oldChild is FrameworkElement oldChildFe && oldChildFe.LogicalParentOverride == this) + if (oldChild is FrameworkElement oldChildFe && oldChildFe.Parent == this) { - oldChildFe.LogicalParentOverride = null; + oldChildFe.SetLogicalParent(null); } if (newChild is FrameworkElement newChildFe) { - newChildFe.LogicalParentOverride = this; + newChildFe.SetLogicalParent(this); } if (oldChild is IDependencyObjectStoreProvider provider && diff --git a/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs b/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs index f7e198c4359e..5fd44c548318 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs @@ -238,7 +238,6 @@ private protected override void OnLoaded() private protected override void OnUnloaded() { base.OnUnloaded(); - this.SetLogicalParent(null); if (XamlRoot is { } xamlRoot) { From 165475e946a4551263657bca29c23909ebd11f0e Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 19:27:50 +0100 Subject: [PATCH 8/9] chore: Check should use LogicalParentOverride --- src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs b/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs index 2c40841c924a..ca5d31241540 100644 --- a/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs +++ b/src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs @@ -138,7 +138,7 @@ partial void OnIsOpenChangedPartial(bool oldIsOpen, bool newIsOpen) partial void OnChildChangedPartial(UIElement oldChild, UIElement newChild) { - if (oldChild is FrameworkElement oldChildFe && oldChildFe.Parent == this) + if (oldChild is FrameworkElement oldChildFe && oldChildFe.LogicalParentOverride == this) { oldChildFe.SetLogicalParent(null); } From 0f10125d4eb71bb944fc02b5c3d15c2d3a57143c Mon Sep 17 00:00:00 2001 From: Martin Zikmund Date: Wed, 27 Nov 2024 23:33:50 +0100 Subject: [PATCH 9/9] chore: Adjust namespace --- src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs index abaef51b6a2e..c2ff6f954032 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs @@ -96,14 +96,14 @@ public async Task When_Style_Flows_To_Popup() await UITestHelper.Load(page); var foreground = (SolidColorBrush)page.GridTextBlock.Foreground; - Assert.AreEqual(Colors.Red, foreground.Color); + Assert.AreEqual(Microsoft.UI.Colors.Red, foreground.Color); page.ShowPopup(); await TestServices.WindowHelper.WaitFor(() => VisualTreeHelper.GetOpenPopupsForXamlRoot(TestServices.WindowHelper.XamlRoot).Count > 0); var popupForeground = (SolidColorBrush)page.PopupTextBlock.Foreground; - Assert.AreEqual(Colors.Red, popupForeground.Color); + Assert.AreEqual(Microsoft.UI.Colors.Red, popupForeground.Color); } } }