From 1f448dbb29995a215b54b64e6fc9e08e53e14410 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Mon, 14 Sep 2026 07:51:26 +0200 Subject: [PATCH] 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 | 104 +++++++++++++++++++++ ILSpy/Controls/TreeView/SharpTreeView.cs | 68 +++++++++----- 2 files changed, 148 insertions(+), 24 deletions(-) diff --git a/ILSpy.Tests/Controls/SharpTreeViewTests.cs b/ILSpy.Tests/Controls/SharpTreeViewTests.cs index a8bcee715..16d0b34b8 100644 --- a/ILSpy.Tests/Controls/SharpTreeViewTests.cs +++ b/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"); } + /// Builds top-level rows in a viewport too short to show + /// them all, with one expandable node, so an expansion has somewhere to scroll. + 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().First(), nodes); + } + + /// Expands a node the way a user does, with Right on its focused row. + 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().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 RenderedRows(SharpTreeView tree) { var rows = new List(); diff --git a/ILSpy/Controls/TreeView/SharpTreeView.cs b/ILSpy/Controls/TreeView/SharpTreeView.cs index 2e2b0bcbe..e8b827ef8 100644 --- a/ILSpy/Controls/TreeView/SharpTreeView.cs +++ b/ILSpy/Controls/TreeView/SharpTreeView.cs @@ -59,7 +59,6 @@ namespace ICSharpCode.ILSpy.Controls.TreeView AvaloniaProperty.Register(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); + } + } + + /// Expands or collapses as a user gesture, so an expansion + /// reveals its children the way describes. + void SetExpanded(SharpTreeNode node, bool expanded) + { + if (node.IsExpanded == expanded) + return; + node.IsExpanded = expanded; + if (expanded) + HandleExpanding(node); + } + /// - /// 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. /// - 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); } /// Scrolls the node into view (unless 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: