Skip to content

Commit 1f448db

Browse files
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
1 parent c9f9008 commit 1f448db

2 files changed

Lines changed: 148 additions & 24 deletions

File tree

ILSpy.Tests/Controls/SharpTreeViewTests.cs

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,15 @@
1818

1919
using System.Collections.Generic;
2020
using System.Linq;
21+
using System.Threading.Tasks;
2122

2223
using Avalonia.Controls;
24+
using Avalonia.Controls.Primitives;
2325
using Avalonia.Headless;
2426
using Avalonia.Headless.NUnit;
2527
using Avalonia.Input;
2628
using Avalonia.Threading;
29+
using Avalonia.VisualTree;
2730

2831
using AwesomeAssertions;
2932

@@ -290,6 +293,107 @@ public void Moving_An_Expanded_Node_Reorders_The_Rendered_Rows()
290293
RenderedRows(tree).Should().Equal("A", "C", "B", "B1");
291294
}
292295

296+
/// <summary>Builds <paramref name="rootCount"/> top-level rows in a viewport too short to show
297+
/// them all, with one expandable node, so an expansion has somewhere to scroll.</summary>
298+
static (Window window, SharpTreeView tree, ScrollViewer scrollViewer, TestNode[] nodes) ShortViewport(
299+
int rootCount, int expandableIndex, int childCount)
300+
{
301+
var nodes = Enumerable.Range(0, rootCount)
302+
.Select(i => i == expandableIndex
303+
? new TestNode($"N{i}", Enumerable.Range(0, childCount)
304+
.Select(c => new TestNode($"N{i}.{c}")).ToArray())
305+
: new TestNode($"N{i}"))
306+
.ToArray();
307+
var root = new TestNode("root", nodes);
308+
var tree = new SharpTreeView { ShowRoot = false, Root = root };
309+
var window = new Window { Content = tree, Width = 300, Height = 180 };
310+
window.Show();
311+
Dispatcher.UIThread.RunJobs();
312+
return (window, tree, tree.GetVisualDescendants().OfType<ScrollViewer>().First(), nodes);
313+
}
314+
315+
/// <summary>Expands a node the way a user does, with Right on its focused row.</summary>
316+
static void PressRightOn(Window window, SharpTreeView tree, SharpTreeNode node)
317+
{
318+
tree.SelectedItem = node;
319+
Dispatcher.UIThread.RunJobs();
320+
tree.ContainerFromItem(node)?.Focus();
321+
Dispatcher.UIThread.RunJobs();
322+
window.KeyPress(Key.Right, RawInputModifiers.None, PhysicalKey.ArrowRight, null);
323+
Dispatcher.UIThread.RunJobs();
324+
}
325+
326+
[AvaloniaTest]
327+
public void Expanding_A_Node_Scrolls_Children_That_Do_Not_Fit_Into_View()
328+
{
329+
var (window, tree, scrollViewer, nodes) = ShortViewport(rootCount: 15, expandableIndex: 5, childCount: 5);
330+
var parent = nodes[5];
331+
scrollViewer.Offset.Y.Should().Be(0, "nothing has moved the viewport yet");
332+
333+
PressRightOn(window, tree, parent);
334+
335+
tree.IsNodeFullyVisible(parent.Children[^1])
336+
.Should().BeTrue("the expansion reveals children below the viewport, so the view scrolls to show them");
337+
tree.IsNodeFullyVisible(parent)
338+
.Should().BeTrue("the scroll is bounded by the expanded node: it never leaves the viewport");
339+
}
340+
341+
[AvaloniaTest]
342+
public void Expanding_A_Node_Whose_Children_Already_Fit_Leaves_The_Viewport_Alone()
343+
{
344+
var (window, tree, scrollViewer, nodes) = ShortViewport(rootCount: 15, expandableIndex: 1, childCount: 2);
345+
var parent = nodes[1];
346+
347+
PressRightOn(window, tree, parent);
348+
349+
scrollViewer.Offset.Y.Should().Be(0, "the children fit below the node, so there is nothing to scroll to");
350+
tree.IsNodeFullyVisible(parent.Children[^1]).Should().BeTrue();
351+
}
352+
353+
[AvaloniaTest]
354+
public void Expanding_A_Node_With_More_Children_Than_Fit_Keeps_The_Node_Visible()
355+
{
356+
var (window, tree, _, nodes) = ShortViewport(rootCount: 15, expandableIndex: 5, childCount: 30);
357+
var parent = nodes[5];
358+
359+
PressRightOn(window, tree, parent);
360+
361+
tree.IsNodeFullyVisible(parent)
362+
.Should().BeTrue("showing every child would push the node off the top, so the scroll stops at the node");
363+
tree.IsNodeFullyVisible(parent.Children[0])
364+
.Should().BeTrue("as many children as fit are shown below it");
365+
}
366+
367+
[AvaloniaTest]
368+
public async Task Clicking_The_Expander_Scrolls_The_Children_Into_View()
369+
{
370+
// The mouse path never passes through SharpTreeView: the row template's toggle writes
371+
// IsExpanded straight to the node, so the reveal hangs off the toggle's Click.
372+
var (window, tree, _, nodes) = ShortViewport(rootCount: 15, expandableIndex: 5, childCount: 5);
373+
var parent = nodes[5];
374+
375+
await window.ClickAsync(() => tree.ContainerFromItem(parent)?.GetVisualDescendants()
376+
.OfType<ToggleButton>().FirstOrDefault(b => b.Name == "PART_Expander"));
377+
Dispatcher.UIThread.RunJobs();
378+
379+
parent.IsExpanded.Should().BeTrue("precondition: the click toggled the node open");
380+
tree.IsNodeFullyVisible(parent.Children[^1])
381+
.Should().BeTrue("a click on the expander reveals the children, just as the keyboard does");
382+
}
383+
384+
[AvaloniaTest]
385+
public void Expanding_A_Node_In_Code_Does_Not_Move_The_Viewport()
386+
{
387+
// Revealing a node expands its ancestors first (ScrollIntoNodeView, TreeSelectionBinder)
388+
// and positions the viewport itself afterwards; a scroll per ancestor would fight that.
389+
var (_, _, scrollViewer, nodes) = ShortViewport(rootCount: 15, expandableIndex: 5, childCount: 5);
390+
391+
nodes[5].IsExpanded = true;
392+
Dispatcher.UIThread.RunJobs();
393+
394+
scrollViewer.Offset.Y.Should().Be(0, "only a user gesture reveals the children");
395+
}
396+
293397
static List<string> RenderedRows(SharpTreeView tree)
294398
{
295399
var rows = new List<string>();

ILSpy/Controls/TreeView/SharpTreeView.cs

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ public class SharpTreeView : ListBox
5959
AvaloniaProperty.Register<SharpTreeView, bool>(nameof(ShowLines), defaultValue: true);
6060

6161
TreeFlattener? flattener;
62-
bool doNotScrollOnExpanding;
6362
string searchBuffer = string.Empty;
6463
DispatcherTimer? searchResetTimer;
6564

@@ -92,6 +91,12 @@ public SharpTreeView()
9291
AddHandler(DragDrop.DragOverEvent, OnDragOver);
9392
AddHandler(DragDrop.DropEvent, OnDrop);
9493
AddHandler(DragDrop.DragLeaveEvent, (_, _) => HideInsertMarker());
94+
// The row template's expander writes IsExpanded straight to the node, so an expansion
95+
// made with the mouse never passes through this control; its Click is what identifies
96+
// one. Only a gesture scrolls: code that expands nodes to reveal a selection (see
97+
// ScrollIntoNodeView) or to open every match of a filter positions the viewport itself,
98+
// and a scroll per expanded node would fight it.
99+
AddHandler(Button.ClickEvent, OnExpanderClick, RoutingStrategies.Bubble, handledEventsToo: true);
95100
}
96101

