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: