From a95f64161bd4ec9995d87a45ab0b78fed800371c Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 5 Jun 2026 16:19:36 +0200 Subject: [PATCH] Restore center-on-select in the tree; retarget the context-menu tests The assembly-tree SharpTreeView migration (0995d32df, "WIP: tests pending") left two bits of debt that surfaced as seven failing tests: - It dropped the old AssemblyListPane.CenterRowInView, so a model-driven selection only scrolled the row to the nearest viewport edge. Reinstate centring in SharpTreeView.ScrollIntoNodeView via CenterNodeInView: bring the row on screen, then offset so it sits at the vertical centre. It skips the move only when the row is already roughly centred (not merely visible at an edge), because the ListBox's AutoScrollToSelectedItem drags the selected row to an edge first and a reveal should still pull it to the centre -- while a click on an already-centred row doesn't twitch. - DecompileInNewViewTests still queried the removed ProDataGrid surface (DataGrid / DataGridRow / HierarchicalNode). Retarget to SharpTreeView / SharpTreeViewItem and the node-valued SelectedItem, and clamp the right-click X to the visible grid width (tree rows stretch to content width, so a row centre can sit off-screen). Full suite: the seven deterministic failures are gone; the only remaining two are the pre-existing group-ordering-flaky pair (both pass in isolation). --- .../ContextMenus/DecompileInNewViewTests.cs | 52 +++++++++---------- ILSpy/Controls/TreeView/SharpTreeView.cs | 45 ++++++++++++++++ 2 files changed, 71 insertions(+), 26 deletions(-) diff --git a/ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs b/ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs index cd8b1f75c..7b164789b 100644 --- a/ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs +++ b/ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs @@ -21,7 +21,6 @@ using System.Threading.Tasks; using Avalonia; using Avalonia.Controls; -using Avalonia.Controls.DataGridHierarchical; using Avalonia.Headless; using Avalonia.Headless.NUnit; using Avalonia.Input; @@ -163,17 +162,17 @@ public class DecompileInNewViewTests public async Task Right_Clicking_An_Unselected_Row_Does_Not_Move_The_Selection() { // Thunderbird-style context target: right-clicking a row the user has NOT selected must - // leave the real selection (and therefore the preview document) untouched. Today - // ProDataGrid's default selects the right-clicked row on press, which yanks the preview - // to B before the menu opens — so "Decompile to new tab" ends up with B twice instead of - // the intended A + B. This probe asserts the selection stays put under a real right-click. + // leave the real selection (and therefore the preview document) untouched. A ListBox's + // default selects the row on press, which would yank the preview to B before the menu opens + // -- so "Decompile to new tab" would end up with B twice instead of the intended A + B. The + // pane suppresses the right-press to prevent that; this probe asserts the selection stays put. var window = AppComposition.Current.GetExport(); window.Show(); var vm = (MainWindowViewModel)window.DataContext!; await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); var pane = await window.WaitForComponent(); - var grid = await pane.WaitForComponent(); + var grid = await pane.WaitForComponent(); var nodeA = vm.AssemblyTreeModel.FindNode("System.Linq"); var nodeB = vm.AssemblyTreeModel.FindNode(TreeNavigation.CoreLibName); @@ -189,7 +188,7 @@ public class DecompileInNewViewTests await Task.Delay(25); } - var rowB = grid.GetVisualDescendants().OfType() + var rowB = grid.GetVisualDescendants().OfType() .FirstOrDefault(r => RowNodeEquals(r, nodeB)); rowB.Should().NotBeNull("the top-level CoreLib row must be realised"); @@ -199,8 +198,10 @@ public class DecompileInNewViewTests grid.AddHandler(Control.ContextRequestedEvent, (_, _) => menuRequested = true, handledEventsToo: true); - // Right-click the centre of B's row (clear of the far-left expander glyph). - var point = rowB!.TranslatePoint(new Point(rowB.Bounds.Width / 2, rowB.Bounds.Height / 2), window); + // Right-click the centre of B's row (clear of the far-left expander glyph). Tree rows + // stretch to content width, so clamp X to the visible grid viewport. + var clickX = System.Math.Min(rowB!.Bounds.Width, grid.Bounds.Width) / 2; + var point = rowB.TranslatePoint(new Point(clickX, rowB.Bounds.Height / 2), window); point.Should().NotBeNull(); HeadlessWindowExtensions.MouseDown(window, point!.Value, MouseButton.Right); HeadlessWindowExtensions.MouseUp(window, point.Value, MouseButton.Right); @@ -229,7 +230,7 @@ public class DecompileInNewViewTests var vm = (MainWindowViewModel)window.DataContext!; await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); var pane = await window.WaitForComponent(); - var grid = await pane.WaitForComponent(); + var grid = await pane.WaitForComponent(); var assemblies = vm.AssemblyTreeModel.Root!.Children.OfType().Take(3).ToArray(); assemblies.Length.Should().BeGreaterThanOrEqualTo(3, "need three top-level rows to click"); @@ -245,13 +246,15 @@ public class DecompileInNewViewTests await Task.Delay(25); } - DataGridRow Row(SharpTreeNode node) => grid.GetVisualDescendants().OfType() + global::ILSpy.Controls.TreeView.SharpTreeViewItem Row(SharpTreeNode node) => grid.GetVisualDescendants() + .OfType() .First(r => RowNodeEquals(r, node)); async Task RightClick(SharpTreeNode node) { var row = Row(node); - var pt = row.TranslatePoint(new Point(row.Bounds.Width / 2, row.Bounds.Height / 2), window); + var clickX = System.Math.Min(row.Bounds.Width, grid.Bounds.Width) / 2; + var pt = row.TranslatePoint(new Point(clickX, row.Bounds.Height / 2), window); HeadlessWindowExtensions.MouseDown(window, pt!.Value, MouseButton.Right); HeadlessWindowExtensions.MouseUp(window, pt.Value, MouseButton.Right); for (int i = 0; i < 4; i++) @@ -295,7 +298,7 @@ public class DecompileInNewViewTests var vm = (MainWindowViewModel)window.DataContext!; await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); var pane = await window.WaitForComponent(); - var grid = await pane.WaitForComponent(); + var grid = await pane.WaitForComponent(); var nodeA = vm.AssemblyTreeModel.FindNode("System.Linq"); var nodeB = vm.AssemblyTreeModel.FindNode(TreeNavigation.CoreLibName); @@ -323,8 +326,7 @@ public class DecompileInNewViewTests // The model selection follows the new active tab... ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, nodeB).Should().BeTrue( "the tree model selection must follow the newly-activated tab"); - // ...and so does the grid's visual selection (the grid stores either the raw node or its - // HierarchicalNode wrapper depending on the path that set it). + // ...and so does the tree's visual selection (SharpTreeView selects the SharpTreeNode directly). ReferenceEquals(GridSelectedNode(grid), nodeB).Should().BeTrue( "the grid's visual selection must follow the newly-activated tab"); } @@ -342,7 +344,7 @@ public class DecompileInNewViewTests var vm = (MainWindowViewModel)window.DataContext!; await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); var pane = await window.WaitForComponent(); - var grid = await pane.WaitForComponent(); + var grid = await pane.WaitForComponent(); var assemblies = vm.AssemblyTreeModel.Root!.Children.OfType().Take(3).ToArray(); assemblies.Length.Should().BeGreaterThanOrEqualTo(3, "need three top-level rows"); @@ -395,7 +397,7 @@ public class DecompileInNewViewTests var vm = (MainWindowViewModel)window.DataContext!; await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3); var pane = await window.WaitForComponent(); - var grid = await pane.WaitForComponent(); + var grid = await pane.WaitForComponent(); var documents = ((ILSpyDockFactory)vm.DockWorkspace.Factory).Documents!; var assemblies = vm.AssemblyTreeModel.Root!.Children.OfType().Take(3).ToArray(); @@ -439,16 +441,14 @@ public class DecompileInNewViewTests "the grid must visually highlight every restored node"); } - static bool RowNodeEquals(DataGridRow row, SharpTreeNode node) - => row.DataContext is HierarchicalNode hn && ReferenceEquals(hn.Item, node); + static bool RowNodeEquals(global::ILSpy.Controls.TreeView.SharpTreeViewItem row, SharpTreeNode node) + => ReferenceEquals(row.DataContext, node); - static SharpTreeNode? GridSelectedNode(DataGrid grid) - => grid.SelectedItem is HierarchicalNode hn ? hn.Item as SharpTreeNode : grid.SelectedItem as SharpTreeNode; + static SharpTreeNode? GridSelectedNode(global::ILSpy.Controls.TreeView.SharpTreeView grid) + => grid.SelectedItem as SharpTreeNode; - static System.Collections.Generic.List GridSelectedNodes(DataGrid grid) - => grid.SelectedItems.Cast() - .Select(o => o is HierarchicalNode hn ? hn.Item as SharpTreeNode : o as SharpTreeNode) - .Where(n => n != null) - .Select(n => n!) + static System.Collections.Generic.List GridSelectedNodes(global::ILSpy.Controls.TreeView.SharpTreeView grid) + => grid.SelectedItems!.Cast() + .OfType() .ToList(); } diff --git a/ILSpy/Controls/TreeView/SharpTreeView.cs b/ILSpy/Controls/TreeView/SharpTreeView.cs index 90499d5c2..d1e9f2558 100644 --- a/ILSpy/Controls/TreeView/SharpTreeView.cs +++ b/ILSpy/Controls/TreeView/SharpTreeView.cs @@ -222,7 +222,52 @@ namespace ILSpy.Controls.TreeView foreach (var ancestor in node.Ancestors()) ancestor.IsExpanded = true; doNotScrollOnExpanding = false; + CenterNodeInView(node); + } + + /// + /// Reveals centred in the viewport so the user's eye lands on a + /// newly selected row, rather than at the nearest edge (where + /// leaves it). Skips the move when the row is already fully visible, so clicking a visible + /// row -- or selecting a freshly-loaded top-level entry that's already on screen -- never + /// yanks the viewport. + /// + void CenterNodeInView(SharpTreeNode node) + { + var scrollViewer = this.GetVisualDescendants().OfType().FirstOrDefault(); + if (scrollViewer is null) + { + ScrollIntoView(node); + return; + } + + // If the row is already realised and roughly centred, leave the viewport alone -- this + // 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. + if (ContainerFromItem(node) is Control visible && visible.IsVisible + && visible.TranslatePoint(new Point(0, 0), scrollViewer) is { } top) + { + var rowMid = top.Y + visible.Bounds.Height / 2; + var viewportMid = scrollViewer.Viewport.Height / 2; + if (Math.Abs(rowMid - viewportMid) <= visible.Bounds.Height) + return; + } + + // Bring it on screen (edge), force layout so the container realises, then offset so the + // row sits at the vertical centre. ScrollIntoView(node); + UpdateLayout(); + if (ContainerFromItem(node) is not Control row) + return; + if (row.TranslatePoint(new Point(0, 0), scrollViewer) is not { } rowTop) + return; + var desiredTop = (scrollViewer.Viewport.Height - row.Bounds.Height) / 2; + var newOffsetY = scrollViewer.Offset.Y + (rowTop.Y - desiredTop); + var maxOffset = Math.Max(0, scrollViewer.Extent.Height - scrollViewer.Viewport.Height); + newOffsetY = Math.Clamp(newOffsetY, 0, maxOffset); + scrollViewer.Offset = new Vector(scrollViewer.Offset.X, newOffsetY); } protected override void OnKeyDown(KeyEventArgs e)