97102
public SharpTreeNode? Root {
@@ -184,32 +189,47 @@ void OnDoubleTapped(object? sender, Avalonia.Interactivity.RoutedEventArgs e)
184189
node.ActivateItem(args);
185190
if (!e.Handled && node.ShowExpander)
186191
{
187-
node.IsExpanded = !node.IsExpanded;
192+
SetExpanded(node, !node.IsExpanded);
188193
e.Handled = true;
189194
}
190195
}
191196

197+
void OnExpanderClick(object? sender, RoutedEventArgs e)
198+
{
199+
if (e.Source is ToggleButton { Name: "PART_Expander" } expander
200+
&& expander.DataContext is SharpTreeNode { IsExpanded: true } node)
201+
{
202+
HandleExpanding(node);
203+
}
204+
}
205+
206+
/// <summary>Expands or collapses <paramref name="node"/> as a user gesture, so an expansion
207+
/// reveals its children the way <see cref="HandleExpanding"/> describes.</summary>
208+
void SetExpanded(SharpTreeNode node, bool expanded)
209+
{
210+
if (node.IsExpanded == expanded)
211+
return;
212+
node.IsExpanded = expanded;
213+
if (expanded)
214+
HandleExpanding(node);
215+
}
216+
192217
/// <summary>
193-
/// Called when a visible node expands so its newly shown children are scrolled into view
194-
/// (without scrolling the node itself off the top).
218+
/// Scrolls the rows a just-expanded node revealed into view, the way the native Windows
219+
/// tree control does: far enough to show the new children, but never so far that the
220+
/// expanded node itself leaves the viewport. Both steps only move the viewport when their
221+
/// row lies outside it, so expanding a node whose children already fit below it does not
222+
/// scroll at all.
195223
/// </summary>
196-
internal void HandleExpanding(SharpTreeNode node)
224+
void HandleExpanding(SharpTreeNode node)
197225
{
198-
if (doNotScrollOnExpanding)
199-
return;
200226
SharpTreeNode lastVisibleChild = node;
201-
while (true)
202-
{
203-
var child = lastVisibleChild.Children.LastOrDefault(c => c.IsVisible);
204-
if (child == null)
205-
break;
227+
while (lastVisibleChild.Children.LastOrDefault(c => c.IsVisible) is { } child)
206228
lastVisibleChild = child;
207-
}
208-
if (lastVisibleChild != node)
209-
{
210-
ScrollRowIntoView(lastVisibleChild, centre: false);
211-
Dispatcher.UIThread.Post(() => ScrollRowIntoView(node, centre: false), DispatcherPriority.Loaded);
212-
}
229+
if (lastVisibleChild == node)
230+
return;
231+
ScrollRowIntoView(lastVisibleChild, centre: false);
232+
ScrollRowIntoView(node, centre: false);
213233
}
214234

