From 8fb188301f834098d8eacd7f2cc470245ff98a0d Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Mon, 1 Jun 2026 22:54:16 +0200 Subject: [PATCH] Fix assembly-tree selection: Ctrl+A and click-to-collapse Two selection-sync defects in the assembly tree. Ctrl+A only selected the last row on the first press: SelectedItem is the last entry of SelectedItems and every collection change re-raised it, so the grid->model sync bounced back through SyncSelectionFromModel and assigned the singular SelectedItem, collapsing the just-made multi-selection. The syncingSelection guard now also covers that notification. Clicking one row of a multi-selection left every row selected: ProDataGrid keeps the selection on press so a row-drag can move all of them (multi-row reorder is a real feature), but 12.0.0 has no release-side collapse for a plain click. Mirror the usual behaviour by collapsing to the clicked row on release when the pointer did not move far enough to be a drag. --- ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs | 101 ++++++++++++++++++ ILSpy/AssemblyTree/AssemblyListPane.axaml.cs | 91 ++++++++++++++-- 2 files changed, 186 insertions(+), 6 deletions(-) diff --git a/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs b/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs index 593d2b9ae..aef68d233 100644 --- a/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs +++ b/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs @@ -24,7 +24,10 @@ using System.Xml.Linq; using Avalonia; using Avalonia.Controls; +using Avalonia.Controls.DataGridHierarchical; +using Avalonia.Headless; using Avalonia.Headless.NUnit; +using Avalonia.Input; using Avalonia.Threading; using Avalonia.VisualTree; @@ -33,6 +36,7 @@ using AwesomeAssertions; using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.ILSpy.Properties; using ICSharpCode.ILSpyX; +using ICSharpCode.ILSpyX.TreeView; using ILSpy; using ILSpy.AppEnv; @@ -1558,4 +1562,101 @@ public class AssemblyTreeTests globalNs.Children.OfType().Should().NotBeEmpty( " (and any other global types) belong under the '-' node"); } + + [AvaloniaTest] + public async Task CtrlA_Selects_Every_Top_Level_Row_On_The_First_Press() + { + // Ctrl+A in the assembly tree must select all rows on the FIRST press, not just move + // the current cell to the last row (the user reported having to press it twice). + + // Arrange — boot, wait for assemblies, realise the grid. + 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(); + grid.UpdateLayout(); + + var topLevelCount = vm.AssemblyTreeModel.Root!.Children.Count; + topLevelCount.Should().BeGreaterThan(1, "the test needs several top-level rows to be meaningful"); + + // Give the grid keyboard focus WITHOUT clicking a cell first — this is the state the + // tree is in right after the user tabs/clicks into the pane but before establishing a + // current cell. The DataGrid only handles Ctrl+A when a focused element lives inside it. + grid.Focus(); + Dispatcher.UIThread.RunJobs(); + + // First press. + window.KeyPress(Key.A, RawInputModifiers.Control, PhysicalKey.A, keySymbol: "a"); + Dispatcher.UIThread.RunJobs(); + await Task.Delay(50); + Dispatcher.UIThread.RunJobs(); + // What the USER sees is the grid's selection. A feedback loop through the model's + // singular SelectedItem can collapse the grid back to one row even while the model's + // SelectedItems still holds all of them — so assert on the grid, not the model. + int gridAfterFirst = grid.SelectedItems.Count; + int modelAfterFirst = vm.AssemblyTreeModel.SelectedItems.Count; + + // Second press. + window.KeyPress(Key.A, RawInputModifiers.Control, PhysicalKey.A, keySymbol: "a"); + Dispatcher.UIThread.RunJobs(); + await Task.Delay(50); + Dispatcher.UIThread.RunJobs(); + int gridAfterSecond = grid.SelectedItems.Count; + + // Assert — every top-level row is selected in the grid after the FIRST press. + gridAfterFirst.Should().Be(topLevelCount, + $"Ctrl+A must select all {topLevelCount} assembly rows on the first press " + + $"(grid after 1st={gridAfterFirst}, model after 1st={modelAfterFirst}, grid after 2nd={gridAfterSecond})"); + } + + [AvaloniaTest] + public async Task Plain_Click_On_A_Row_Reduces_A_Multi_Selection_To_That_Row() + { + // With several rows selected (e.g. after Ctrl+A), a plain left-click (no modifier) on + // one row must collapse the selection down to just that row. + + // Arrange — boot, wait for assemblies, realise the grid, select everything. + 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(); + grid.UpdateLayout(); + + var topLevelCount = vm.AssemblyTreeModel.Root!.Children.Count; + topLevelCount.Should().BeGreaterThan(1, "the test needs several top-level rows to be meaningful"); + + grid.Focus(); + Dispatcher.UIThread.RunJobs(); + window.KeyPress(Key.A, RawInputModifiers.Control, PhysicalKey.A, keySymbol: "a"); + Dispatcher.UIThread.RunJobs(); + await Waiters.WaitForAsync(() => grid.SelectedItems.Count == topLevelCount); + + // Act — plain left-click on the second visible row. + var targetRow = grid.GetVisualDescendants().OfType() + .OrderBy(r => r.TranslatePoint(new Point(0, 0), grid)?.Y ?? double.MaxValue) + .Skip(1).First(); + var targetNode = (targetRow.DataContext as HierarchicalNode)?.Item as SharpTreeNode; + Assert.That(targetNode, Is.Not.Null, "the clicked row must wrap a tree node"); + var rowCentre = targetRow.TranslatePoint( + new Point(targetRow.Bounds.Width / 2, targetRow.Bounds.Height / 2), window)!.Value; + HeadlessWindowExtensions.MouseDown(window, rowCentre, MouseButton.Left); + HeadlessWindowExtensions.MouseUp(window, rowCentre, MouseButton.Left); + Dispatcher.UIThread.RunJobs(); + await Task.Delay(50); + Dispatcher.UIThread.RunJobs(); + + // Assert — selection collapsed to exactly the clicked row, in both grid and model. + grid.SelectedItems.Count.Should().Be(1, + $"a plain click must reduce the selection to one row (grid has {grid.SelectedItems.Count})"); + var modelSelection = vm.AssemblyTreeModel.SelectedItems; + modelSelection.Count.Should().Be(1, "the model selection must collapse to the clicked node too"); + ReferenceEquals(modelSelection[0], targetNode).Should().BeTrue( + "the surviving selection must be the row the user clicked"); + } } diff --git a/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs b/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs index cb4e93994..a0727869b 100644 --- a/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs +++ b/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs @@ -47,6 +47,13 @@ namespace ILSpy.AssemblyTree // selection into the DataGrid. bool syncingSelection; + // A plain left-click on one row of a multi-selection: ProDataGrid keeps the whole + // selection on press (so the user can drag every selected row together), so the + // collapse-to-the-clicked-row has to happen on release if it turned out to be a click, + // not a drag. Recorded on press, resolved on release. + SharpTreeNode? pendingClickCollapseNode; + Point pendingClickCollapsePos; + LanguageSettings? languageSettings; public AssemblyListPane() @@ -76,6 +83,11 @@ namespace ILSpy.AssemblyTree TreeGrid.AddHandler(PointerPressedEvent, OnTreeGridPointerPressed, global::Avalonia.Interactivity.RoutingStrategies.Bubble, handledEventsToo: true); + // Same handledEventsToo opt-in for release: the press that preserves a multi-selection + // for a potential row-drag marks itself handled, so we'd miss the release otherwise. + TreeGrid.AddHandler(PointerReleasedEvent, OnTreeGridPointerReleased, + global::Avalonia.Interactivity.RoutingStrategies.Bubble, + handledEventsToo: true); // Drag-reorder of top-level assembly rows. The actual reorder lives in // AssemblyRowDropHandler (wired to the live AssemblyList in BindTree); the @@ -399,18 +411,79 @@ namespace ILSpy.AssemblyTree // by an unwanted new tab. LMB double-click is reserved for expand/collapse, // handled by OnTreeGridDoubleTapped. MMB doesn't move selection on its own, so we // hit-test the row from e.Source rather than reading SelectedItem. + pendingClickCollapseNode = null; if (e.Source is not Visual hit) return; var point = e.GetCurrentPoint(hit).Properties; - if (!point.IsMiddleButtonPressed) + if (point.IsMiddleButtonPressed) + { + var visual = hit; + while (visual != null && visual.DataContext is not HierarchicalNode) + visual = visual.GetVisualParent(); + if (visual?.DataContext is not HierarchicalNode hn || hn.Item is not ILSpyTreeNode node) + return; + OpenNodeInNewTab(node); + e.Handled = true; + return; + } + + // Plain LMB on a row that's already part of a multi-selection: the grid keeps the + // whole selection on press (for a potential drag of all rows), so remember the row + // and collapse to it on release if the gesture was a click rather than a drag. + // Clicking the expander glyph or holding a modifier are excluded — those have their + // own meaning (toggle / extend selection). + if (point.IsLeftButtonPressed + && e.KeyModifiers == KeyModifiers.None + && !IsExpanderHit(hit) + && DataContext is AssemblyTreeModel model + && model.SelectedItems.Count > 1) + { + var clicked = HitTestRowNode(hit); + if (clicked != null && model.SelectedItems.Contains(clicked)) + { + pendingClickCollapseNode = clicked; + pendingClickCollapsePos = e.GetPosition(TreeGrid); + } + } + } + + void OnTreeGridPointerReleased(object? sender, PointerReleasedEventArgs e) + { + var node = pendingClickCollapseNode; + pendingClickCollapseNode = null; + if (node == null || e.InitialPressMouseButton != MouseButton.Left) + return; + // A pointer that moved beyond the drag threshold was a drag (reorder), not a click — + // leave the multi-selection intact so the drop can move every selected row. + var delta = e.GetPosition(TreeGrid) - pendingClickCollapsePos; + if (Math.Abs(delta.X) > 4 || Math.Abs(delta.Y) > 4) + return; + if (DataContext is not AssemblyTreeModel model || !model.SelectedItems.Contains(node)) return; - var visual = hit; + + // Collapse to just the clicked row. Setting SelectedItem to the row that's already the + // grid's primary would no-op (DirectProperty SetAndRaise), so clear first in that case; + // otherwise SelectedItem's SelectCurrent action clears the rest for us. The resulting + // SelectionChanged feeds the model through OnTreeGridSelectionChanged as usual. + if (ReferenceEquals(TreeGrid.SelectedItem, node)) + TreeGrid.SelectedItem = null!; + TreeGrid.SelectedItem = node; + } + + static bool IsExpanderHit(Visual? source) + { + for (var v = source; v != null; v = v.GetVisualParent()) + if (v is global::Avalonia.Controls.Primitives.ToggleButton { Name: "PART_Expander" }) + return true; + return false; + } + + static SharpTreeNode? HitTestRowNode(Visual? source) + { + var visual = source; while (visual != null && visual.DataContext is not HierarchicalNode) visual = visual.GetVisualParent(); - if (visual?.DataContext is not HierarchicalNode hn || hn.Item is not ILSpyTreeNode node) - return; - OpenNodeInNewTab(node); - e.Handled = true; + return (visual?.DataContext as HierarchicalNode)?.Item as SharpTreeNode; } /// @@ -467,6 +540,12 @@ namespace ILSpy.AssemblyTree } else if (e.PropertyName == nameof(AssemblyTreeModel.SelectedItem)) { + // Skip the bounce-back while we're pushing the grid's own selection into the + // model. SelectedItem is the LAST entry of SelectedItems, so a multi-row grid + // change (e.g. Ctrl+A select-all) raises this notification; reacting to it would + // assign the singular SelectedItem and collapse the grid back to that one row. + if (syncingSelection) + return; SyncSelectionFromModel(model.SelectedItem); } }