Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Popup.Child parent hierarchy, ResourceDictionary resolution #18942

Merged
merged 9 commits into from
Nov 28, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<Page
x:Class="Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls.StyleFlowToPopup"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls"
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
mc:Ignorable="d"
Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">

<Grid>
<Grid.Resources>
<Style TargetType="TextBlock">
<Setter Property="Foreground" Value="Red" />
</Style>
</Grid.Resources>
<StackPanel>
<TextBlock x:Name="InnerTextBlock" Text="Red foreground" />
</StackPanel>
<Popup x:Name="TestPopup">
<TextBlock x:Name="PopupContentTextBlock" Text="This should have red foreground too!" />
</Popup>
</Grid>
</Page>
Original file line number Diff line number Diff line change
@@ -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;
}
33 changes: 25 additions & 8 deletions src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs
Original file line number Diff line number Diff line change
@@ -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
{
Expand Down Expand Up @@ -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(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(Microsoft.UI.Colors.Red, popupForeground.Color);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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<Popup> LoadAndOpenPopupWithButtonAsync()
{
var popup = new Popup();
popup.XamlRoot = TestServices.WindowHelper.XamlRoot;
Expand All @@ -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<DependencyObject> predicate, Func<DependencyObject, DependencyObject> 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]
Expand Down
9 changes: 9 additions & 0 deletions src/Uno.UI/UI/Xaml/Controls/Popup/Popup.Base.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.SetLogicalParent(null);
}
if (newChild is FrameworkElement newChildFe)
{
newChildFe.SetLogicalParent(this);
}

if (oldChild is IDependencyObjectStoreProvider provider &&
provider.Store.ReadLocalValue(provider.Store.DataContextProperty) != DependencyProperty.UnsetValue)
{
Expand Down
4 changes: 0 additions & 4 deletions src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -231,17 +231,13 @@ 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;
}

private protected override void OnUnloaded()
{
base.OnUnloaded();
this.SetLogicalParent(null);

if (XamlRoot is { } xamlRoot)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI/UI/Xaml/Controls/Popup/PopupRoot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

namespace Microsoft.UI.Xaml.Controls.Primitives;

internal partial class PopupRoot : Panel
internal partial class PopupRoot : Canvas
{
private readonly List<ManagedWeakReference> _openPopups = new();

Expand Down
9 changes: 7 additions & 2 deletions src/Uno.UI/UI/Xaml/DependencyObjectStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
using Windows.ApplicationModel.Calls;
using Microsoft.UI.Xaml.Controls;
using System.Diagnostics.CodeAnalysis;
using Microsoft.UI.Xaml.Media;



Expand Down Expand Up @@ -1637,9 +1638,13 @@ internal IEnumerable<ResourceDictionary> 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)
Expand Down
6 changes: 6 additions & 0 deletions src/Uno.UI/UI/Xaml/Media/VisualTreeHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,12 @@ private static IReadOnlyList<Popup> 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;
}

Expand Down
Loading