Browse Source

Route every tree selection to the one preview tab

The preview tab only got reused when it was the active document; selecting a
node while a frozen tab was active spawned a fresh preview beside it, so
previews piled up. Make 'the One' an invariant: tree selections always
find-or-create the single preview tracked by factory.MainTab and reuse it
wherever it sits (activating it), only forging a new one when it was frozen
away or closed. The fresh One is inserted at documents-dock index 0 (leftmost)
rather than appended, so the preview always sits at the front.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3755/head
Siegfried Pammer 1 month ago
parent
commit
0f88723bec
  1. 79
      ILSpy.Tests/Docking/PreviewTabPromotionTests.cs
  2. 56
      ILSpy/Docking/DockWorkspace.cs
  3. 24
      ILSpy/Docking/ILSpyDockFactory.cs

79
ILSpy.Tests/Docking/PreviewTabPromotionTests.cs

@ -544,19 +544,21 @@ public class PreviewTabPromotionTests
} }
[AvaloniaTest] [AvaloniaTest]
public async Task Tree_Selection_While_Frozen_Tab_Active_Opens_A_New_Preview_Tab() public async Task Tree_Selection_While_Frozen_Tab_Active_Reuses_The_One()
{ {
// "Frozen" covers: (a) user-frozen tabs (IsPreview=false via FreezeCurrentTab), // Exactly one preview tab ("the One"): even when a frozen tab (carve-out / Options /
// (b) carve-out tabs (born IsPreview=false via OpenNodeInNewTab), and (c) static // About) is the active document, a tree-node selection routes to the EXISTING One --
// content tabs (Options, About) whose Content is a static viewmodel. Tree-node // reuse + activate it -- and never spawns a second preview. This is the regression test
// selections while any of those are active must spawn a fresh preview tab and // for the stray-preview bug (selecting while a frozen tab was active used to pile up
// route the new content there. This test uses a carve-out as a stand-in for the // previews).
// frozen-tab family — the IsWritablePreview check returns null for all three.
var (_, vm) = await TestHarness.BootAsync(3); var (_, vm) = await TestHarness.BootAsync(3);
var factory = (ILSpyDockFactory)vm.DockWorkspace.Factory; var factory = (ILSpyDockFactory)vm.DockWorkspace.Factory;
// Open a carve-out (born frozen) and let it become the active dockable. // The One exists from boot.
var theOne = factory.MainTab!;
theOne.IsPreview.Should().BeTrue("baseline: the One is a preview tab");
// Open a carve-out (born frozen) and make it the active dockable.
var typeA = vm.AssemblyTreeModel.FindNode<TypeTreeNode>( var typeA = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Linq", "System.Linq", "System.Linq.Enumerable"); "System.Linq", "System.Linq", "System.Linq.Enumerable");
vm.DockWorkspace.OpenNodeInNewTab(typeA); vm.DockWorkspace.OpenNodeInNewTab(typeA);
@ -571,25 +573,56 @@ public class PreviewTabPromotionTests
var tabCountBefore = vm.DockWorkspace.Documents!.VisibleDockables!.Count; var tabCountBefore = vm.DockWorkspace.Documents!.VisibleDockables!.Count;
// Select a different tree node — the carve-out is frozen, so this must NOT // Select a different tree node while the frozen carve-out is active.
// overwrite it.
var typeB = vm.AssemblyTreeModel.FindNode<TypeTreeNode>( var typeB = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Private.Uri", "System", "System.Uri"); "System.Private.Uri", "System", "System.Uri");
vm.AssemblyTreeModel.SelectNode(typeB); vm.AssemblyTreeModel.SelectNode(typeB);
vm.DockWorkspace.SettleSelection(); vm.DockWorkspace.SettleSelection();
TestCapture.Step("type-b-opens-new-preview"); TestCapture.Step("type-b-reuses-the-one");
vm.DockWorkspace.Documents!.VisibleDockables!.Count.Should().Be(tabCountBefore + 1, // No new tab -- the existing One was reused, not a second preview spawned.
"selecting a tree node while a frozen tab is active must spawn a new preview tab"); vm.DockWorkspace.Documents!.VisibleDockables!.Count.Should().Be(tabCountBefore,
"selecting while a frozen tab is active must reuse the One, not spawn a second preview");
ReferenceEquals(factory.MainTab, theOne).Should().BeTrue(
"the One must be the same instance -- no fresh preview spawned");
ReferenceEquals(theOne.SourceNode, typeB).Should().BeTrue(
"the One now hosts the just-selected node");
// The frozen carve-out is untouched, and the One was brought to the front.
ReferenceEquals(carveOut.SourceNode, typeA).Should().BeTrue( ReferenceEquals(carveOut.SourceNode, typeA).Should().BeTrue(
"the frozen carve-out tab must keep its original content"); "the frozen carve-out must keep its original content");
ReferenceEquals(vm.DockWorkspace.Documents.ActiveDockable, theOne).Should().BeTrue(
var newPreview = factory.MainTab!; "reusing the One activates it");
newPreview.Should().NotBeSameAs(carveOut, }
"the freshly-spawned preview must be a separate tab from the frozen one");
newPreview.IsPreview.Should().BeTrue( [AvaloniaTest]
"the freshly-spawned tab is itself a preview tab"); public async Task The_Preview_Is_Always_At_Index_0()
ReferenceEquals(newPreview.SourceNode, typeB).Should().BeTrue( {
"the new preview tab must host the just-selected node's content"); // The One lives at documents-dock index 0. After it is frozen, the next selection forges
// a fresh One -- which must again land at index 0 (the frozen ex-One slides to the right).
var (_, vm) = await TestHarness.BootAsync(3);
var factory = (ILSpyDockFactory)vm.DockWorkspace.Factory;
var docs = vm.DockWorkspace.Documents!;
var typeA = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Linq", "System.Linq", "System.Linq.Enumerable");
vm.AssemblyTreeModel.SelectNode(typeA);
vm.DockWorkspace.SettleSelection();
docs.VisibleDockables!.IndexOf(factory.MainTab!).Should().Be(0,
"the One is the leftmost document tab");
// Freeze it, then select a different node -> a fresh One must appear at index 0.
vm.DockWorkspace.FreezeCurrentTab();
var frozen = factory.MainTab!;
var typeB = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Private.Uri", "System", "System.Uri");
vm.AssemblyTreeModel.SelectNode(typeB);
vm.DockWorkspace.SettleSelection();
ReferenceEquals(factory.MainTab, frozen).Should().BeFalse("a fresh One was forged");
factory.MainTab!.IsPreview.Should().BeTrue("the fresh One is a preview tab");
docs.VisibleDockables!.IndexOf(factory.MainTab!).Should().Be(0,
"the fresh One must be created at index 0, leftmost");
docs.VisibleDockables!.IndexOf(frozen).Should().BeGreaterThan(0,
"the frozen ex-One slides to the right of the new One");
} }
} }

