Browse Source

Don't recentre tree row on user click

Clicking an already-visible row was running ScrollIntoView+CenterRowInView via
the SelectionChanged → model PropertyChanged round-trip, yanking the viewport.
Add a clickInProgress flag separate from syncingSelection: clickInProgress is
set in OnTreeGridSelectionChanged and cleared at Background priority alongside
syncingSelection, so the model's two PropertyChanged notifications (Clear+Add
on SelectedItems) both bail out — but hyperlink / Back/Forward / search-hit
navigation, which never enters OnTreeGridSelectionChanged, still scrolls the
target into view.

Assisted-by: Claude:claude-opus-4-7:Claude Code
pull/3755/head
Siegfried Pammer 2 months ago
parent
commit
8e3a922b98
  1. 80
      ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs
  2. 23
      ILSpy/AssemblyTree/AssemblyListPane.axaml.cs

80
ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs

@ -22,7 +22,12 @@ using System.Linq; @@ -22,7 +22,12 @@ using System.Linq;
using System.Threading.Tasks;
using System.Xml.Linq;
using Avalonia;
using Avalonia.Controls;
using Avalonia.Controls.DataGridHierarchical;
using Avalonia.Headless.NUnit;
using Avalonia.Threading;
using Avalonia.VisualTree;
using AwesomeAssertions;
@ -30,9 +35,7 @@ using ICSharpCode.ILSpy.Properties; @@ -30,9 +35,7 @@ using ICSharpCode.ILSpy.Properties;
using ICSharpCode.Decompiler.TypeSystem;
using ICSharpCode.ILSpyX;
using Avalonia.Controls;
using Avalonia.VisualTree;
using ICSharpCode.ILSpyX.TreeView;
using ILSpy;
using ILSpy.AppEnv;
@ -589,4 +592,75 @@ public class AssemblyTreeTests @@ -589,4 +592,75 @@ public class AssemblyTreeTests
node.LoadedAssembly.Should().BeSameAs(loaded);
node.IsSelected.Should().BeTrue();
}
[AvaloniaTest]
public async Task User_Click_On_Visible_Row_Does_Not_Recentre_Viewport()
{
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3);
var enumerable = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Linq", "System.Linq", "System.Linq.Enumerable");
enumerable.EnsureLazyChildren();
var ns = (NamespaceTreeNode)enumerable.Parent!;
ns.EnsureLazyChildren();
foreach (var child in ns.Children.OfType<TypeTreeNode>())
child.EnsureLazyChildren();
var pane = await window.WaitForComponent<AssemblyListPane>();
var grid = await pane.WaitForComponent<DataGrid>();
// Park the viewport mid-list so the next-clicked row sits at a non-zero offset. The
// regression would re-centre on click and snap the offset back; we want offset to
// stay put.
vm.AssemblyTreeModel.SelectedItem = enumerable;
await Waiters.WaitForAsync(() => ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, enumerable));
for (int i = 0; i < 8; i++)
{
Dispatcher.UIThread.RunJobs();
await Task.Delay(25);
}
grid.UpdateLayout();
var scrollViewer = grid.GetVisualDescendants().OfType<ScrollViewer>().Single();
(scrollViewer.Extent.Height - scrollViewer.Viewport.Height).Should().BeGreaterThan(50,
"the grid must have something to scroll for this test to be meaningful");
scrollViewer.Offset = new Vector(scrollViewer.Offset.X, 50);
grid.UpdateLayout();
Dispatcher.UIThread.RunJobs();
scrollViewer.Offset.Y.Should().BeGreaterThan(5,
"the test scenario requires the viewport be parked mid-list");
// Pick a different visible row to click on; clicking an already-visible row must NOT
// move the viewport. Use the *bottom-most* visible row for a strict check — if the bug
// regressed, CenterRowInView would scroll it up to the middle and offset would change.
var candidateRow = grid.GetVisualDescendants().OfType<DataGridRow>()
.Where(r => !r.IsSelected
&& r.TranslatePoint(new Point(0, 0), scrollViewer) is { } p
&& p.Y >= 0 && p.Y + r.Bounds.Height <= scrollViewer.Viewport.Height)
.OrderByDescending(r => r.TranslatePoint(new Point(0, 0), scrollViewer)!.Value.Y)
.FirstOrDefault();
candidateRow.Should().NotBeNull("the test needs a visible non-selected row to click");
var offsetBefore = scrollViewer.Offset.Y;
// Real pointer click — setting SelectedItem programmatically would fire DataGrid's
// internal ScrollIntoView too, which a real user click does not.
var rowCentre = candidateRow!.TranslatePoint(
new Point(candidateRow.Bounds.Width / 2, candidateRow.Bounds.Height / 2),
window)!.Value;
global::Avalonia.Headless.HeadlessWindowExtensions.MouseDown(window, rowCentre, global::Avalonia.Input.MouseButton.Left);
global::Avalonia.Headless.HeadlessWindowExtensions.MouseUp(window, rowCentre, global::Avalonia.Input.MouseButton.Left);
for (int i = 0; i < 8; i++)
{
Dispatcher.UIThread.RunJobs();
await Task.Delay(25);
}
scrollViewer.Offset.Y.Should().BeApproximately(offsetBefore, 1.0,
"clicking an already-visible row must not move the viewport");
}
}

23
ILSpy/AssemblyTree/AssemblyListPane.axaml.cs

@ -132,9 +132,26 @@ namespace ILSpy.AssemblyTree @@ -132,9 +132,26 @@ namespace ILSpy.AssemblyTree
}
// DataGrid.ScrollIntoView only brings the row to the nearest viewport edge; we want
// the row centred so the user's eye lands on it.
// the row centred so the user's eye lands on it. Skip the move when the row is already
// fully in view — re-centring an in-view row would yank the viewport on every selection
// (e.g. user clicks a visible row, or Ctrl+O selects a freshly-loaded top-level entry).
void CenterRowInView(HierarchicalNode node)
{
var scrollViewer = TreeGrid.GetVisualDescendants().OfType<ScrollViewer>().FirstOrDefault();
if (scrollViewer is null)
return;
// Cheap pre-check: if the row's already realised AND fully visible, leave the
// viewport alone. ScrollIntoView (next call) would otherwise drag it to the edge,
// and our centring step would then drag it again.
var existingRow = TreeGrid.GetVisualDescendants().OfType<DataGridRow>()
.FirstOrDefault(r => ReferenceEquals(r.DataContext, node));
if (existingRow is { IsVisible: true }
&& existingRow.TranslatePoint(new Point(0, 0), scrollViewer) is { } existingTop
&& existingTop.Y >= 0
&& existingTop.Y + existingRow.Bounds.Height <= scrollViewer.Viewport.Height)
return;
TreeGrid.ScrollIntoView(node, TreeGrid.Columns[0]);
// ScrollIntoView only changes ScrollViewer.Offset; the row becomes a realised
// DataGridRow during the next layout pass. Force it now so we can read the row's
@ -146,10 +163,6 @@ namespace ILSpy.AssemblyTree @@ -146,10 +163,6 @@ namespace ILSpy.AssemblyTree
if (row is null)
return;
var scrollViewer = TreeGrid.GetVisualDescendants().OfType<ScrollViewer>().FirstOrDefault();
if (scrollViewer is null)
return;
var rowTopInViewer = row.TranslatePoint(new Point(0, 0), scrollViewer);
if (rowTopInViewer is null)
return;

Loading…
Cancel
Save