Scroll a node's new children into view when it is expanded

The WPF tree did this, and the Avalonia port kept the routine but never
wired it up: the row template's expander binds IsExpanded straight to the
node, so no expansion reached the control and HandleExpanding sat with no
callers. Expanding a row near the bottom of the pane left its children
off screen.

The rule copies the native Windows tree control: scroll far enough to
show the new children, but stop at the expanded node so it never leaves
the viewport, and do not move at all when the children already fit. The
reveal now hangs off user gestures only -- the expander's Click and the
keyboard cases -- because the paths that expand nodes programmatically
position the viewport themselves afterwards, which is what the removed
doNotScrollOnExpanding flag used to arrange.

Assisted-by: Claude:claude-opus-5:Claude Code

ILSpy.Tests/Controls/SharpTreeViewTests.cs

@@ -18,12 +18,15 @@
 
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
 
 using Avalonia.Controls;
+using Avalonia.Controls.Primitives;
 using Avalonia.Headless;
 using Avalonia.Headless.NUnit;
 using Avalonia.Input;
 using Avalonia.Threading;
+using Avalonia.VisualTree;
 
 using AwesomeAssertions;
 
@@ -290,6 +293,107 @@ public class SharpTreeViewTests
 		RenderedRows(tree).Should().Equal("A", "C", "B", "B1");
 	}
 
