fix(treeview): typed data-driven TreeView<T> with per-container hosting (#447)#453
Open
codemonkeychris wants to merge 4 commits into
Open
fix(treeview): typed data-driven TreeView<T> with per-container hosting (#447)#453codemonkeychris wants to merge 4 commits into
codemonkeychris wants to merge 4 commits into
Conversation
…ng (#447) A node-mode WinUI TreeView stringifies TreeViewNode.Content and cannot host a pre-built UIElement, so TreeViewNodeData.ContentElement rendered blank rows (#447). Add a typed, data-driven TreeView<T> — the hierarchical peer of ListView<T> — that renders each node from a data -> Element viewBuilder (the WinUI ItemTemplate equivalent), with heterogeneous nodes handled by a switch in the viewBuilder (the ItemTemplateSelector pattern). Hosting mirrors the typed ListView<T>: the ItemTemplate is an empty ContentControl shell and each node view is mounted imperatively into the realized container via the internal TreeViewList's ContainerContentChanging (fresh mount on realize, unmount on recycle). This keeps expand/collapse robust under container recycling — fixing the "every other expand/collapse blanks the first child row(s)" regression that the earlier declarative {Binding Content} approach suffered (a recycled container retained the element's visual parent). node.Content holds the data item; the ItemInvoked and Expanding trampolines read T back from it. TreeViewNodeData.ContentElement is marked [Obsolete] pointing at TreeView<T>; the legacy path stays functional (CS0618 suppressed at internal use sites). - Element.cs: TemplatedTreeViewElementBase (object-erased base) + TemplatedTreeViewElement<T>; OwnPropsEqual arm. - Dsl.cs: TreeView<T> factory overloads (explicit + IReactorKeyed). - Reconciler: empty-shell ItemTemplate, ContainerContentChanging hosting on the internal TreeViewList, keyed in-place node diff that reconciles only realized containers, full-tree unmount walk. - Samples: DataTemplateDemo section 4 migrated to a discriminated PetNode model. - Tests: TemplatedTreeViewFixtures (TTV_) — render, heterogeneous templates, keyed update, expand/collapse cycles, events, value-type T, unmount. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a typed, data-driven TreeView<T> that renders rich per-node visuals via a viewBuilder, aligning TreeView with existing typed collection controls and avoiding legacy TreeViewNodeData.ContentElement hosting issues.
Changes:
- Introduces
TemplatedTreeViewElement<T>and DSL factory overloads. - Adds reconciler mount/update/unmount support for per-container TreeView node hosting.
- Migrates sample/demo usage and adds self-test fixtures for rendering, updates, events, value types, expansion, and unmount.
Show a summary per file
| File | Description |
|---|---|
src/Reactor/Core/Element.cs |
Adds typed TreeView element model and obsoletes legacy ContentElement. |
src/Reactor/Elements/Dsl.cs |
Adds TreeView<T> factory overloads. |
src/Reactor/Core/Reconciler.Mount.cs |
Mounts typed TreeView nodes and hosts realized container content. |
src/Reactor/Core/Reconciler.Update.cs |
Adds keyed hierarchical update/reconcile for typed TreeView. |
src/Reactor/Core/Reconciler.cs |
Adds typed TreeView container cache and unmount traversal. |
src/Reactor/Core/V1Protocol/ChildrenStrategy.cs |
Suppresses obsolete warnings for legacy TreeView content reads. |
samples/Reactor.TestApp/Demos/DataTemplateDemo.cs |
Updates demo to use typed TreeView<T>. |
tests/Reactor.AppTests.Host/SelfTest/* |
Registers and adds typed TreeView self-test fixtures. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/9 changed files
- Comments generated: 2
Comment on lines
+4161
to
+4162
| bool expanded = newEl.GetIsExpanded(newItem); | ||
| if (node.IsExpanded != expanded) node.IsExpanded = expanded; |
| public bool CanDragItems { get; init; } | ||
| public bool AllowDrop { get; init; } | ||
| public bool CanReorderItems { get; init; } | ||
| internal Action<WinUI.TreeView>[] Setters { get; init; } = []; |
… attach Two CI failures on the typed TreeView<T> change: Unit Tests — PublicApiSurfaceGuardTests.EveryCallbackPropertyHasMatchingFluent required every Action/Action<T> callback property to have a matching fluent extension. Add `.ItemInvoked<T>` / `.Expanding<T>` for TemplatedTreeViewElement<T> in ElementExtensions.Events.cs (mirrors the existing TreeViewElement fluents). AOT Selftests — every visual-render TTV_ assertion failed while element-level ones passed: node views were never hosted. Hosting was deferred to the TreeView's Loaded event, which (unlike the typed ListView, which subscribes to ContainerContentChanging at mount) does not fire reliably in the headless AOT host before the fixture asserts. The internal TreeViewList only exists after the template applies, so we can't subscribe at mount — instead drive the attach from a bounded dispatcher retry that lands on the first pump (creating the subscription before/around the first realization and catching any later realization within the same render), with Loaded kept only as a backup for the show-later case (e.g. a tree in an unselected tab). FindTypedTreeListControl no longer writes the subscription-marker cache (read-only on the Update path). Full self-test suite green (4454 ok, 0 failures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(AOT) The AOT selftest failed deterministically on every TTV_ fixture that asserted rendered node text after a single Render(), while JIT passed. Root cause is not a product bug: per-node views host into their containers when the TreeView realizes them, which lands a dispatcher cycle after mount — and the NativeAOT host consistently needs one more pump cycle than JIT to get there (visible in that TTV_AddChild_NewNodeRenders, which already did two render passes, passed under AOT while the single-Render fixtures did not). Asserting after exactly one Render() was a test-harness assumption, not a runtime contract. Fix at the test layer: add a bounded WaitFor(condition) helper that pumps render passes until the rendered content appears, and use it for the render-dependent assertions. A genuinely blank/missing row never appears within the budget, so the regression coverage (including the expand/collapse-cycle test) is preserved. Also reverts the previous attempt to make hosting robust via a dispatcher-retry attach: JIT selftests passed with the plain Loaded subscription, so Loaded does fire in the headless host — the retry addressed a non-issue and is removed. Hosting is back to subscribing the internal TreeViewList's ContainerContentChanging on Loaded (idempotent), with no runtime side-effects added for the test's sake. Full self-test suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| H.Check("TTV_IsExpanded_SelectedNodeExpanded", | ||
| tv!.RootNodes.Count == 2 && tv.RootNodes[0].IsExpanded); | ||
| H.Check("TTV_IsExpanded_OtherNodeCollapsed", | ||
| !tv.RootNodes[1].IsExpanded); |
| H.Check("TTV_IsExpanded_TreeFound", tv is not null); | ||
| // First root ("docs") is expanded; the second ("pics") is not. | ||
| H.Check("TTV_IsExpanded_SelectedNodeExpanded", | ||
| tv!.RootNodes.Count == 2 && tv.RootNodes[0].IsExpanded); |
Temporary # TTVDIAG TAP-comment logging in the typed TreeView hosting path to capture why initial node containers don't host under the NativeAOT selftest host. To be reverted once root cause is confirmed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #447 — a
TreeViewwhoseTreeViewNodeDatanodes carry aContentElementrendered blank rows. A node-mode WinUITreeViewstringifiesTreeViewNode.Contentand cannot host a pre-builtUIElement; rich per-node visuals must come from a template (adata → Elementfunction), never an element instance.This adds a typed, data-driven
TreeView<T>— the hierarchical peer ofListView<T>/GridView<T>/FlipView<T>:Heterogeneous nodes with per-shape visuals are a
switchinside theviewBuilder(the C# equivalent of WinUI'sItemTemplateSelector).OnItemInvoked/OnExpandingareAction<T>and hand back the developer's ownT.TreeViewNodeData.ContentElementis marked[Obsolete]pointing atTreeView<T>; the legacy path stays functional (CS0618 suppressed at the internal use sites).Hosting — aligned with WinUI's model
Hosting mirrors the typed
ListView<T>, which is how WinUI itself drives rich item content:ItemTemplateis an emptyContentControlshell.TreeViewList'sContainerContentChanging— a fresh view on realize, unmounted on recycle. (We don't setargs.Handled, so theTreeViewList's own handler keeps doing its indentation/selection work.)node.Contentholds the developer's data item; theItemInvoked/Expandingtrampolines readTback from it.This keeps expand/collapse robust under container recycling, fixing the "every other expand/collapse blanks the first child row(s)" regression that an earlier declarative
{Binding Content}approach suffered (a recycled/collapsed container retained the element's visual parent, so a different pooled container couldn't re-host it).Updatereconciles only the realized containers' views in place, with a minimal-mutation node reorder so unchanged-order updates touch the live node collection not at all; unrealized nodes rebuild fromnode.Contenton next realization. Full-tree unmount walks realized containers so each node-view Component's cleanups run.Files
src/Reactor/Core/Element.cs—TemplatedTreeViewElementBase(object-erased base so the reconciler switch matches one type) +TemplatedTreeViewElement<T>;OwnPropsEqualarm. Value-typeTis boxed once; reference-typeTflows through the covariantIReadOnlyList<object>conversion.src/Reactor/Elements/Dsl.cs— the twoTreeView<T>factory overloads.src/Reactor/Core/Reconciler*.cs— empty-shellItemTemplate,ContainerContentChanginghosting on the internalTreeViewList(found + cached per TreeView), keyed in-place node diff, full-tree unmount walk.src/Reactor/Core/V1Protocol/ChildrenStrategy.cs— CS0618 suppression around the legacyContentElementreads.samples/Reactor.TestApp/Demos/DataTemplateDemo.cs— section 4 migrated fromContentElementtoTreeView<T>with a discriminatedPetNodemodel (distinct templates viaswitch).Tests
tests/Reactor.AppTests.Host/SelfTest/Fixtures/TemplatedTreeViewFixtures.cs(TTV_): rich-content render, heterogeneous per-shape templates, in-place keyed reconcile, structural add, expand/collapse cycles stay rendered, event-erasureTresolution, value-typeT,IsExpandedselector, unmount teardown.src/Reactor, TestApp, self-test host) — 0 errors, no CS0618 leakage.ok, 0 failures), including a fixture that drives several collapse→expand cycles and asserts every child row stays rendered.🤖 Generated with Claude Code