Merge pull request #4080 from AvaloniaUI/fixes/scrollchanged-behavior

Match LayoutUpdated and ScrollChanged event behavior to WPF/UWP

build/Moq.props

@@ -1,5 +1,5 @@
 <Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <ItemGroup>
-    <PackageReference Include="Moq" Version="4.7.99" />
+    <PackageReference Include="Moq" Version="4.14.1" />
   </ItemGroup>
 </Project>

src/Avalonia.Controls/ScrollViewer.cs

@@ -181,6 +181,9 @@ namespace Avalonia.Controls
         private Size _extent;
         private Vector _offset;
         private Size _viewport;
+        private Size _oldExtent;
+        private Vector _oldOffset;
+        private Size _oldViewport;
         private Size _largeChange;
         private Size _smallChange = new Size(DefaultSmallChange, DefaultSmallChange);
 
@@ -198,6 +201,7 @@ namespace Avalonia.Controls
         /// </summary>
         public ScrollViewer()
         {
+            LayoutUpdated += OnLayoutUpdated;
         }
 
         /// <summary>
@@ -221,11 +225,9 @@ namespace Avalonia.Controls
 
             private set
             {
-                var old = _extent;
-
                 if (SetAndRaise(ExtentProperty, ref _extent, value))
                 {
-                    CalculatedPropertiesChanged(extentDelta: value - old);
+                    CalculatedPropertiesChanged();
                 }
             }
         }
@@ -242,13 +244,11 @@ namespace Avalonia.Controls
 
             set
             {
-                var old = _offset;
-
                 value = ValidateOffset(this, value);
 
                 if (SetAndRaise(OffsetProperty, ref _offset, value))
                 {
-                    CalculatedPropertiesChanged(offsetDelta: value - old);
+                    CalculatedPropertiesChanged();
                 }
             }
         }
@@ -265,11 +265,9 @@ namespace Avalonia.Controls
 
             private set
             {
-                var old = _viewport;
-
                 if (SetAndRaise(ViewportProperty, ref _viewport, value))
                 {
-                    CalculatedPropertiesChanged(viewportDelta: value - old);
+                    CalculatedPropertiesChanged();
                 }
             }
         }
@@ -581,10 +579,7 @@ namespace Avalonia.Controls
             }
         }
 
-        private void CalculatedPropertiesChanged(
+        private void CalculatedPropertiesChanged()
-            Size extentDelta = default,
-            Vector offsetDelta = default,
-            Size viewportDelta = default)
         {
             // Pass old values of 0 here because we don't have the old values at this point,
             // and it shouldn't matter as only the template uses these properies.
@@ -605,20 +600,6 @@ namespace Avalonia.Controls
                 SetAndRaise(SmallChangeProperty, ref _smallChange, new Size(DefaultSmallChange, DefaultSmallChange));
                 SetAndRaise(LargeChangeProperty, ref _largeChange, Viewport);
             }
-
-            if (extentDelta != default || offsetDelta != default || viewportDelta != default)
-            {
-                using var route = BuildEventRoute(ScrollChangedEvent);
-
-                if (route.HasHandlers)
-                {
-                    var e = new ScrollChangedEventArgs(
-                        new Vector(extentDelta.Width, extentDelta.Height),
-                        offsetDelta,
-                        new Vector(viewportDelta.Width, viewportDelta.Height));
-                    route.RaiseEvent(this, e);
-                }
-            }
         }
 
         protected override void OnKeyDown(KeyEventArgs e)
@@ -634,5 +615,38 @@ namespace Avalonia.Controls
                 e.Handled = true;
             }
         }
+
+        /// <summary>
+        /// Called when a change in scrolling state is detected, such as a change in scroll
+        /// position, extent, or viewport size.
+        /// </summary>
+        /// <param name="e">The event args.</param>
+        /// <remarks>
+        /// If you override this method, call `base.OnScrollChanged(ScrollChangedEventArgs)` to
+        /// ensure that this event is raised.
+        /// </remarks>
+        protected virtual void OnScrollChanged(ScrollChangedEventArgs e)
+        {
+            RaiseEvent(e);
+        }
+
+        private void OnLayoutUpdated(object sender, EventArgs e) => RaiseScrollChanged();
+
+        private void RaiseScrollChanged()
+        {
+            var extentDelta = new Vector(Extent.Width - _oldExtent.Width, Extent.Height - _oldExtent.Height);
+            var offsetDelta = Offset - _oldOffset;
+            var viewportDelta = new Vector(Viewport.Width - _oldViewport.Width, Viewport.Height - _oldViewport.Height);
+
+            if (!extentDelta.NearlyEquals(default) || !offsetDelta.NearlyEquals(default) || !viewportDelta.NearlyEquals(default))
+            {
+                var e = new ScrollChangedEventArgs(extentDelta, offsetDelta, viewportDelta);
+                OnScrollChanged(e);
+
+                _oldExtent = Extent;
+                _oldOffset = Offset;
+                _oldViewport = Viewport;
+            }
+        }
     }
 }

src/Avalonia.Layout/ILayoutManager.cs