+	/// <summary>Builds <paramref name="rootCount"/> top-level rows in a viewport too short to show
+	/// them all, with one expandable node, so an expansion has somewhere to scroll.</summary>
+	static (Window window, SharpTreeView tree, ScrollViewer scrollViewer, TestNode[] nodes) ShortViewport(
+		int rootCount, int expandableIndex, int childCount)
+	{
+		var nodes = Enumerable.Range(0, rootCount)
+			.Select(i => i == expandableIndex
+				? new TestNode($"N{i}", Enumerable.Range(0, childCount)
+					.Select(c => new TestNode($"N{i}.{c}")).ToArray())
+				: new TestNode($"N{i}"))
+			.ToArray();
+		var root = new TestNode("root", nodes);
+		var tree = new SharpTreeView { ShowRoot = false, Root = root };
+		var window = new Window { Content = tree, Width = 300, Height = 180 };
+		window.Show();
+		Dispatcher.UIThread.RunJobs();
+		return (window, tree, tree.GetVisualDescendants().OfType<ScrollViewer>().First(), nodes);
+	}
+
+	/// <summary>Expands a node the way a user does, with Right on its focused row.</summary>
+	static void PressRightOn(Window window, SharpTreeView tree, SharpTreeNode node)
+	{
+		tree.SelectedItem = node;
+		Dispatcher.UIThread.RunJobs();
+		tree.ContainerFromItem(node)?.Focus();
+		Dispatcher.UIThread.RunJobs();
+		window.KeyPress(Key.Right, RawInputModifiers.None, PhysicalKey.ArrowRight, null);
+		Dispatcher.UIThread.RunJobs();
+	}
+
+	[AvaloniaTest]
+	public void Expanding_A_Node_Scrolls_Children_That_Do_Not_Fit_Into_View()
+	{
+		var (window, tree, scrollViewer, nodes) = ShortViewport(rootCount: 15, expandableIndex: 5, childCount: 5);
+		var parent = nodes[5];
+		scrollViewer.Offset.Y.Should().Be(0, "nothing has moved the viewport yet");
+
+		PressRightOn(window, tree, parent);
+
+		tree.IsNodeFullyVisible(parent.Children[^1])
+			.Should().BeTrue("the expansion reveals children below the viewport, so the view scrolls to show them");
+		tree.IsNodeFullyVisible(parent)
+			.Should().BeTrue("the scroll is bounded by the expanded node: it never leaves the viewport");
+	}
+
+	[AvaloniaTest]
+	public void Expanding_A_Node_Whose_Children_Already_Fit_Leaves_The_Viewport_Alone()
+	{
+		var (window, tree, scrollViewer, nodes) = ShortViewport(rootCount: 15, expandableIndex: 1, childCount: 2);
+		var parent = nodes[1];
+
+		PressRightOn(window, tree, parent);
+
+		scrollViewer.Offset.Y.Should().Be(0, "the children fit below the node, so there is nothing to scroll to");
+		tree.IsNodeFullyVisible(parent.Children[^1]).Should().BeTrue();
+	}
+
+	[AvaloniaTest]
+	public void Expanding_A_Node_With_More_Children_Than_Fit_Keeps_The_Node_Visible()
+	{
+		var (window, tree, _, nodes) = ShortViewport(rootCount: 15, expandableIndex: 5, childCount: 30);
+		var parent = nodes[5];
+
+		PressRightOn(window, tree, parent);
+
+		tree.IsNodeFullyVisible(parent)
+			.Should().BeTrue("showing every child would push the node off the top, so the scroll stops at the node");
+		tree.IsNodeFullyVisible(parent.Children[0])
+			.Should().BeTrue("as many children as fit are shown below it");
+	}
+
+	[AvaloniaTest]
+	public async Task Clicking_The_Expander_Scrolls_The_Children_Into_View()
+	{
+		// The mouse path never passes through SharpTreeView: the row template's toggle writes
+		// IsExpanded straight to the node, so the reveal hangs off the toggle's Click.
+		var (window, tree, _, nodes) = ShortViewport(rootCount: 15, expandableIndex: 5, childCount: 5);
+		var parent = nodes[5];
+
+		await window.ClickAsync(() => tree.ContainerFromItem(parent)?.GetVisualDescendants()
+			.OfType<ToggleButton>().FirstOrDefault(b => b.Name == "PART_Expander"));
+		Dispatcher.UIThread.RunJobs();
+
+		parent.IsExpanded.Should().BeTrue("precondition: the click toggled the node open");
+		tree.IsNodeFullyVisible(parent.Children[^1])
+			.Should().BeTrue("a click on the expander reveals the children, just as the keyboard does");
+	}
+
+	[AvaloniaTest]
+	public void Expanding_A_Node_In_Code_Does_Not_Move_The_Viewport()
+	{
+		// Revealing a node expands its ancestors first (ScrollIntoNodeView, TreeSelectionBinder)
+		// and positions the viewport itself afterwards; a scroll per ancestor would fight that.
+		var (_, _, scrollViewer, nodes) = ShortViewport(rootCount: 15, expandableIndex: 5, childCount: 5);
+
+		nodes[5].IsExpanded = true;
+		Dispatcher.UIThread.RunJobs();
+
+		scrollViewer.Offset.Y.Should().Be(0, "only a user gesture reveals the children");
+	}
+
 	static List<string> RenderedRows(SharpTreeView tree)
 	{
 		var rows = new List<string>();

ILSpy/Controls/TreeView/SharpTreeView.cs

@@ -59,7 +59,6 @@ namespace ICSharpCode.ILSpy.Controls.TreeView
 			AvaloniaProperty.Register<SharpTreeView, bool>(nameof(ShowLines), defaultValue: true);
 
 		TreeFlattener? flattener;
-		bool doNotScrollOnExpanding;
 		string searchBuffer = string.Empty;
 		DispatcherTimer? searchResetTimer;
 
@@ -92,6 +91,12 @@ namespace ICSharpCode.ILSpy.Controls.TreeView
 			AddHandler(DragDrop.DragOverEvent, OnDragOver);
 			AddHandler(DragDrop.DropEvent, OnDrop);
 			AddHandler(DragDrop.DragLeaveEvent, (_, _) => HideInsertMarker());
+			// The row template's expander writes IsExpanded straight to the node, so an expansion
+			// made with the mouse never passes through this control; its Click is what identifies
+			// one. Only a gesture scrolls: code that expands nodes to reveal a selection (see
+			// ScrollIntoNodeView) or to open every match of a filter positions the viewport itself,
+			// and a scroll per expanded node would fight it.
+			AddHandler(Button.ClickEvent, OnExpanderClick, RoutingStrategies.Bubble, handledEventsToo: true);
 		}
 
 		public SharpTreeNode? Root {
@@ -184,32 +189,47 @@ namespace ICSharpCode.ILSpy.Controls.TreeView
 			node.ActivateItem(args);
 			if (!e.Handled && node.ShowExpander)
 			{
-				node.IsExpanded = !node.IsExpanded;
+				SetExpanded(node, !node.IsExpanded);
 				e.Handled = true;
 			}
 		}
 
