Skip to content

Commit 2dc5b14

Browse files
fix(treeview): stop reconcile from clobbering runtime expansion; keep TreeView<T> hosting declarative
Two expand/collapse fixes: 1. Legacy TreeView (the "expand flashes open then shut" regression that repros on main): DiffTreeViewNodes reset each live node's IsExpanded to the static TreeViewNodeData value on every reconcile. When a reconcile fires after the user toggles a node — e.g. an OnExpanding lazy-load that updates state, or any unrelated state change while nodes are data-expanded — it snapped the node back to the data's state. Now expansion is treated as uncontrolled: the data value is only applied when it actually changes between renders, never clobbering the user's (or WinUI's native) toggle. 2. TreeView<T>: applied the same uncontrolled-expansion rule, and reverted the experimental ContainerContentChanging hosting back to the declarative {Binding Content} template. The imperative host/recycle handler perturbed WinUI's synchronous expand pass (manifested as expand opening then closing, and child-click collapsing the parent); the binding never perturbs native expand/collapse. Tests: legacy + typed expansion-not-clobbered guards, collapse/expand cycle, expand-collapsed-node, constrained-viewport expand. Full suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b7d25af commit 2dc5b14

5 files changed

Lines changed: 110 additions & 139 deletions

File tree

src/Reactor/Core/Reconciler.Mount.cs

Lines changed: 10 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1990,9 +1990,12 @@ internal WinUI.TreeView MountTemplatedTreeView(TemplatedTreeViewElementBase tv,
19901990
CanDragItems = tv.CanDragItems,
19911991
AllowDrop = tv.AllowDrop,
19921992
CanReorderItems = tv.CanReorderItems,
1993-
// Unbound shell; per-node views are hosted/released through the
1994-
// internal list's ContainerContentChanging (see Loaded below).
1995-
ItemTemplate = SharedContentControlTemplate.Value,
1993+
// Each node's mounted view lives in TreeViewNode.Content and is
1994+
// surfaced by the {Binding Content} ContentControl template — the
1995+
// node-mode default template can only stringify (issue #447). A
1996+
// declarative binding (rather than imperative ContainerContentChanging
1997+
// hosting) keeps native expand/collapse from being perturbed.
1998+
ItemTemplate = TreeViewContentElementTemplate.Value,
19961999
};
19972000

19982001
foreach (var root in tv.GetRoots())
@@ -2005,46 +2008,18 @@ internal WinUI.TreeView MountTemplatedTreeView(TemplatedTreeViewElementBase tv,
20052008
if (tv.HasExpanding)
20062009
treeView.Expanding += TemplatedTreeExpanding;
20072010

2008-
// The TreeViewList (internal ListView, template part "ListControl")
2009-
// owns container realization/recycling. Hook its ContainerContentChanging
2010-
// so a node's mounted view is hosted on realize and its visual parent
2011-
// released on recycle — the only way to keep a shared live element from
2012-
// blanking out across collapse/expand under virtualization.
2013-
treeView.Loaded += static (s, _) =>
2014-
{
2015-
var tree = (WinUI.TreeView)s;
2016-
if (FindInnerList(tree) is not { } list) return;
2017-
// Idempotent: method-group -= removes the prior subscription if the
2018-
// tree reloaded, so we never double-host.
2019-
list.ContainerContentChanging -= TemplatedTreeContainerContentChanging;
2020-
list.ContainerContentChanging += TemplatedTreeContainerContentChanging;
2021-
2022-
// Fill containers already realized before we subscribed.
2023-
for (int i = 0; i < list.Items.Count; i++)
2024-
if (list.ContainerFromIndex(i) is WinUI.TreeViewItem tvi
2025-
&& tvi.ContentTemplateRoot is ContentControl cc
2026-
&& cc.Content is null
2027-
&& list.Items[i] is WinUI.TreeViewNode node
2028-
&& GetTreeNodeView(node) is UIElement view)
2029-
{
2030-
DetachContentHost(view);
2031-
cc.Content = view;
2032-
}
2033-
};
2034-
20352011
ApplySetters(tv.SettersErased, treeView);
20362012
return treeView;
20372013
}
20382014