56
ILSpy/Docking/DockWorkspace.cs

@ -706,25 +706,10 @@ namespace ILSpy.Docking
} }
lastShownNodes = nodes; lastShownNodes = nodes;
// Pick or spawn the target tab. If the currently-active document tab is a // Route to THE single preview tab ("the One") and replace its content in place.
// writable preview ContentTabPage we replace its content in place. Anything var main = GetOrCreateThePreview();
// else — frozen tab, Options page, About page, no Documents dock — counts as if (main == null)
// frozen and gets a brand-new preview tab spawned beside it. return;
ContentTabPage? main;
if (IsWritablePreview(factory.Documents?.ActiveDockable) is ContentTabPage activePreview)
{
main = activePreview;
factory.MainTab = main;
}
else
{
main = factory.CreateAndAttachPreviewTab();
if (main == null)
return;
// Fresh tab needs a fresh DecompilerTabPageModel — the cached one (if any)
// belongs to a previous tab and must not be re-assigned across tabs.
decompilerContent = null;
}
// Tree-node selections always reuse the single document slot. The Document // Tree-node selections always reuse the single document slot. The Document
// instance never changes; its inner Content swaps between viewmodels. The // instance never changes; its inner Content swaps between viewmodels. The
@ -764,13 +749,32 @@ namespace ILSpy.Docking
ActivateMainTabIfNeeded(main); ActivateMainTabIfNeeded(main);
} }
// Returns the active document tab if it's a writable preview ContentTabPage — meaning // The "exactly one preview tab" rule: tree selections always route to the single preview
// IsPreview=true AND not hosting static-content (Options, About). Otherwise returns // ("the One"), tracked by factory.MainTab. Reuse it wherever it sits -- even if a frozen
// null, signalling that ShowSelectedNode must spawn a fresh preview tab instead of // tab is currently active, ActivateMainTabIfNeeded brings it forward -- as long as it is
// overwriting the active one. Options pages and About pages are born with // still a writable preview AND still lives in the dock. Otherwise the One was frozen-away
// IsPreview=false (carve-out semantics) AND carry IsStaticContent=true on their // or closed, so forge a fresh one at index 0. This is what stops a selection-while-frozen
// content viewmodel — either flag alone would suffice, both check guards against // from spawning a SECOND preview.
// drift. ContentTabPage? GetOrCreateThePreview()
{
if (factory.Documents is not { } docs)
return null;
if (factory.MainTab is { } current
&& IsWritablePreview(current) is not null
&& docs.VisibleDockables?.Contains(current) == true)
{
return current;
}
// A fresh One needs a fresh DecompilerTabPageModel -- the cached one (if any) belongs
// to a previous tab and must not be re-assigned across tabs.
decompilerContent = null;
return factory.CreateThePreviewTab();
}
// Pure predicate: returns the tab if it is a writable preview ContentTabPage -- IsPreview
// true AND not hosting static content (Options, About). Otherwise null. Options/About are
// born frozen (IsPreview=false) AND carry IsStaticContent=true; either flag alone would
// suffice, both guard against drift.
static ContentTabPage? IsWritablePreview(IDockable? dockable) static ContentTabPage? IsWritablePreview(IDockable? dockable)
{ {
if (dockable is not ContentTabPage tab) if (dockable is not ContentTabPage tab)

24
ILSpy/Docking/ILSpyDockFactory.cs

@ -53,8 +53,8 @@ namespace ILSpy.Docking
/// (<see cref="ContentTabPage.IsPreview"/> = false). Returns the frozen tab, or /// (<see cref="ContentTabPage.IsPreview"/> = false). Returns the frozen tab, or
/// <see langword="null"/> when there's nothing to freeze (no current MainTab, MainTab /// <see langword="null"/> when there's nothing to freeze (no current MainTab, MainTab
/// already frozen). Does NOT spawn a new preview tab — the next selection change /// already frozen). Does NOT spawn a new preview tab — the next selection change
/// that finds the active tab frozen will spawn one lazily via /// finds no preview to reuse and forges a fresh One lazily via
/// <see cref="CreateAndAttachPreviewTab"/>. /// <see cref="CreateThePreviewTab"/>.
/// </summary> /// </summary>
public ContentTabPage? FreezeCurrentMainTab() public ContentTabPage? FreezeCurrentMainTab()
{ {
@ -65,22 +65,20 @@ namespace ILSpy.Docking
} }
/// <summary> /// <summary>
/// Creates a fresh preview <see cref="ContentTabPage"/>, attaches it to /// Creates THE single preview <see cref="ContentTabPage"/> ("the One"), attaches it to
/// <see cref="Documents"/>, and sets <see cref="MainTab"/> to it. Used when a /// <see cref="Documents"/> at index 0, and sets <see cref="MainTab"/> to it. Called when a
/// tree-node selection changes while the active tab is frozen (frozen, Options, /// tree-node selection has no preview to reuse (the previous One was frozen, closed, or
/// About, …) — a brand-new preview slot is needed to host the new content. /// none exists yet). Returns the new tab, or <see langword="null"/> if
/// Returns the new tab, or <see langword="null"/> if <see cref="Documents"/> is /// <see cref="Documents"/> is missing.
/// missing.
/// </summary> /// </summary>
public ContentTabPage? CreateAndAttachPreviewTab() public ContentTabPage? CreateThePreviewTab()
{ {
if (Documents is not { } docs) if (Documents is not { } docs)
return null; return null;
var fresh = new ContentTabPage { Title = "(no selection)" }; var fresh = new ContentTabPage { Title = "(no selection)" };
// Use the framework's AddDockable so owner/factory wiring matches every other // The One always occupies index 0 (leftmost); frozen tabs live to its right. Use
// add path; default insertion is at the end (rightmost — VS convention for the // InsertDockable (not AddDockable, which appends) so it lands at the front.
// preview slot). InsertDockable(docs, fresh, 0);
AddDockable(docs, fresh);
MainTab = fresh; MainTab = fresh;
return fresh; return fresh;
} }

Loading…
Cancel
Save