+		void OnExpanderClick(object? sender, RoutedEventArgs e)
+		{
+			if (e.Source is ToggleButton { Name: "PART_Expander" } expander
+				&& expander.DataContext is SharpTreeNode { IsExpanded: true } node)
+			{
+				HandleExpanding(node);
+			}
+		}
+
+		/// <summary>Expands or collapses <paramref name="node"/> as a user gesture, so an expansion
+		/// reveals its children the way <see cref="HandleExpanding"/> describes.</summary>
+		void SetExpanded(SharpTreeNode node, bool expanded)
+		{
+			if (node.IsExpanded == expanded)
+				return;
+			node.IsExpanded = expanded;
+			if (expanded)
+				HandleExpanding(node);
+		}
+
 		/// <summary>
-		/// Called when a visible node expands so its newly shown children are scrolled into view
-		/// (without scrolling the node itself off the top).
+		/// Scrolls the rows a just-expanded node revealed into view, the way the native Windows
+		/// tree control does: far enough to show the new children, but never so far that the
+		/// expanded node itself leaves the viewport. Both steps only move the viewport when their
+		/// row lies outside it, so expanding a node whose children already fit below it does not
+		/// scroll at all.
 		/// </summary>
-		internal void HandleExpanding(SharpTreeNode node)
+		void HandleExpanding(SharpTreeNode node)
 		{
-			if (doNotScrollOnExpanding)
-				return;
 			SharpTreeNode lastVisibleChild = node;
-			while (true)
-			{
-				var child = lastVisibleChild.Children.LastOrDefault(c => c.IsVisible);
-				if (child == null)
-					break;
+			while (lastVisibleChild.Children.LastOrDefault(c => c.IsVisible) is { } child)
 				lastVisibleChild = child;
-			}
-			if (lastVisibleChild != node)
-			{
-				ScrollRowIntoView(lastVisibleChild, centre: false);
-				Dispatcher.UIThread.Post(() => ScrollRowIntoView(node, centre: false), DispatcherPriority.Loaded);
-			}
+			if (lastVisibleChild == node)
+				return;
+			ScrollRowIntoView(lastVisibleChild, centre: false);
+			ScrollRowIntoView(node, centre: false);
 		}
 
 		/// <summary>Scrolls the node into view (unless <paramref name="scroll"/> is false) and gives it
@@ -252,10 +272,8 @@ namespace ICSharpCode.ILSpy.Controls.TreeView
 		public void ScrollIntoNodeView(SharpTreeNode node)
 		{
 			ArgumentNullException.ThrowIfNull(node);
-			doNotScrollOnExpanding = true;
 			foreach (var ancestor in node.Ancestors())
 				ancestor.IsExpanded = true;
-			doNotScrollOnExpanding = false;
 			CenterNodeInView(node);
 		}
 
@@ -403,7 +421,7 @@ namespace ICSharpCode.ILSpy.Controls.TreeView
 				{
 					case Key.Left:
 						if (node.IsExpanded)
-							node.IsExpanded = false;
+							SetExpanded(node, false);
 						else if (node.Parent != null && !node.Parent.IsRoot)
 							SelectAndFocus(node.Parent);
 						else
@@ -412,7 +430,7 @@ namespace ICSharpCode.ILSpy.Controls.TreeView
 						break;
 					case Key.Right:
 						if (!node.IsExpanded && node.ShowExpander)
-							node.IsExpanded = true;
+							SetExpanded(node, true);
 						else if (node.Children.Count > 0)
 							SelectAndFocus(node.Children.First(c => c.IsVisible));
 						else
@@ -420,16 +438,18 @@ namespace ICSharpCode.ILSpy.Controls.TreeView
 						e.Handled = true;
 						break;
 					case Key.Add:
-						node.IsExpanded = true;
+						SetExpanded(node, true);
 						e.Handled = true;
 						break;
 					case Key.Subtract:
-						node.IsExpanded = false;
+						SetExpanded(node, false);
 						e.Handled = true;
 						break;
 					case Key.Multiply:
 						node.IsExpanded = true;
 						ExpandRecursively(node);
+						// The whole subtree is open now, so this reveals as much of it as fits.
+						HandleExpanding(node);
 						e.Handled = true;
 						break;
 					case Key.Enter: