diff --git a/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs b/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs index d00c1eb42..49b5010dc 100644 --- a/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs +++ b/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs @@ -684,6 +684,74 @@ public class AssemblyTreeTests "clicking an already-visible row must not move the viewport"); } + [AvaloniaTest] + public async Task Selecting_A_Visible_Row_Via_The_Model_Does_Not_Recentre_Viewport() + { + // "Decompile to new tab" (and any model-driven navigation) selects the node in the tree, which + // syncs through TreeSelectionBinder -> ScrollIntoNodeView -> CenterNodeInView. Unlike a real + // mouse click, that path DOES run the centring code, so the in-viewport early-return must also + // cover it -- otherwise opening an already-visible item in a new tab yanks the tree to the centre. + + var (window, vm) = await TestHarness.BootAsync(3); + + var enumerable = vm.AssemblyTreeModel.FindNode( + "System.Linq", "System.Linq", "System.Linq.Enumerable"); + enumerable.Expand(); + var ns = (NamespaceTreeNode)enumerable.Parent!; + ns.Expand(); + var asm = (AssemblyTreeNode)ns.Parent!; + asm.IsExpanded = true; + foreach (var child in ns.Children.OfType()) + { + child.Expand(); + } + + var pane = await window.WaitForComponent(); + var grid = await pane.WaitForComponent(); + + vm.AssemblyTreeModel.SelectNode(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 = await grid.WaitForComponent(); + (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 the bottom-most fully-visible non-selected row -- the strictest probe for an unwanted recentre. + var candidateRow = grid.GetVisualDescendants().OfType() + .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 select"); + var candidateNode = candidateRow!.Node; + ((object?)candidateNode).Should().NotBeNull(); + + var offsetBefore = scrollViewer.Offset.Y; + + // Act — model-driven selection (the open-in-new-tab path), NOT a mouse click. + vm.AssemblyTreeModel.SelectNode(candidateNode); + for (int i = 0; i < 8; i++) + { + Dispatcher.UIThread.RunJobs(); + await Task.Delay(25); + } + + scrollViewer.Offset.Y.Should().BeApproximately(offsetBefore, 1.0, + "selecting an already-visible row via the model (e.g. Decompile to new tab) must not move the viewport"); + } + [AvaloniaTest] public async Task Save_Code_Command_Dispatches_Single_Selected_Node_Save_Override() { diff --git a/ILSpy/Controls/TreeView/SharpTreeView.cs b/ILSpy/Controls/TreeView/SharpTreeView.cs index d1e9f2558..f4960d995 100644 --- a/ILSpy/Controls/TreeView/SharpTreeView.cs +++ b/ILSpy/Controls/TreeView/SharpTreeView.cs @@ -196,17 +196,35 @@ namespace ILSpy.Controls.TreeView } } - /// Scrolls the node into view and gives it keyboard focus. - public void FocusNode(SharpTreeNode node) + /// Scrolls the node into view (unless is false) and gives it + /// keyboard focus. Pass scroll: false to focus a row that is already visible without + /// disturbing the scroll position. + public void FocusNode(SharpTreeNode node, bool scroll = true) { ArgumentNullException.ThrowIfNull(node); - ScrollIntoNodeView(node); + if (scroll) + ScrollIntoNodeView(node); if (ContainerFromItem(node) is { } container) container.Focus(); else Dispatcher.UIThread.Post(() => (ContainerFromItem(node))?.Focus(), DispatcherPriority.Loaded); } + /// True when the node's row is realised and lies fully within the scroll viewport. + /// Used to decide, before a selection change scrolls the list, whether a reveal is needed at + /// all -- an already-visible row should not be pulled to the centre. + public bool IsNodeFullyVisible(SharpTreeNode node) + { + ArgumentNullException.ThrowIfNull(node); + var scrollViewer = this.GetVisualDescendants().OfType().FirstOrDefault(); + if (scrollViewer is null) + return false; + if (ContainerFromItem(node) is Control row && row.IsVisible + && row.TranslatePoint(new Point(0, 0), scrollViewer) is { } top) + return top.Y >= 0 && top.Y + row.Bounds.Height <= scrollViewer.Viewport.Height; + return false; + } + /// Moves the (single) selection to and focuses it. /// Used by keyboard navigation where selection must follow the caret. void SelectAndFocus(SharpTreeNode node) @@ -245,7 +263,8 @@ namespace ILSpy.Controls.TreeView // keeps a re-selection of an already-centred row from twitching. We deliberately do NOT // skip a merely-visible row sitting at an edge: the ListBox's AutoScrollToSelectedItem // drags the selected row to the nearest edge first, and a reveal should still pull it to - // the centre from there. + // the centre from there. (Skipping an already-visible row is decided one level up, before + // AutoScroll runs, in the model->tree sync -- see TreeSelectionBinder.SyncModelToTree.) if (ContainerFromItem(node) is Control visible && visible.IsVisible && visible.TranslatePoint(new Point(0, 0), scrollViewer) is { } top) { diff --git a/ILSpy/Controls/TreeView/TreeSelectionBinder.cs b/ILSpy/Controls/TreeView/TreeSelectionBinder.cs index 1a4727ad2..a3832d2e4 100644 --- a/ILSpy/Controls/TreeView/TreeSelectionBinder.cs +++ b/ILSpy/Controls/TreeView/TreeSelectionBinder.cs @@ -18,6 +18,7 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Collections.ObjectModel; using System.Collections.Specialized; using System.Linq; @@ -111,6 +112,16 @@ namespace ILSpy.Controls.TreeView syncing = true; try { + // Snapshot which selected rows are already fully visible BEFORE mutating the selection: + // the change triggers the ListBox's AutoScrollToSelectedItem, which drags an off-screen + // row to an edge and would make it look "visible" by reveal time. A row that was already + // on screen must not be revealed/recentred (e.g. Decompile to new tab on a visible row); + // a genuinely off-screen one still gets centred (Back navigation, go-to-definition). + var visibleBefore = new HashSet(ReferenceEqualityComparer.Instance); + foreach (var node in modelSelection) + if (tree.IsNodeFullyVisible(node)) + visibleBefore.Add(node); + tree.SelectedItems!.Clear(); SharpTreeNode? primary = null; var items = tree.ItemsSource as IList; @@ -132,9 +143,11 @@ namespace ILSpy.Controls.TreeView // synchronous ScrollIntoView would throw "Invalid Arrange rectangle". if (primary is { } toReveal) { + bool wasVisible = visibleBefore.Contains(toReveal); Dispatcher.UIThread.Post(() => { - tree.ScrollIntoNodeView(toReveal); - tree.FocusNode(toReveal); + if (!wasVisible) + tree.ScrollIntoNodeView(toReveal); + tree.FocusNode(toReveal, scroll: !wasVisible); }); } }