@@ -1,3 +1,6 @@
+using System;
+
+#nullable enable
 
 namespace Avalonia.Layout
 {
@@ -6,6 +9,11 @@ namespace Avalonia.Layout
     /// </summary>
     public interface ILayoutManager
     {
+        /// <summary>
+        /// Raised when the layout manager completes a layout pass.
+        /// </summary>
+        event EventHandler LayoutUpdated;
+
         /// <summary>
         /// Notifies the layout manager that a control requires a measure.
         /// </summary>

src/Avalonia.Layout/ILayoutable.cs

@@ -1,5 +1,7 @@
 using Avalonia.VisualTree;
 
+#nullable enable
+
 namespace Avalonia.Layout
 {
     /// <summary>

src/Avalonia.Layout/LayoutManager.cs

@@ -3,6 +3,8 @@ using System.Diagnostics;
 using Avalonia.Logging;
 using Avalonia.Threading;
 
+#nullable enable
+
 namespace Avalonia.Layout
 {
     /// <summary>
@@ -21,10 +23,12 @@ namespace Avalonia.Layout
             _executeLayoutPass = ExecuteLayoutPass;
         }
 
+        public event EventHandler? LayoutUpdated;
+
         /// <inheritdoc/>
         public void InvalidateMeasure(ILayoutable control)
         {
-            Contract.Requires<ArgumentNullException>(control != null);
+            control = control ?? throw new ArgumentNullException(nameof(control));
             Dispatcher.UIThread.VerifyAccess();
 
             if (!control.IsAttachedToVisualTree)
@@ -45,7 +49,7 @@ namespace Avalonia.Layout
         /// <inheritdoc/>
         public void InvalidateArrange(ILayoutable control)
         {
-            Contract.Requires<ArgumentNullException>(control != null);
+            control = control ?? throw new ArgumentNullException(nameof(control));
             Dispatcher.UIThread.VerifyAccess();
 
             if (!control.IsAttachedToVisualTree)
@@ -73,7 +77,7 @@ namespace Avalonia.Layout
             {
                 _running = true;
 
-                Stopwatch stopwatch = null;
+                Stopwatch? stopwatch = null;
 
                 const LogEventLevel timingLogLevel = LogEventLevel.Information;
                 bool captureTiming = Logger.IsEnabled(timingLogLevel);
@@ -117,13 +121,14 @@ namespace Avalonia.Layout
 
                 if (captureTiming)
                 {
-                    stopwatch.Stop();
+                    stopwatch!.Stop();
 
                     Logger.TryGet(timingLogLevel)?.Log(LogArea.Layout, this, "Layout pass finished in {Time}", stopwatch.Elapsed);
                 }
             }
 
             _queued = false;
+            LayoutUpdated?.Invoke(this, EventArgs.Empty);
         }
 
         /// <inheritdoc/>

src/Avalonia.Layout/Layoutable.cs

@@ -1,8 +1,9 @@
 using System;
 using Avalonia.Logging;
-using Avalonia.Utilities;
 using Avalonia.VisualTree;
 
+#nullable enable
+
 namespace Avalonia.Layout
 {
     /// <summary>
@@ -131,6 +132,7 @@ namespace Avalonia.Layout
         private bool _measuring;
         private Size? _previousMeasure;
         private Rect? _previousArrange;
+        private EventHandler? _layoutUpdated;
 
         /// <summary>
         /// Initializes static members of the <see cref="Layoutable"/> class.
@@ -153,7 +155,28 @@ namespace Avalonia.Layout
         /// <summary>
         /// Occurs when a layout pass completes for the control.
         /// </summary>
-        public event EventHandler LayoutUpdated;
+        public event EventHandler? LayoutUpdated
+        {
+            add
+            {
+                if (_layoutUpdated is null && VisualRoot is ILayoutRoot r)
+                {
+                    r.LayoutManager.LayoutUpdated += LayoutManagedLayoutUpdated;
+                }
+
+                _layoutUpdated += value;
+            }
+
+            remove
+            {
+                _layoutUpdated -= value;
+
+                if (_layoutUpdated is null && VisualRoot is ILayoutRoot r)
+                {
+                    r.LayoutManager.LayoutUpdated -= LayoutManagedLayoutUpdated;
+                }
+            }
+        }
 
         /// <summary>
         /// Gets or sets the width of the element.
@@ -358,12 +381,9 @@ namespace Avalonia.Layout
                 IsArrangeValid = true;
                 ArrangeCore(rect);
                 _previousArrange = rect;
-
-                LayoutUpdated?.Invoke(this, EventArgs.Empty);
             }
         }
 
-
         /// <summary>
         /// Called by InvalidateMeasure
         /// </summary>
@@ -693,6 +713,26 @@ namespace Avalonia.Layout
             InvalidateMeasure();
         }
 
+        protected override void OnAttachedToVisualTreeCore(VisualTreeAttachmentEventArgs e)
+        {
+            base.OnAttachedToVisualTreeCore(e);
+
+            if (_layoutUpdated is object && e.Root is ILayoutRoot r)
+            {
+                r.LayoutManager.LayoutUpdated += LayoutManagedLayoutUpdated;
+            }
+        }
+
+        protected override void OnDetachedFromVisualTreeCore(VisualTreeAttachmentEventArgs e)
+        {
+            base.OnDetachedFromVisualTreeCore(e);
+
+            if (_layoutUpdated is object && e.Root is ILayoutRoot r)
+            {
+                r.LayoutManager.LayoutUpdated -= LayoutManagedLayoutUpdated;
+            }
+        }
+
         /// <inheritdoc/>
         protected sealed override void OnVisualParentChanged(IVisual oldParent, IVisual newParent)
         {
@@ -701,6 +741,13 @@ namespace Avalonia.Layout
             base.OnVisualParentChanged(oldParent, newParent);
         }
 
+        /// <summary>
+        /// Called when the layout manager raises a LayoutUpdated event.
+        /// </summary>
+        /// <param name="sender">The sender.</param>
+        /// <param name="e">The event args.</param>
+        private void LayoutManagedLayoutUpdated(object sender, EventArgs e) => _layoutUpdated?.Invoke(this, e);
+
         /// <summary>
         /// Tests whether any of a <see cref="Rect"/>'s properties include negative values,
         /// a NaN or Infinity.

tests/Avalonia.Controls.UnitTests/GridTests.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using Avalonia.UnitTests;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -1182,13 +1183,18 @@ namespace Avalonia.Controls.UnitTests
             foreach (var xgrids in grids)
                 scope.Children.Add(xgrids);
 
-            var root = new Grid();
+            var rootGrid = new Grid();
-            root.UseLayoutRounding = false;
+            rootGrid.UseLayoutRounding = false;
-            root.SetValue(Grid.IsSharedSizeScopeProperty, true);
+            rootGrid.SetValue(Grid.IsSharedSizeScopeProperty, true);
-            root.Children.Add(scope);
+            rootGrid.Children.Add(scope);
 
-            root.Measure(new Size(50, 50));
+            var root = new TestRoot(rootGrid)
-            root.Arrange(new Rect(new Point(), new Point(50, 50)));
+            {
+                Width = 50,
+                Height = 50,
+            };
+
+            root.LayoutManager.ExecuteInitialLayoutPass(root);
 
             PrintColumnDefinitions(grids[0]);
             Assert.Equal(5, grids[0].ColumnDefinitions[0].ActualWidth);

tests/Avalonia.Controls.UnitTests/ScrollViewerTests.cs

@@ -4,6 +4,7 @@ using Avalonia.Controls.Presenters;
 using Avalonia.Controls.Primitives;
 using Avalonia.Controls.Templates;
 using Avalonia.Layout;
+using Avalonia.UnitTests;
 using Moq;
 using Xunit;
 
@@ -150,12 +151,15 @@ namespace Avalonia.Controls.UnitTests
         public void Changing_Extent_Should_Raise_ScrollChanged()
         {
             var target = new ScrollViewer();
+            var root = new TestRoot(target);
             var raised = 0;
 
             target.SetValue(ScrollViewer.ExtentProperty, new Size(100, 100));
             target.SetValue(ScrollViewer.ViewportProperty, new Size(50, 50));
             target.Offset = new Vector(10, 10);
 
+            root.LayoutManager.ExecuteInitialLayoutPass(root);
+
             target.ScrollChanged += (s, e) =>
             {
                 Assert.Equal(new Vector(11, 12), e.ExtentDelta);
@@ -166,20 +170,26 @@ namespace Avalonia.Controls.UnitTests
 
             target.SetValue(ScrollViewer.ExtentProperty, new Size(111, 112));
 
-            Assert.Equal(1, raised);
+            Assert.Equal(0, raised);
+
+            root.LayoutManager.ExecuteLayoutPass();
 
+            Assert.Equal(1, raised);
         }
 
         [Fact]
         public void Changing_Offset_Should_Raise_ScrollChanged()
         {
             var target = new ScrollViewer();
+            var root = new TestRoot(target);
             var raised = 0;
 
             target.SetValue(ScrollViewer.ExtentProperty, new Size(100, 100));
             target.SetValue(ScrollViewer.ViewportProperty, new Size(50, 50));
             target.Offset = new Vector(10, 10);
 
+            root.LayoutManager.ExecuteInitialLayoutPass(root);
+
             target.ScrollChanged += (s, e) =>
             {
                 Assert.Equal(default, e.ExtentDelta);
@@ -190,20 +200,26 @@ namespace Avalonia.Controls.UnitTests
 
             target.Offset = new Vector(22, 24);
 
-            Assert.Equal(1, raised);
+            Assert.Equal(0, raised);
 
+            root.LayoutManager.ExecuteLayoutPass();
+
+            Assert.Equal(1, raised);
         }
 
         [Fact]
         public void Changing_Viewport_Should_Raise_ScrollChanged()
         {
             var target = new ScrollViewer();
+            var root = new TestRoot(target);
             var raised = 0;
 
             target.SetValue(ScrollViewer.ExtentProperty, new Size(100, 100));
             target.SetValue(ScrollViewer.ViewportProperty, new Size(50, 50));
             target.Offset = new Vector(10, 10);
 
+            root.LayoutManager.ExecuteInitialLayoutPass(root);
+
             target.ScrollChanged += (s, e) =>
             {
                 Assert.Equal(default, e.ExtentDelta);
@@ -214,8 +230,11 @@ namespace Avalonia.Controls.UnitTests
 
             target.SetValue(ScrollViewer.ViewportProperty, new Size(56, 58));
 
-            Assert.Equal(1, raised);
+            Assert.Equal(0, raised);
+
+            root.LayoutManager.ExecuteLayoutPass();
 
+            Assert.Equal(1, raised);
         }
 
         private Control CreateTemplate(ScrollViewer control, INameScope scope)

tests/Avalonia.Layout.UnitTests/LayoutableTests.cs

@@ -203,6 +203,125 @@ namespace Avalonia.Layout.UnitTests
             Assert.Equal(new Rect(expectedX, 0, childWidth, 100), target.Bounds);
         }
 
+        [Fact]
+        public void LayoutUpdated_Is_Called_At_End_Of_Layout_Pass()
+        {
+            Border border1;
+            Border border2;
+            var layoutManager = new LayoutManager();
+            var root = new TestRoot
+            {
+                Child = border1 = new Border
+                {
+                    Child = border2 = new Border(),
+                },
+                LayoutManager = layoutManager,
+            };
+            var raised = 0;
+
+            void ValidateBounds(object sender, EventArgs e)
+            {
+                Assert.Equal(new Rect(0, 0, 100, 100), border1.Bounds);
+                Assert.Equal(new Rect(0, 0, 100, 100), border2.Bounds);
+                ++raised;
+            }
+
+            root.LayoutUpdated += ValidateBounds;
+            border1.LayoutUpdated += ValidateBounds;
+            border2.LayoutUpdated += ValidateBounds;
+
+            root.Measure(new Size(100, 100));
+            root.Arrange(new Rect(0, 0, 100, 100));
+            
+            layoutManager.ExecuteLayoutPass();
+
+            Assert.Equal(3, raised);
+            Assert.Equal(new Rect(0, 0, 100, 100), border1.Bounds);
+            Assert.Equal(new Rect(0, 0, 100, 100), border2.Bounds);
+        }
+
+        [Fact]
+        public void LayoutUpdated_Subscribes_To_LayoutManager()
+        {
+            Border target;
+            var layoutManager = new Mock<ILayoutManager>();
+            layoutManager.SetupAdd(m => m.LayoutUpdated += (sender, args) => { });
+
+            var root = new TestRoot
+            {
+                Child = new Border
+                {
+                    Child = target = new Border(),
+                },
+                LayoutManager = layoutManager.Object,
+            };
+
+            void Handler(object sender, EventArgs e) {}
+
+            layoutManager.Invocations.Clear();
+            target.LayoutUpdated += Handler;
+
+            layoutManager.VerifyAdd(
+                x => x.LayoutUpdated += It.IsAny<EventHandler>(),
+                Times.Once);
+
+            layoutManager.Invocations.Clear();
+            target.LayoutUpdated -= Handler;
+
+            layoutManager.VerifyRemove(
+                x => x.LayoutUpdated -= It.IsAny<EventHandler>(),
+                Times.Once);
+        }
+
+        [Fact]
+        public void LayoutManager_LayoutUpdated_Is_Subscribed_When_Attached_To_Tree()
+        {
+            Border border1;
+            var layoutManager = new Mock<ILayoutManager>();
+            layoutManager.SetupAdd(m => m.LayoutUpdated += (sender, args) => { });
+
+            var root = new TestRoot
+            {
+                Child = border1 = new Border(),
+                LayoutManager = layoutManager.Object,
+            };
+
+            var border2 = new Border();
+            border2.LayoutUpdated += (s, e) => { };
+
+            layoutManager.Invocations.Clear();
+            border1.Child = border2;
+
+            layoutManager.VerifyAdd(
+                x => x.LayoutUpdated += It.IsAny<EventHandler>(),
+                Times.Once);
+        }
+
+        [Fact]
+        public void LayoutManager_LayoutUpdated_Is_Unsubscribed_When_Detached_From_Tree()
+        {
+            Border border1;
+            var layoutManager = new Mock<ILayoutManager>();
+            layoutManager.SetupAdd(m => m.LayoutUpdated += (sender, args) => { });
+
+            var root = new TestRoot
+            {
+                Child = border1 = new Border(),
+                LayoutManager = layoutManager.Object,
+            };
+
+            var border2 = new Border();
+            border2.LayoutUpdated += (s, e) => { };
+            border1.Child = border2;
+
+            layoutManager.Invocations.Clear();
+            border1.Child = null;
+
+            layoutManager.VerifyRemove(
+                x => x.LayoutUpdated -= It.IsAny<EventHandler>(),
+                Times.Once);
+        }
+
         private class TestLayoutable : Layoutable
         {
             public Size ArrangeSize { get; private set; }