From 59a75d01557ee9082a848c9b783ef0287c1a67b1 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Sun, 5 Dec 2021 20:06:20 -0500 Subject: [PATCH] Merge pull request #7071 from simonhaines/fixes/popup-positioning Fix issues with positioning of non-overlay popups --- src/Avalonia.Controls/Primitives/Popup.cs | 33 +++++- .../Primitives/PopupTests.cs | 108 ++++++++++++++++++ .../MockWindowingPlatform.cs | 3 + 3 files changed, 138 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs index 50f130ab71..30e90267db 100644 --- a/src/Avalonia.Controls/Primitives/Popup.cs +++ b/src/Avalonia.Controls/Primitives/Popup.cs @@ -8,6 +8,7 @@ using Avalonia.Controls.Presenters; using Avalonia.Controls.Primitives.PopupPositioning; using Avalonia.Input; using Avalonia.Input.Raw; +using Avalonia.Layout; using Avalonia.LogicalTree; using Avalonia.Metadata; using Avalonia.Platform; @@ -395,7 +396,7 @@ namespace Avalonia.Controls.Primitives _isOpenRequested = false; var popupHost = OverlayPopupHost.CreatePopupHost(placementTarget, DependencyResolver); - var handlerCleanup = new CompositeDisposable(5); + var handlerCleanup = new CompositeDisposable(7); popupHost.BindConstraints(this, WidthProperty, MinWidthProperty, MaxWidthProperty, HeightProperty, MinHeightProperty, MaxHeightProperty, TopmostProperty).DisposeWith(handlerCleanup); @@ -423,14 +424,28 @@ namespace Avalonia.Controls.Primitives (x, handler) => x.Deactivated -= handler).DisposeWith(handlerCleanup); SubscribeToEventHandler(window.PlatformImpl, WindowLostFocus, - (x, handler) => x.LostFocus += handler, - (x, handler) => x.LostFocus -= handler).DisposeWith(handlerCleanup); + (x, handler) => x.LostFocus += handler, + (x, handler) => x.LostFocus -= handler).DisposeWith(handlerCleanup); + + SubscribeToEventHandler>(window.PlatformImpl, WindowPositionChanged, + (x, handler) => x.PositionChanged += handler, + (x, handler) => x.PositionChanged -= handler).DisposeWith(handlerCleanup); + + if (placementTarget is Layoutable layoutTarget) + { + // If the placement target is moved, update the popup position + SubscribeToEventHandler(layoutTarget, PlacementTargetLayoutUpdated, + (x, handler) => x.LayoutUpdated += handler, + (x, handler) => x.LayoutUpdated -= handler).DisposeWith(handlerCleanup); + } } - else + else if (topLevel is PopupRoot parentPopupRoot) { - var parentPopupRoot = topLevel as PopupRoot; + SubscribeToEventHandler>(parentPopupRoot, ParentPopupPositionChanged, + (x, handler) => x.PositionChanged += handler, + (x, handler) => x.PositionChanged -= handler).DisposeWith(handlerCleanup); - if (parentPopupRoot?.Parent is Popup popup) + if (parentPopupRoot.Parent is Popup popup) { SubscribeToEventHandler>(popup, ParentClosed, (x, handler) => x.Closed += handler, @@ -795,6 +810,12 @@ namespace Avalonia.Controls.Primitives Close(); } + private void WindowPositionChanged(PixelPoint pp) => HandlePositionChange(); + + private void PlacementTargetLayoutUpdated(object src, EventArgs e) => HandlePositionChange(); + + private void ParentPopupPositionChanged(object src, PixelPointEventArgs e) => HandlePositionChange(); + private IgnoreIsOpenScope BeginIgnoringIsOpen() { return new IgnoreIsOpenScope(this); diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs index 090a20c857..24e4631aff 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs @@ -5,6 +5,7 @@ using System.Linq; using Moq; using Avalonia.Controls.Presenters; using Avalonia.Controls.Primitives; +using Avalonia.Controls.Primitives.PopupPositioning; using Avalonia.Controls.Templates; using Avalonia.Layout; using Avalonia.LogicalTree; @@ -595,6 +596,113 @@ namespace Avalonia.Controls.UnitTests.Primitives } } + [Fact] + public void Popup_Should_Follow_Placement_Target_On_Window_Move() + { + using (CreateServices()) + { + var popup = new Popup { Width = 400, Height = 200 }; + var window = PreparedWindow(popup); + popup.Open(); + + if (popup.Host is PopupRoot popupRoot) + { + // Moving the window must move the popup (screen coordinates have changed) + var raised = false; + popupRoot.PositionChanged += (_, args) => + { + Assert.Equal(new PixelPoint(10, 10), args.Point); + raised = true; + }; + + window.Position = new PixelPoint(10, 10); + Assert.True(raised); + } + } + } + + [Fact] + public void Popup_Should_Follow_Placement_Target_On_Window_Resize() + { + using (CreateServices()) + { + + var placementTarget = new Panel() + { + Width = 10, + Height = 10, + HorizontalAlignment = HorizontalAlignment.Center, + VerticalAlignment = VerticalAlignment.Center + }; + var popup = new Popup() { PlacementTarget = placementTarget, Width = 10, Height = 10 }; + ((ISetLogicalParent)popup).SetParent(popup.PlacementTarget); + + var window = PreparedWindow(placementTarget); + window.Show(); + popup.Open(); + + // The target's initial placement is (395,295) which is a 10x10 panel centered in a 800x600 window + Assert.Equal(placementTarget.Bounds, new Rect(395D, 295D, 10, 10)); + + if (popup.Host is PopupRoot popupRoot) + { + // Resizing the window to 700x500 must move the popup to (345,255) as this is the new + // location of the placement target + var raised = false; + popupRoot.PositionChanged += (_, args) => + { + Assert.Equal(new PixelPoint(345, 255), args.Point); + raised = true; + }; + + window.PlatformImpl?.Resize(new Size(700D, 500D), PlatformResizeReason.Unspecified); + Assert.True(raised); + } + } + } + + [Fact] + public void Popup_Should_Follow_Popup_Root_Placement_Target() + { + // When the placement target of a popup is another popup (e.g. nested menu items), the child popup must + // follow the parent popup if it moves (due to root window movement or resize) + using (CreateServices()) + { + // The child popup is placed directly over the parent popup for position testing + var parentPopup = new Popup() { Width = 10, Height = 10 }; + var childPopup = new Popup() { + Width = 20, + Height = 20, + PlacementTarget = parentPopup, + PlacementMode = PlacementMode.AnchorAndGravity, + PlacementAnchor = PopupAnchor.TopLeft, + PlacementGravity = PopupGravity.BottomRight + }; + ((ISetLogicalParent)childPopup).SetParent(childPopup.PlacementTarget); + + var window = PreparedWindow(parentPopup); + window.Show(); + parentPopup.Open(); + childPopup.Open(); + + if (childPopup.Host is PopupRoot popupRoot) + { + var raised = false; + popupRoot.PositionChanged += (_, args) => + { + // The parent's initial placement is (395,295) which is a 10x10 popup centered + // in a 800x600 window. When the window is moved, the child's final placement is (405, 305) + // which is the parent's placement moved 10 pixels left and down. + Assert.Equal(new PixelPoint(405, 305), args.Point); + raised = true; + }; + + window.Position = new PixelPoint(10, 10); + Assert.True(raised); + } + } + } + private IDisposable CreateServices() { return UnitTestApplication.Start(TestServices.StyledWindow.With(windowingPlatform: diff --git a/tests/Avalonia.UnitTests/MockWindowingPlatform.cs b/tests/Avalonia.UnitTests/MockWindowingPlatform.cs index eb18030ca8..317a1c50d6 100644 --- a/tests/Avalonia.UnitTests/MockWindowingPlatform.cs +++ b/tests/Avalonia.UnitTests/MockWindowingPlatform.cs @@ -64,6 +64,9 @@ namespace Avalonia.UnitTests windowImpl.Object.Activated?.Invoke(); }); + windowImpl.Setup(x => x.PointToScreen(It.IsAny())) + .Returns((Point p) => PixelPoint.FromPoint(p, 1D) + position); + return windowImpl; }