Browse Source

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.
pull/3755/head
Siegfried Pammer 1 month ago
parent
commit
8fb188301f
  1. 101
      ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs
  2. 91
      ILSpy/AssemblyTree/AssemblyListPane.axaml.cs

101
ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs

@ -24,7 +24,10 @@ using System.Xml.Linq; @@ -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; @@ -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 @@ -1558,4 +1562,101 @@ public class AssemblyTreeTests
globalNs.Children.OfType<TypeTreeNode>().Should().NotBeEmpty(
"<Module> (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<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3);
var pane = await window.WaitForComponent<AssemblyListPane>();
var grid = await pane.WaitForComponent<DataGrid>();
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<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3);
var pane = await window.WaitForComponent<AssemblyListPane>();
var grid = await pane.WaitForComponent<DataGrid>();
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<DataGridRow>()
.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");
}
}

91
ILSpy/AssemblyTree/AssemblyListPane.axaml.cs

@ -47,6 +47,13 @@ namespace ILSpy.AssemblyTree @@ -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 @@ -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 @@ -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;
}
/// <summary>
@ -467,6 +540,12 @@ namespace ILSpy.AssemblyTree @@ -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);
}
}

Loading…
Cancel
Save