Bring control into view only if control isn't properly visible in viewport (#18359)

* bring control into view only if control isn't currently in viewport

* fix margin add comments

* add more bring to view tests

src/Avalonia.Controls/Presenters/ScrollContentPresenter.cs

@@ -257,28 +257,12 @@ namespace Avalonia.Controls.Presenters
                 return false;
             }
 
-            var rect = targetRect.TransformToAABB(transform.Value);
+            var rectangle = targetRect.TransformToAABB(transform.Value).Deflate(new Thickness(Child.Margin.Left, Child.Margin.Top, 0, 0));
-            var offset = Offset;
+            Rect viewport = new Rect(Offset.X, Offset.Y, Viewport.Width, Viewport.Height);
 
-            if (rect.Bottom > offset.Y + Viewport.Height)
+            double minX = ComputeScrollOffsetWithMinimalScroll(viewport.Left, viewport.Right, rectangle.Left, rectangle.Right);
-            {
+            double minY = ComputeScrollOffsetWithMinimalScroll(viewport.Top, viewport.Bottom, rectangle.Top, rectangle.Bottom);
-                offset = offset.WithY((rect.Bottom - Viewport.Height) + Child.Margin.Top);
+            var offset = new Vector(minX, minY);
-            }
-
-            if (rect.Y < offset.Y)
-            {
-                offset = offset.WithY(rect.Y);
-            }
-
-            if (rect.Right > offset.X + Viewport.Width)
-            {
-                offset = offset.WithX((rect.Right - Viewport.Width) + Child.Margin.Left);
-            }
-
-            if (rect.X < offset.X)
-            {
-                offset = offset.WithX(rect.X);
-            }
 
             if (Offset.NearlyEquals(offset))
             {
@@ -293,6 +277,47 @@ namespace Avalonia.Controls.Presenters
             return !Offset.NearlyEquals(oldOffset);
         }
 
