From 0e53fb9be1246e898191ed3c31b02043a372b978 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Mon, 27 Jul 2026 17:37:28 +0200 Subject: [PATCH] Center the selected step in the viewport after filter changes Filtering and un-filtering the Debug Steps tree re-arranges rows around the selection: matches surface under newly expanded groups and the restore step re-opens the selected step's ancestors. Without scrolling, the selected row regularly ended up outside the viewport even though it was expanded into view. The ViewModel now raises a reveal request after each filter pass while a visible step is selected; the View answers it by centering the row, because scrolling needs containers and the ScrollViewer. The measurement anchors on the row header since an expanded group's container spans its whole subtree, and it runs at Loaded dispatcher priority so freshly expanded containers have valid geometry. Assisted-by: Claude:claude-fable-5:Claude Code --- .../Views/DebugStepsFilterStateTests.cs | 42 +++++++++ ILSpy/ViewModels/DebugStepsPaneModel.cs | 11 +++ ILSpy/Views/DebugSteps.axaml.cs | 90 ++++++++++++++++++- 3 files changed, 140 insertions(+), 3 deletions(-) diff --git a/ILSpy.Tests/Views/DebugStepsFilterStateTests.cs b/ILSpy.Tests/Views/DebugStepsFilterStateTests.cs index 7b380630f..0f96f6b34 100644 --- a/ILSpy.Tests/Views/DebugStepsFilterStateTests.cs +++ b/ILSpy.Tests/Views/DebugStepsFilterStateTests.cs @@ -18,10 +18,12 @@ #if DEBUG +using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Avalonia; using Avalonia.Controls; using Avalonia.Headless.NUnit; using Avalonia.Threading; @@ -99,6 +101,46 @@ public class DebugStepsFilterStateTests _ => null, }; + [AvaloniaTest] + public Task Clearing_The_Filter_Centers_The_Selected_Step() + { + // A tree tall enough that the selected step sits far outside the initial viewport, so + // centering requires an actual scroll rather than being satisfied trivially. + var group = new Stepper.Node("TransformGroup"); + for (int i = 0; i < 60; i++) + { + group.Children.Add(new Stepper.Node($"{i}: Step number {i}")); + } + var vm = new DebugStepsPaneModel(); + SetSteps(vm, new[] { group }); + vm.IsAvailable = true; + var window = new Window { Width = 400, Height = 300, Content = new DebugSteps { DataContext = vm } }; + window.Show(); + Dispatcher.UIThread.RunJobs(); + var tree = window.GetVisualDescendants().OfType().First(); + + vm.FilterText = "number 42"; + Dispatcher.UIThread.RunJobs(); + var leaf = RowFor(tree, "42: Step number 42"); + tree.SelectedItem = leaf.DataContext; + Dispatcher.UIThread.RunJobs(); + + vm.FilterText = ""; + Dispatcher.UIThread.RunJobs(); + Dispatcher.UIThread.RunJobs(); + + var scrollViewer = tree.GetVisualDescendants().OfType().First(); + scrollViewer.Offset.Y.Should().BeGreaterThan(0, + "revealing a selected step deep in the tree must scroll the viewport"); + var rowCenter = leaf.TranslatePoint(new Point(0, leaf.Bounds.Height / 2), scrollViewer)!.Value.Y; + var viewportCenter = scrollViewer.Viewport.Height / 2; + Math.Abs(rowCenter - viewportCenter).Should().BeLessThan(leaf.Bounds.Height * 1.5, + "the selected step must end up roughly centered in the viewport"); + + window.Close(); + return Task.CompletedTask; + } + [AvaloniaTest] public Task Manual_Expansion_Survives_A_Filter_Round_Trip() { diff --git a/ILSpy/ViewModels/DebugStepsPaneModel.cs b/ILSpy/ViewModels/DebugStepsPaneModel.cs index 2f7603f22..319cf2ec8 100644 --- a/ILSpy/ViewModels/DebugStepsPaneModel.cs +++ b/ILSpy/ViewModels/DebugStepsPaneModel.cs @@ -134,6 +134,13 @@ namespace ICSharpCode.ILSpy.ViewModels public IRelayCommand ShowStateAfterCommand { get; } public IRelayCommand DebugStepCommand { get; } + /// + /// Raised after a filter change re-arranged the tree while a step is selected and still + /// visible. Scrolling is a view concern (it needs containers and a ScrollViewer), so the + /// view listens and centers the selected row in its viewport. + /// + public event System.EventHandler? SelectionRevealRequested; + /// Design-time / fallback ctor — no dependencies wired. public DebugStepsPaneModel() { @@ -281,6 +288,8 @@ namespace ICSharpCode.ILSpy.ViewModels string filter = FilterText!.Trim(); foreach (var step in Steps) ApplyFilterToNode(step, filter); + if (SelectedStep is { IsVisible: true }) + SelectionRevealRequested?.Invoke(this, System.EventArgs.Empty); } else if (filterSnapshotTaken) { @@ -289,6 +298,8 @@ namespace ICSharpCode.ILSpy.ViewModels RestoreExpansion(step); for (var ancestor = SelectedStep?.Parent; ancestor != null; ancestor = ancestor.Parent) ancestor.IsExpanded = true; + if (SelectedStep != null) + SelectionRevealRequested?.Invoke(this, System.EventArgs.Empty); } } diff --git a/ILSpy/Views/DebugSteps.axaml.cs b/ILSpy/Views/DebugSteps.axaml.cs index cf6de5de0..65cb1ea9f 100644 --- a/ILSpy/Views/DebugSteps.axaml.cs +++ b/ILSpy/Views/DebugSteps.axaml.cs @@ -18,9 +18,16 @@ #if DEBUG +using System; +using System.Linq; + +using Avalonia; using Avalonia.Controls; using Avalonia.Input; using Avalonia.Interactivity; +using Avalonia.LogicalTree; +using Avalonia.Threading; +using Avalonia.VisualTree; using ICSharpCode.ILSpy.ViewModels; @@ -28,12 +35,16 @@ namespace ICSharpCode.ILSpy.Views { /// /// Thin renderer over . All cross-language / - /// cross-decompile state lives on the ViewModel, so the View has no event subscriptions - /// and no awareness of language switches — it just binds. The two pointer / keyboard - /// handlers below translate user gestures into ViewModel command invocations. + /// cross-decompile state lives on the ViewModel, so the View holds no state of its own — + /// it just binds. The pointer / keyboard handlers below translate user gestures into + /// ViewModel command invocations, and the only ViewModel event the View listens to + /// (bounded by attach/detach, so a discarded view never leaks through the long-lived + /// ViewModel) is the purely visual "center the selection" request after filter changes. /// public partial class DebugSteps : UserControl { + DebugStepsPaneModel? attachedModel; + public DebugSteps() { InitializeComponent(); @@ -42,6 +53,79 @@ namespace ICSharpCode.ILSpy.Views // Enter on a group row. Intercept in the tunnel phase, ahead of the item, so Enter and // Shift+Enter drive the show-state commands for both leaf and group steps. StepsTree.AddHandler(InputElement.KeyDownEvent, OnTreeKeyDown, RoutingStrategies.Tunnel, handledEventsToo: true); + // DataContext usually arrives before the control enters the logical tree; defer to + // OnAttachedToLogicalTree in that case (and after a detach), so the subscription + // stays strictly within the attach/detach bracket and a view that never attaches + // cannot leak through the long-lived pane model. + DataContextChanged += (_, _) => { + if (((ILogical)this).IsAttachedToLogicalTree) + AttachModel(); + }; + } + + protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) + { + base.OnAttachedToLogicalTree(e); + AttachModel(); + } + + protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs e) + { + base.OnDetachedFromLogicalTree(e); + DetachModel(); + } + + void AttachModel() + { + var model = DataContext as DebugStepsPaneModel; + if (ReferenceEquals(attachedModel, model)) + return; + DetachModel(); + attachedModel = model; + if (model != null) + model.SelectionRevealRequested += OnSelectionRevealRequested; + } + + void DetachModel() + { + if (attachedModel != null) + { + attachedModel.SelectionRevealRequested -= OnSelectionRevealRequested; + attachedModel = null; + } + } + + void OnSelectionRevealRequested(object? sender, EventArgs e) + { + // Containers for rows the filter just expanded materialise in the next layout pass; + // Loaded priority runs after it, so the container geometry is valid when we measure. + Dispatcher.UIThread.Post(CenterSelectedStep, DispatcherPriority.Loaded); + } + + void CenterSelectedStep() + { + if (attachedModel?.SelectedStep is not { } selected) + return; + if (StepsTree.TreeContainerFromItem(selected) is not Control container) + { + StepsTree.UpdateLayout(); + if (StepsTree.TreeContainerFromItem(selected) is not Control lateContainer) + return; + container = lateContainer; + } + var scrollViewer = StepsTree.GetVisualDescendants().OfType().FirstOrDefault(); + if (scrollViewer == null) + return; + // An expanded group's container spans its whole subtree; center on the header row. + var header = container.GetVisualDescendants().OfType() + .FirstOrDefault(c => c.Name == "PART_Header") ?? container; + if (header.TranslatePoint(new Point(0, header.Bounds.Height / 2), scrollViewer) is not { } rowCenter) + return; + double delta = rowCenter.Y - scrollViewer.Viewport.Height / 2; + double maxOffset = Math.Max(0, scrollViewer.Extent.Height - scrollViewer.Viewport.Height); + scrollViewer.Offset = new Vector( + scrollViewer.Offset.X, + Math.Clamp(scrollViewer.Offset.Y + delta, 0, maxOffset)); } void OnTreeDoubleTapped(object? sender, TappedEventArgs e)