215235
/// <summary>Scrolls the node into view (unless <paramref name="scroll"/> is false) and gives it
@@ -252,10 +272,8 @@ void SelectAndFocus(SharpTreeNode node)
252272
public void ScrollIntoNodeView(SharpTreeNode node)
253273
{
254274
ArgumentNullException.ThrowIfNull(node);
255-
doNotScrollOnExpanding = true;
256275
foreach (var ancestor in node.Ancestors())
257276
ancestor.IsExpanded = true;
258-
doNotScrollOnExpanding = false;
259277
CenterNodeInView(node);
260278
}
261279

@@ -403,7 +421,7 @@ protected override void OnKeyDown(KeyEventArgs e)
403421
{
404422
case Key.Left:
405423
if (node.IsExpanded)
406-
node.IsExpanded = false;
424+
SetExpanded(node, false);
407425
else if (node.Parent != null && !node.Parent.IsRoot)
408426
SelectAndFocus(node.Parent);
409427
else
@@ -412,24 +430,26 @@ protected override void OnKeyDown(KeyEventArgs e)
412430
break;
413431
case Key.Right:
414432
if (!node.IsExpanded && node.ShowExpander)
415-
node.IsExpanded = true;
433+
SetExpanded(node, true);
416434
else if (node.Children.Count > 0)
417435
SelectAndFocus(node.Children.First(c => c.IsVisible));
418436
else
419437
break;
420438
e.Handled = true;
421439
break;
422440
case Key.Add:
423-
node.IsExpanded = true;
441+
SetExpanded(node, true);
424442
e.Handled = true;
425443
break;
426444
case Key.Subtract:
427-
node.IsExpanded = false;
445+
SetExpanded(node, false);
428446
e.Handled = true;
429447
break;
430448
case Key.Multiply:
431449
node.IsExpanded = true;
432450
ExpandRecursively(node);
451+
// The whole subtree is open now, so this reveals as much of it as fits.
452+
HandleExpanding(node);
433453
e.Handled = true;
434454
break;
435455
case Key.Enter:

0 commit comments

Comments
 (0)