+        /// <summary>
+        /// Computes the closest offset to ensure most of the child is visible in the viewport along an axis.
+        /// </summary>
+        /// <param name="viewportStart">The left or top of the viewport</param>
+        /// <param name="viewportEnd">The right or bottom of the viewport</param>
+        /// <param name="childStart">The left or top of the child</param>
+        /// <param name="childEnd">The right or bottom of the child</param>
+        /// <returns></returns>
+        internal static double ComputeScrollOffsetWithMinimalScroll(
+            double viewportStart,
+            double viewportEnd,
+            double childStart,
+            double childEnd)
+        {
+            // If child is at least partially above viewport, i.e. top of child is above viewport top and bottom of child is above viewport bottom.
+            bool isChildAbove = MathUtilities.LessThan(childStart, viewportStart) && MathUtilities.LessThan(childEnd, viewportEnd);
+
+            // If child is at least partially below viewport, i.e. top of child is below viewport top and bottom of child is below viewport bottom.
+            bool isChildBelow = MathUtilities.GreaterThan(childEnd, viewportEnd) && MathUtilities.GreaterThan(childStart, viewportStart);
+            bool isChildLarger = (childEnd - childStart) > (viewportEnd - viewportStart);
+
+            // Value if no updates is needed. The child is fully visible in the viewport, or the viewport is completely within the child's bounds
+            var res = viewportStart;
+
+            // The child is above the viewport and is smaller than the viewport, or if the child's top is below the viewport top
+            // and is larger than the viewport, we align the child top to the top of the viewport
+            if ((isChildAbove && !isChildLarger)
+               || (isChildBelow && isChildLarger))
+            {
+                res = childStart;
+            }
+            // The child is above the viewport and is larger than the viewport, or if the child's smaller but is below the viewport,
+            // we align the child's bottom to the bottom of the viewport
+            else if (isChildAbove || isChildBelow)
+            {
+                res = (childEnd - (viewportEnd - viewportStart));
+            }
+
+            return res;
+        }
+
         protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
         {
             base.OnAttachedToVisualTree(e);

tests/Avalonia.Controls.UnitTests/Presenters/ScrollContentPresenterTests.cs

@@ -5,6 +5,7 @@ using Avalonia.Controls.Presenters;
 using Avalonia.Layout;
 using Avalonia.UnitTests;
 using Xunit;
+using Xunit.Sdk;
 
 namespace Avalonia.Controls.UnitTests.Presenters
 {
@@ -399,6 +400,198 @@ namespace Avalonia.Controls.UnitTests.Presenters
             Assert.Equal(new Vector(150, 150), target.Offset);
         }
 
+        [Fact]
+        public void BringDescendantIntoView_Should_Not_Move_Child_If_Completely_In_View()
+        {
+            Border border = new Border
+            {
+                Width = 100,
+                Height = 20
+            };
+            var content = new StackPanel()
+            {
+                Orientation = Orientation.Vertical,
+                Width = 100,
+            };
+
+            for(int i = 0; i < 100; i++)
+            {
+                // border position will be (0,60)
+                var child = i == 3 ? border : new Border
+                {
+                    Width = 100,
+                    Height = 20,
+                };
+                content.Children.Add(child);
+            }
+            var target = new ScrollContentPresenter
+            {
+                CanHorizontallyScroll = true,
+                CanVerticallyScroll = true,
+                Width = 200,
+                Height = 100,
+                Content = new Decorator
+                {
+                    Child = content
+                }
+            };
+
+            target.UpdateChild();
+            target.Measure(Size.Infinity);
+            target.Arrange(new Rect(0, 0, 100, 100));
+            target.BringDescendantIntoView(border, new Rect(border.Bounds.Size));
+
+            Assert.Equal(new Vector(0, 0), target.Offset);
+        }
+
+        [Fact]
+        public void BringDescendantIntoView_Should_Move_Child_At_Least_Partially_Above_Viewport()
+        {
+            Border border = new Border
+            {
+                Width = 100,
+                Height = 20
+            };
+            var content = new StackPanel()
+            {
+                Orientation = Orientation.Vertical,
+                Width = 100,
+            };
+
+            for(int i = 0; i < 100; i++)
+            {
+                // border position will be (0,60)
+                var child = i == 3 ? border : new Border
+                {
+                    Width = 100,
+                    Height = 20,
+                };
+                content.Children.Add(child);
+            }
+            var target = new ScrollContentPresenter
+            {
+                CanHorizontallyScroll = true,
+                CanVerticallyScroll = true,
+                Width = 200,
+                Height = 100,
+                Content = new Decorator
+                {
+                    Child = content
+                }
+            };
+
+            target.UpdateChild();
+            target.Measure(Size.Infinity);
+            target.Arrange(new Rect(0, 0, 100, 100));
+            // move border to above the view port
+            target.Offset = new Vector(0, 90);
+            target.BringDescendantIntoView(border, new Rect(border.Bounds.Size));
+
+            Assert.Equal(new Vector(0, 60), target.Offset);
+
+            // move border to partially above the view port
+            target.Offset = new Vector(0, 70);
+            target.BringDescendantIntoView(border, new Rect(border.Bounds.Size));
+
+            Assert.Equal(new Vector(0, 60), target.Offset);
+        }
+
+        [Fact]
+        public void BringDescendantIntoView_Should_Not_Move_Child_If_Completely_Covers_Viewport()
+        {
+            Border border = new Border
+            {
+                Width = 100,
+                Height = 200
+            };
+            var content = new StackPanel()
+            {
+                Orientation = Orientation.Vertical,
+                Width = 100,
+            };
+
+            for (int i = 0; i < 100; i++)
+            {
+                // border position will be (0,60)
+                var child = i == 3 ? border : new Border
+                {
+                    Width = 100,
+                    Height = 20,
+                };
+                content.Children.Add(child);
+            }
+            var target = new ScrollContentPresenter
+            {
+                CanHorizontallyScroll = true,
+                CanVerticallyScroll = true,
+                Width = 200,
+                Height = 100,
+                Content = new Decorator
+                {
+                    Child = content
+                }
+            };
+
+            target.UpdateChild();
+            target.Measure(Size.Infinity);
+            target.Arrange(new Rect(0, 0, 100, 100));
+            // move border such that it's partially above viewport and partially below viewport
+            target.Offset = new Vector(0, 90);
+            target.BringDescendantIntoView(border, new Rect(border.Bounds.Size));
+
+            Assert.Equal(new Vector(0, 90), target.Offset);
+        }
+
+        [Fact]
+        public void BringDescendantIntoView_Should_Move_Child_At_Least_Partially_Below_Viewport()
+        {
+            Border border = new Border
+            {
+                Width = 100,
+                Height = 20
+            };
+            var content = new StackPanel()
+            {
+                Orientation = Orientation.Vertical,
+                Width = 100,
+            };
+
+            for (int i = 0; i < 100; i++)
+            {
+                // border position will be (0,180)
+                var child = i == 9 ? border : new Border
+                {
+                    Width = 100,
+                    Height = 20,
+                };
+                content.Children.Add(child);
+            }
+            var target = new ScrollContentPresenter
+            {
+                CanHorizontallyScroll = true,
+                CanVerticallyScroll = true,
+                Width = 200,
+                Height = 100,
+                Content = new Decorator
+                {
+                    Child = content
+                }
+            };
+
+            target.UpdateChild();
+            target.Measure(Size.Infinity);
+            target.Arrange(new Rect(0, 0, 100, 100));
+
+            // border is at (0, 180) and below the viewport
+            target.BringDescendantIntoView(border, new Rect(border.Bounds.Size));
+
+            Assert.Equal(new Vector(0, 100), target.Offset);
+
+            // move border to partially below the view port
+            target.Offset = new Vector(0, 90);
+            target.BringDescendantIntoView(border, new Rect(border.Bounds.Size));
+        }
+
         [Fact]
         public void Nested_Presenters_Should_Scroll_Outer_When_Content_Exceeds_Viewport()
         {