20392015
private WinUI.TreeViewNode BuildTemplatedTreeNode(
20402016
TemplatedTreeViewElementBase tv, object item, Action requestRerender)
20412017
{
2042-
// Content holds the data item (identity/stringification); the mounted
2043-
// view rides on an attached property and is hosted on demand by the
2044-
// ContainerContentChanging handler.
2045-
var node = new WinUI.TreeViewNode { IsExpanded = tv.GetIsExpanded(item), Content = item };
2018+
// Content = the mounted view (surfaced by {Binding Content}); the
2019+
// originating T rides on an attached property for event resolution.
2020+
var node = new WinUI.TreeViewNode { IsExpanded = tv.GetIsExpanded(item) };
2021+
node.Content = Mount(tv.BuildView(item), requestRerender);
20462022
SetTreeNodeItem(node, item);
2047-
SetTreeNodeView(node, Mount(tv.BuildView(item), requestRerender));
20482023

20492024
var children = tv.GetChildren(item);
20502025
if (children is not null)
@@ -2053,55 +2028,6 @@ private WinUI.TreeViewNode BuildTemplatedTreeNode(
20532028
return node;
20542029
}
20552030

2056-
// Hosts/releases the per-node mounted view as the internal list realizes
2057-
// and recycles TreeViewItem containers. Not setting args.Handled so the
2058-
// TreeViewList still applies its own chrome (expand glyph, indentation).
2059-
//
2060-
// The host is DEFERRED to a low-priority dispatch (after the synchronous
2061-
// prepare/layout pass) because mutating container content mid-prepare
2062-
// perturbs layout and can recycle/re-prepare the expanding node's own
2063-
// container with a stale IsExpanded — TreeViewList then enqueues a sync
2064-
// that snaps the node shut ("expand opens & closes"). The recycle release
2065-
// stays synchronous (always safe). The deferred host re-validates that the
2066-
// container still belongs to the same node (cc.DataContext) so a container
2067-
// recycled to another node in the meantime is never mis-hosted.
2068-
private static void TemplatedTreeContainerContentChanging(
2069-
Microsoft.UI.Xaml.Controls.ListViewBase sender,
2070-
Microsoft.UI.Xaml.Controls.ContainerContentChangingEventArgs args)
2071-
{
2072-
if (args.ItemContainer?.ContentTemplateRoot is not ContentControl cc) return;
2073-
if (args.InRecycleQueue)
2074-
{
2075-
cc.Content = null; // release the element's visual parent; view persists on the node
2076-
return;
2077-
}
2078-
if (args.Item is not WinUI.TreeViewNode node) return;
2079-
2080-
var dq = cc.DispatcherQueue;
2081-
if (dq is null) { HostNodeView(cc, node); return; }
2082-
dq.TryEnqueue(Microsoft.UI.Dispatching.DispatcherQueuePriority.Low, () => HostNodeView(cc, node));
2083-
}
2084-
2085-
private static void HostNodeView(ContentControl cc, WinUI.TreeViewNode node)
2086-
{
2087-
// Container may have been recycled to a different node before this ran.
2088-
if (!ReferenceEquals(cc.DataContext, node)) return;
2089-
if (GetTreeNodeView(node) is not UIElement view || ReferenceEquals(cc.Content, view)) return;
2090-
DetachContentHost(view);
2091-
cc.Content = view;
2092-
}
2093-
2094-
// Releases a view from whatever ContentControl currently hosts it, so it
2095-
// can be parented elsewhere (a UIElement may have only one visual parent).
2096-
private static void DetachContentHost(UIElement view)
2097-
{
2098-
DependencyObject? p = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetParent(view);
2099-
while (p is not null and not ContentControl)
2100-
p = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetParent(p);
2101-
if (p is ContentControl host && host.Content is not null)
2102-
host.Content = null;
2103-
}
2104-
21052031
private static Microsoft.UI.Xaml.Controls.ListViewBase? FindInnerList(DependencyObject root)
21062032
{
21072033
int n = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetChildrenCount(root);

src/Reactor/Core/Reconciler.Update.cs

Lines changed: 18 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4017,7 +4017,13 @@ private void DiffTreeViewNodes(
40174017
var liveNode = match.Node;
40184018
var oldNodeData = oldData[match.OldIdx];
40194019

4020-
if (liveNode.IsExpanded != nd.IsExpanded)
4020+
// Uncontrolled expansion: only push the data value when it
4021+
// actually changed between renders. Otherwise leave the node's
4022+
// IsExpanded alone, so a reconcile triggered AFTER the user
4023+
// expands/collapses (e.g. OnExpanding lazy-load, or any state
4024+
// change) doesn't snap the node back to the static data value —
4025+
// the "expand flashes open then shut" regression.
4026+
if (oldNodeData.IsExpanded != nd.IsExpanded && liveNode.IsExpanded != nd.IsExpanded)
40214027
liveNode.IsExpanded = nd.IsExpanded;
40224028

40234029
ReconcileTreeNodeContent(liveNode, oldNodeData, nd, requestRerender);
@@ -4168,10 +4174,13 @@ private void DiffTemplatedTreeNodes(
41684174
int cur = liveNodes.IndexOf(node);
41694175
if (cur != i)
41704176
{
4171-
// A move recycles + re-realizes the container; the
4172-
// ContainerContentChanging handler re-hosts the view.
4177+
// Move: release the live view's parent before re-realizing
4178+
// so the bound ContentControl can re-host it at the new slot.
4179+
var content = node.Content as UIElement;
4180+
if (content is not null) node.Content = null;
41734181
liveNodes.RemoveAt(cur);
41744182
liveNodes.Insert(i, node);
4183+
if (content is not null) node.Content = content;
41754184
}
41764185
ReconcileTemplatedNodeView(node, o, n, newItem, requestRerender);
41774186
DiffTemplatedTreeNodes(
@@ -4195,29 +4204,19 @@ private void ReconcileTemplatedNodeView(
41954204
var oldItem = GetTreeNodeItem(node);
41964205
var newView = n.BuildView(newItem);
41974206
var oldView = oldItem is not null ? o.BuildView(oldItem) : null;
4198-
var existing = GetTreeNodeView(node);
41994207

4200-
if (oldView is not null && existing is not null && CanUpdate(oldView, newView))
4208+
if (oldView is not null && node.Content is UIElement existing && CanUpdate(oldView, newView))
42014209
{
4202-
// In-place update mutates the live element; a realized container is
4203-
// already showing it, so nothing else to do unless it's replaced.
42044210
var replacement = Update(oldView, newView, existing, requestRerender);
4205-
if (replacement is not null && !ReferenceEquals(existing, replacement))
4206-
{
4207-
SetTreeNodeView(node, replacement);
4208-
RehostReplacedView(existing, replacement);
4209-
}
4211+
if (replacement is not null && !ReferenceEquals(node.Content, replacement))
4212+
node.Content = replacement;
42104213
}
42114214
else
42124215
{
4213-
if (existing is not null) UnmountChild(existing);
4214-
var mounted = Mount(newView, requestRerender);
4215-
SetTreeNodeView(node, mounted);
4216-
RehostReplacedView(existing, mounted);
4216+
if (node.Content is UIElement stale) UnmountChild(stale);
4217+
node.Content = Mount(newView, requestRerender);
42174218
}
42184219

4219-
node.Content = newItem;
4220-
42214220
// Expansion is uncontrolled: only push the selector value when it
42224221
// actually CHANGES between renders (developer-driven control). Otherwise
42234222
// leave node.IsExpanded alone so the user's own expand/collapse is never
@@ -4230,22 +4229,9 @@ private void ReconcileTemplatedNodeView(
42304229
SetTreeNodeItem(node, newItem);
42314230
}
42324231

4233-
// If a node's view was replaced (CanUpdate false) while its container is
4234-
// realized, swap the new view into the ContentControl currently hosting the
4235-
// old one so the change is visible without waiting for a recycle.
4236-
private static void RehostReplacedView(UIElement? oldView, UIElement newView)
4237-
{
4238-
if (oldView is null) return;
4239-
DependencyObject? p = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetParent(oldView);
4240-
while (p is not null and not WinUI.ContentControl)
4241-
p = Microsoft.UI.Xaml.Media.VisualTreeHelper.GetParent(p);
4242-
if (p is WinUI.ContentControl host && ReferenceEquals(host.Content, oldView))
4243-
host.Content = newView;
4244-
}
4245-
42464232
private void UnmountTemplatedTreeNode(WinUI.TreeViewNode node)
42474233
{
4248-
if (GetTreeNodeView(node) is UIElement ui) UnmountChild(ui);
4234+
if (node.Content is UIElement ui) UnmountChild(ui);
42494235
foreach (var child in node.Children)
42504236
UnmountTemplatedTreeNode(child);
42514237
}

src/Reactor/Core/Reconciler.cs

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -648,28 +648,6 @@ internal static void SetTreeNodeItem(WinUI.TreeViewNode node, object? item) =>
648648
internal static object? GetTreeNodeItem(WinUI.TreeViewNode node) =>
649649
node.GetValue(TreeNodeItemAttached.ItemProperty);
650650

651-
// Holds the node's mounted view for the typed TreeView<T>. The view is
652-
// mounted ONCE and persists here across collapse/expand (so component
653-
// state survives); the internal list's ContainerContentChanging hosts it
654-
// into the realized container's ContentControl on realize and releases it
655-
// (Content = null) on recycle. Without that release the shared element
656-
// keeps a stale visual parent and the next realization renders blank
657-
// (issue #447 follow-up: every-other expand/collapse blanked rows).
658-
private static class TreeNodeViewAttached
659-
{
660-
public static readonly DependencyProperty MountedViewProperty =
661-
DependencyProperty.RegisterAttached(
662-
"ReactorTreeMountedView",
663-
typeof(UIElement),
664-
typeof(TreeNodeViewAttached),
665-
new PropertyMetadata(null));
666-
}
667-
668-
internal static void SetTreeNodeView(WinUI.TreeViewNode node, UIElement? view) =>
669-
node.SetValue(TreeNodeViewAttached.MountedViewProperty, view);
670-
671-
internal static UIElement? GetTreeNodeView(WinUI.TreeViewNode node) =>
672-
node.GetValue(TreeNodeViewAttached.MountedViewProperty) as UIElement;
673651

674652
// ════════════════════════════════════════════════════════════════════
675653
// ReactorAttached.StateProperty (ReactorState)
@@ -2043,7 +2021,7 @@ private void UnmountRecursive(UIElement control)
20432021

20442022
private void UnmountTemplatedTreeNodeContents(WinUI.TreeViewNode node)
20452023
{
2046-
if (GetTreeNodeView(node) is UIElement ui) UnmountRecursive(ui);
2024+
if (node.Content is UIElement ui) UnmountRecursive(ui);
20472025
foreach (var child in node.Children)
20482026
UnmountTemplatedTreeNodeContents(child);
20492027
}

tests/Reactor.AppTests.Host/SelfTest/Fixtures/TemplatedTreeViewFixtures.cs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,83 @@ public override async Task RunAsync()
288288
}
289289
}
290290

291+
// ── 3e. Constrained-height expand must stick (virtualization race) ───────
292+
// Tiny viewport + many children forces container recycling; reproduces the
293+
// GUI "expand opens & closes immediately" report.
294+
internal sealed class ConstrainedExpandSticks(Harness h) : SelfTestFixtureBase(h)
295+
{
296+
public override async Task RunAsync()
297+
{
298+
var kids = new Node[20];
299+
for (int i = 0; i < kids.Length; i++) kids[i] = new Node($"k{i}", $"KID_{i}");
300+
301+
var host = H.CreateHost();
302+
host.Mount(_ => (TreeView<Node>(
303+
items: new[] { new Node("root", "ROOT", kids) },
304+
childrenSelector: Children,
305+
viewBuilder: n => Button(n.Label)) with
306+
{
307+
IsExpanded = _ => false, // root starts collapsed
308+
}).Height(90));
309+
310+
await Harness.Render();
311+
var tv = H.FindControl<TreeView>(_ => true);
312+
H.Check("TTV_Constrained_Mounted", tv is not null && tv.RootNodes.Count == 1);
313+
if (tv is null || tv.RootNodes.Count == 0)
314+
{
315+
H.Check("TTV_Constrained_StaysExpanded", false);
316+
return;
317+
}
318+
319+
var root = tv.RootNodes[0];
320+
root.IsExpanded = true;
321+
await Harness.Render();
322+
await Harness.Render();
323+
await Harness.Render();
324+
325+
H.Check("TTV_Constrained_StaysExpanded", root.IsExpanded);
326+
}
327+
}
328+
329+
// ── Legacy TreeView (TreeViewElement): reconcile must not clobber the
330+
// user's runtime expansion back to the static data value (the
331+
// "expand flashes open then shut" regression — repros on main).
332+
internal sealed class LegacyExpansionNotClobbered(Harness h) : SelfTestFixtureBase(h)
333+
{
334+
public override async Task RunAsync()
335+
{
336+
Action<int>? bump = null;
337+
var host = H.CreateHost();
338+
host.Mount(ctx =>
339+
{
340+
var (_, set) = ctx.UseState(0);
341+
bump = set;
342+
// Data default IsExpanded = false (collapsed).
343+
return new TreeViewElement(new[]
344+
{
345+
new TreeViewNodeData("root", new[] { new TreeViewNodeData("child") }),
346+
});
347+
});
348+
349+
await Harness.Render();
350+
var tv = H.FindControl<TreeView>(_ => true);
351+
H.Check("LegacyTV_Mounted", tv is not null && tv.RootNodes.Count == 1);
352+
if (tv is null || tv.RootNodes.Count == 0)
353+
{
354+
H.Check("LegacyTV_StaysExpanded", false);
355+
return;
356+
}
357+
358+
var root = tv.RootNodes[0];
359+
root.IsExpanded = true; // user expands at runtime
360+
bump?.Invoke(1); // unrelated state change → reconcile (data unchanged)
361+
await Harness.Render();
362+
363+
// Old code reset IsExpanded to the data value (false) → collapse.
364+
H.Check("LegacyTV_StaysExpanded", root.IsExpanded);
365+
}
366+
}
367+
291368
// ── 4. Value-type T (exercises the boxing Project path) ──────────────────
292369
internal sealed class ValueTypeItems(Harness h) : SelfTestFixtureBase(h)
293370
{

tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,6 +1261,8 @@ internal static class SelfTestFixtureRegistry
12611261
"TTV_ExpansionNotClobbered",
12621262
"TTV_CollapseExpandCycle",
12631263
"TTV_ExpandCollapsedNode",
1264+
"TTV_ConstrainedExpandSticks",
1265+
"TTV_LegacyExpansionNotClobbered",
12641266
"TTV_ValueTypeItems",
12651267
];
12661268

@@ -1273,6 +1275,8 @@ internal static class SelfTestFixtureRegistry
12731275
"TTV_ExpansionNotClobbered" => new TemplatedTreeViewFixtures.ExpansionNotClobbered(harness),
12741276
"TTV_CollapseExpandCycle" => new TemplatedTreeViewFixtures.CollapseExpandCycle(harness),
12751277
"TTV_ExpandCollapsedNode" => new TemplatedTreeViewFixtures.ExpandCollapsedNode(harness),
1278+
"TTV_ConstrainedExpandSticks" => new TemplatedTreeViewFixtures.ConstrainedExpandSticks(harness),
1279+
"TTV_LegacyExpansionNotClobbered" => new TemplatedTreeViewFixtures.LegacyExpansionNotClobbered(harness),
12761280
"TTV_ValueTypeItems" => new TemplatedTreeViewFixtures.ValueTypeItems(harness),
12771281
"ErrorBoundary_CatchesRenderError" => new ErrorBoundaryFixtures.CatchesRenderError(harness),
12781282
"ErrorBoundary_Recovery" => new ErrorBoundaryFixtures.Recovery(harness),

0 commit comments

Comments
 (0)