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): Put Popup in Parent hierarchy of its Child content #5944

Merged
merged 8 commits into from
May 21, 2021
4 changes: 2 additions & 2 deletions src/Uno.UI.Runtime.Skia.Gtk/GtkHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ private void UpdateWindowPropertiesFromPackage()

if (File.Exists(iconPath))
{
if (this.Log().IsEnabled(Microsoft.Extensions.Logging.LogLevel.Warning))
if (this.Log().IsEnabled(Microsoft.Extensions.Logging.LogLevel.Information))
{
this.Log().Warn($"Loading icon file [{iconPath}] from Package.appxmanifest file");
this.Log().Info($"Loading icon file [{iconPath}] from Package.appxmanifest file");
}

GtkHost.Window.SetIconFromFile(iconPath);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,28 +110,29 @@ void AssertItem(int index)
?? throw new NullReferenceException($"Cannot find the materialized border of item {index}");

var containerToListView = container.TransformToVisual(listView).TransformBounds(new Rect(0, 0, 42, 42));
Assert.IsTrue(Math.Abs(containerToListView.X) < tolerance);
Assert.IsTrue(Math.Abs(containerToListView.Y - ((100 + 5 * 2) * index)) < tolerance);
Assert.IsTrue(Math.Abs(containerToListView.Width - 42) < tolerance);
Assert.IsTrue(Math.Abs(containerToListView.Height - 42) < tolerance);
Assert.AreEqual(containerToListView.X, 0, tolerance);
Assert.AreEqual(containerToListView.X, 0, tolerance);
Assert.AreEqual(containerToListView.Y, ((100 + 5 * 2) * index), tolerance);
Assert.AreEqual(containerToListView.Width, 42, tolerance);
Assert.AreEqual(containerToListView.Height, 42, tolerance);

var borderToListView = border.TransformToVisual(listView).TransformBounds(new Rect(0, 0, 42, 42));
Assert.IsTrue(Math.Abs(borderToListView.X) < tolerance);
Assert.IsTrue(Math.Abs(borderToListView.Y - ((100 + 5 * 2) * index + 5)) < tolerance);
Assert.IsTrue(Math.Abs(borderToListView.Width - 42) < tolerance);
Assert.IsTrue(Math.Abs(borderToListView.Height - 42) < tolerance);
Assert.AreEqual(borderToListView.X, 0, tolerance);
Assert.AreEqual(borderToListView.Y, ((100 + 5 * 2) * index + 5), tolerance);
Assert.AreEqual(borderToListView.Width, 42, tolerance);
Assert.AreEqual(borderToListView.Height, 42, tolerance);

var containerToSut = container.TransformToVisual(sut).TransformBounds(new Rect(0, 0, 42, 42));
Assert.IsTrue(Math.Abs(containerToSut.X - 15) < tolerance);
Assert.IsTrue(Math.Abs(containerToSut.Y - (15 + (100 + 5 * 2) * index)) < tolerance);
Assert.IsTrue(Math.Abs(containerToSut.Width - 42) < tolerance);
Assert.IsTrue(Math.Abs(containerToSut.Height - 42) < tolerance);
Assert.AreEqual(containerToSut.X, 15, tolerance);
Assert.AreEqual(containerToSut.Y, (15 + (100 + 5 * 2) * index), tolerance);
Assert.AreEqual(containerToSut.Width, 42, tolerance);
Assert.AreEqual(containerToSut.Height, 42, tolerance);

var borderToSut = border.TransformToVisual(sut).TransformBounds(new Rect(0, 0, 42, 42));
Assert.IsTrue(Math.Abs(borderToSut.X - 15) < tolerance);
Assert.IsTrue(Math.Abs(borderToSut.Y - (15 + (100 + 5 * 2) * index + 5)) < tolerance);
Assert.IsTrue(Math.Abs(borderToSut.Width - 42) < tolerance);
Assert.IsTrue(Math.Abs(borderToSut.Height - 42) < tolerance);
Assert.AreEqual(borderToSut.X, 15, tolerance);
Assert.AreEqual(borderToSut.Y, (15 + (100 + 5 * 2) * index + 5), tolerance);
Assert.AreEqual(borderToSut.Width, 42, tolerance);
Assert.AreEqual(borderToSut.Height, 42, tolerance);
}
}
#endif
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls_Primitives.PopupPages;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Media;
using static Private.Infrastructure.TestServices;

namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls_Primitives
{
[TestClass]
[RunsOnUIThread]
public class Given_Popup
{
[TestMethod]
public async Task Check_Can_Reach_Main_Visual_Tree()
{
var page = new ReachMainTreePage();
WindowHelper.WindowContent = page;

await WindowHelper.WaitForLoaded(page);

Assert.IsTrue(CanReach(page.DummyTextBlock, page));

try
{
page.TargetPopup.IsOpen = true;
await WindowHelper.WaitForLoaded(page.PopupButton);

Assert.IsTrue(CanReach(page.PopupButton, page));
}
finally
{
page.TargetPopup.IsOpen = false;
}
}

#if __ANDROID__
[TestMethod]
public async Task Check_Can_Reach_Main_Visual_Tree_Alternate_Mode()
{
var originalConfig = FeatureConfiguration.Popup.UseNativePopup;
FeatureConfiguration.Popup.UseNativePopup = !originalConfig;
try
{
await Check_Can_Reach_Main_Visual_Tree();
}
finally
{
FeatureConfiguration.Popup.UseNativePopup = originalConfig;
}
}
#endif

private static bool CanReach(DependencyObject startingElement, DependencyObject targetElement)
{
var currentElement = startingElement;
while (currentElement != null)
{
if (currentElement == targetElement)
{
return true;
}

// Quoting WCT DataGrid:
// // Walk up the visual tree. Try using the framework element's
// // parent. We do this because Popups behave differently with respect to the visual tree,
// // and it could have a parent even if the VisualTreeHelper doesn't find it.
DependencyObject parent = null;
if (currentElement is FrameworkElement fe)
{
parent = fe.Parent;
}
if (parent == null)
{
parent = VisualTreeHelper.GetParent(currentElement);
}

currentElement = parent;
}

// Did not hit targetElement
return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<Page x:Class="Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls_Primitives.PopupPages.ReachMainTreePage"
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_Primitives.PopupPages"
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>
<TextBlock x:Name="DummyTextBlock"
x:FieldModifier="public"
Text="Page with Popup" />
<Popup x:Name="TargetPopup"
x:FieldModifier="public">
<StackPanel>
<TextBlock Text="Popup content" />
<Grid>
<Button x:Name="PopupButton"
x:FieldModifier="public"
Content="Button in Popup" />
</Grid>
</StackPanel>
</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 Windows.Foundation;
using Windows.Foundation.Collections;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;
using Windows.UI.Xaml.Controls.Primitives;
using Windows.UI.Xaml.Data;
using Windows.UI.Xaml.Input;
using Windows.UI.Xaml.Media;
using Windows.UI.Xaml.Navigation;

// The Blank Page item template is documented at https://go.microsoft.com/fwlink/?LinkId=234238

namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls_Primitives.PopupPages
{
/// <summary>
/// An empty page that can be used on its own or navigated to within a Frame.
/// </summary>
public sealed partial class ReachMainTreePage : Page
{
public ReachMainTreePage()
{
this.InitializeComponent();
}
}
}
21 changes: 0 additions & 21 deletions src/Uno.UI/UI/Xaml/Controls/ListViewBase/ListViewBaseSource.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,6 @@ public override CGRect Frame
{
base.Frame = value;
UpdateContentViewFrame();
UpdateContentLayoutSlots(value);
}
}

Expand All @@ -763,7 +762,6 @@ public override CGRect Bounds
}
base.Bounds = value;
UpdateContentViewFrame();
UpdateContentLayoutSlots(Frame);
}
}

Expand All @@ -779,20 +777,6 @@ private void UpdateContentViewFrame()
}
}

/// <summary>
/// Fakely propagate the applied Frame of this internal container as the LayoutSlot of the publicly visible container.
/// This is required for the UIElement.TransformToVisual to work properly.
/// </summary>
private void UpdateContentLayoutSlots(Rect frame)
{
var content = Content;
if (content != null)
{
LayoutInformation.SetLayoutSlot(content, frame);
content.LayoutSlotWithMarginsAndAlignments = frame;
}
}

/// <summary>
/// Clear the cell's measured size, this allows the static template size to be updated with the correct databound size.
/// </summary>
Expand Down Expand Up @@ -930,11 +914,6 @@ public override void LayoutSubviews()
if (Content != null)
{
Layouter.ArrangeChild(Content, new Rect(0, 0, (float)size.Width, (float)size.Height));

// The item has to be arranged relative to this internal container (at 0,0),
// but doing this the LayoutSlot[WithMargins] has been updated,
// so we fakely re-inject the relative position of the item in its parent.
UpdateContentLayoutSlots(Frame);
}
}

Expand Down
21 changes: 11 additions & 10 deletions src/Uno.UI/UI/Xaml/Controls/Popup/PopupPanel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ protected override Size MeasureOverride(Size availableSize)

if (this.Log().IsEnabled(LogLevel.Debug))
{
this.Log().LogDebug($"Measured PopupPanel #={GetHashCode()} ({(Popup.CustomLayouter == null?"":"**using custom layouter**")}) DC={Popup.DataContext} child={child} offset={Popup.HorizontalOffset},{Popup.VerticalOffset} availableSize={availableSize} measured={_lastMeasuredSize}");
this.Log().LogDebug($"Measured PopupPanel #={GetHashCode()} ({(Popup.CustomLayouter == null ? "" : "**using custom layouter**")}) DC={Popup.DataContext} child={child} offset={Popup.HorizontalOffset},{Popup.VerticalOffset} availableSize={availableSize} measured={_lastMeasuredSize}");
}

