Merge pull request #7071 from simonhaines/fixes/popup-positioning

Fix issues with positioning of non-overlay popups

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<IWindowImpl, Action>(window.PlatformImpl, WindowLostFocus,
-                        (x, handler) => x.LostFocus += handler,
+                    (x, handler) => x.LostFocus += handler,
-                        (x, handler) => x.LostFocus -= handler).DisposeWith(handlerCleanup);
+                    (x, handler) => x.LostFocus -= handler).DisposeWith(handlerCleanup);
+
+                SubscribeToEventHandler<IWindowImpl, Action<PixelPoint>>(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<Layoutable, EventHandler>(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<PopupRoot, EventHandler<PixelPointEventArgs>>(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, EventHandler<EventArgs>>(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);

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:

tests/Avalonia.UnitTests/MockWindowingPlatform.cs

@@ -64,6 +64,9 @@ namespace Avalonia.UnitTests
                 windowImpl.Object.Activated?.Invoke();
             });
 
+            windowImpl.Setup(x => x.PointToScreen(It.IsAny<Point>()))
+                .Returns((Point p) => PixelPoint.FromPoint(p, 1D) + position);
+
             return windowImpl;
         }