// Note that we return the availableSize and not the _lastMeasuredSize. This is because this
Expand Down Expand Up @@ -123,7 +123,7 @@ protected override Size ArrangeOverride(Size finalSize)
// for android, the above line returns the absolute coordinates of anchor on the screen
// because the parent view of this PopupPanel is a PopupWindow and GetLocationInWindow will be (0,0)
// therefore, we need to make the relative adjustment
if (this.GetParent() is Android.Views.View view)
if (this.VisualParent is Android.Views.View view)
{
var windowLocation = Point.From(view.GetLocationInWindow);
var screenLocation = Point.From(view.GetLocationOnScreen);
Expand Down Expand Up @@ -174,17 +174,18 @@ protected override Size ArrangeOverride(Size finalSize)
return finalSize;
}

#if __ANDROID__
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
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);
}

private protected override void OnUnloaded()
{
base.OnUnloaded();

if (FeatureConfiguration.Popup.UseNativePopup)
{
// Unset parent here, because it doesn't get unset in the usual way because the parent is a native view.
this.SetParent(null);
}
this.SetLogicalParent(null);
}
#endif
}
}
17 changes: 16 additions & 1 deletion src/Uno.UI/UI/Xaml/DependencyObjectExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ internal static bool HasParent(this object dependencyObject, DependencyObject se
var parent = dependencyObject.GetParent();
while (parent != null)
{
if(ReferenceEquals(parent, searchedParent))
if (ReferenceEquals(parent, searchedParent))
{
return true;
}
Expand All @@ -127,6 +127,21 @@ internal static void SetParent(this object dependencyObject, object parent)
GetStore(dependencyObject).Parent = parent;
}

internal static void SetLogicalParent(this FrameworkElement element, DependencyObject logicalParent)
{
#if UNO_HAS_MANAGED_POINTERS || __WASM__ // WASM has managed-esque pointers

// UWP distinguishes between the 'logical parent' (or inheritance parent) and the 'visual parent' of an element. Uno already
// recognises this distinction on some targets, but for targets using CoerceHitTestVisibility() for hit testing, the pointer
// implementation depends upon the logical parent (ie DepObjStore.Parent) being identical to the visual parent, because it
// piggybacks on the DP inheritance mechanism. Therefore we use LogicalParentOverride as a workaround to modify the publicly-visible
// FrameworkElement.Parent without affecting DP propagation.
element.LogicalParentOverride = logicalParent;
#else
SetParent(element, logicalParent);
#endif
}

/// <summary>
/// Creates a SetValue precedence scoped override. All calls to SetValue
/// on the specified instance will be set to the specified precedence.
Expand Down
7 changes: 6 additions & 1 deletion src/Uno.UI/UI/Xaml/FrameworkElement.EffectiveViewport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
using Uno.UI;
using Uno.UI.Extensions;
using _This = Windows.UI.Xaml.FrameworkElement;
#if __IOS__
using UIKit;
#elif __MACOS__
using AppKit;
#endif

namespace Windows.UI.Xaml
{
Expand Down Expand Up @@ -77,7 +82,7 @@ private void ReconfigureViewportPropagation(IFrameworkElement_EffectiveViewport
{
TRACE_EFFECTIVE_VIEWPORT("Enabling effective viewport propagation.");

var parent = Parent;
var parent = this.GetVisualTreeParent();
if (parent is IFrameworkElement_EffectiveViewport parentFwElt)
{
_parentViewportUpdatesSubscription = parentFwElt.RequestViewportUpdates(this);
Expand Down
16 changes: 15 additions & 1 deletion src/Uno.UI/UI/Xaml/FrameworkElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,21 @@ Windows.UI.Xaml.ResourceDictionary Resources
#if __ANDROID__
new
#endif
DependencyObject Parent => ((IDependencyObjectStoreProvider)this).Store.Parent as DependencyObject;
DependencyObject Parent =>
#if UNO_HAS_MANAGED_POINTERS || __WASM__
LogicalParentOverride ??
#endif
((IDependencyObjectStoreProvider)this).Store.Parent as DependencyObject;


#if UNO_HAS_MANAGED_POINTERS || __WASM__
/// <summary>
/// Allows to override the publicly-visible <see cref="Parent"/> without modifying DP propagation.
/// </summary>
internal DependencyObject LogicalParentOverride { get; set; }

internal UIElement VisualParent => ((IDependencyObjectStoreProvider)this).Store.Parent as UIElement;
#endif

private bool _isParsing;
/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI/UI/Xaml/FrameworkElement.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public override void LayoutSubviews()

OnBeforeArrange();

var finalRect = Parent is UIElement ? LayoutSlotWithMarginsAndAlignments : RectFromUIRect(Frame);
var finalRect = Superview is UIElement ? LayoutSlotWithMarginsAndAlignments : RectFromUIRect(Frame);

_layouter.Arrange(finalRect);

Expand Down
2 changes: 2 additions & 0 deletions src/Uno.UI/UI/Xaml/FrameworkElement.net.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public partial class FrameworkElement : IEnumerable

internal List<View> _children = new List<View>();

internal UIElement VisualParent => ((IDependencyObjectStoreProvider)this).Store.Parent as UIElement;

private protected virtual void OnPostLoading() { }

partial void OnLoadingPartial();
Expand Down
Loading