Skip to content

Commit 21a1a4a

Browse files
fix(navigationview): reconcile menu items in place to preserve expansion
NavigationView cleared and rebuilt every menu item whenever the MenuItems array reference changed — which is effectively every render, since consumers build a fresh array each Render(). Per-item expansion lives only on the live NavigationViewItem (NavigationViewItemData has no IsExpanded), so each re-render snapped expanded hierarchical categories shut, and selecting a child collapsed its parent. Reconcile menu items in place instead: match NavigationViewItems by Tag, update only changed Content/Icon, recurse into children, and reuse the container so IsExpanded survives the re-render. A fast path skips all detaching when the structure is unchanged. Mirrored into the V1 NavigationViewDescriptor so V1 ON behaves identically to V1 OFF, and mount-time selection now recurses so a child tag selects correctly. Adds a NavigationView E2E guard (NavigationView_Hierarchical fixture + NavigationViewInteractionTests) that fails on the pre-fix build and passes after, plus a TreeView E2E guard exercising the Gallery's basic text-tree path. Compact (icon-only) pane interaction is deferred to a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 70b4245 commit 21a1a4a

8 files changed

Lines changed: 682 additions & 15 deletions

File tree

src/Reactor/Core/Reconciler.Mount.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1450,10 +1450,7 @@ private WinUI.NavigationView MountNavigationView(NavigationViewElement nav, Acti
14501450
}
14511451
if (nav.Content is not null) nv.Content = Mount(nav.Content, requestRerender);
14521452
if (nav.SelectedTag is not null)
1453-
{
1454-
foreach (var mi in nv.MenuItems.OfType<WinUI.NavigationViewItem>())
1455-
if (mi.Tag as string == nav.SelectedTag) { nv.SelectedItem = mi; break; }
1456-
}
1453+
nv.SelectedItem = FindNavItemByTag(nv.MenuItems, nav.SelectedTag);
14571454
SetElementTag(nv, nav);
14581455
if (nav.OnSelectedTagChanged is not null)
14591456
nv.SelectionChanged += (s, args) =>

src/Reactor/Core/Reconciler.Update.cs

Lines changed: 127 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1750,17 +1750,17 @@ void FinalizeOldPage(UIElement? oldCtrl, Element? oldElem, object? oldRoute)
17501750
if (!double.IsNaN(n.CompactModeThresholdWidth) && nv.CompactModeThresholdWidth != n.CompactModeThresholdWidth) nv.CompactModeThresholdWidth = n.CompactModeThresholdWidth;
17511751
if (!double.IsNaN(n.ExpandedModeThresholdWidth) && nv.ExpandedModeThresholdWidth != n.ExpandedModeThresholdWidth) nv.ExpandedModeThresholdWidth = n.ExpandedModeThresholdWidth;
17521752

1753+
// Reconcile menu items in place rather than clear-and-rebuild. The old
1754+
// rebuild recreated every NavigationViewItem on each render, and since
1755+
// per-item expansion (IsExpanded) lives only on the live container — it
1756+
// is not modeled in NavigationViewItemData — every re-render snapped all
1757+
// expanded hierarchical items shut. Because consumers typically pass a
1758+
// fresh MenuItems array each render (e.g. built via LINQ), the
1759+
// ReferenceEquals guard never held and the rebuild fired constantly,
1760+
// producing the "expand then collapse" flash and "child click collapses
1761+
// parent". Reusing containers by Tag preserves IsExpanded across renders.
17531762
if (!ReferenceEquals(o.MenuItems, n.MenuItems))
1754-
{
1755-
nv.MenuItems.Clear();
1756-
foreach (var item in n.MenuItems)
1757-
{
1758-
if (item.IsHeader)
1759-
nv.MenuItems.Add(new WinUI.NavigationViewItemHeader { Content = item.Content });
1760-
else
1761-
nv.MenuItems.Add(CreateNavItem(item));
1762-
}
1763-
}
1763+
ReconcileNavMenuItems(nv.MenuItems, o.MenuItems, n.MenuItems);
17641764

17651765
// AutoSuggestBox / PaneFooter / PaneCustomContent reconcile in place
17661766
// when possible so the controls keep focus / scroll state across re-renders.
@@ -1835,6 +1835,123 @@ void FinalizeOldPage(UIElement? oldCtrl, Element? oldElem, object? oldRoute)
18351835
return null;
18361836
}
18371837

1838+
/// <summary>
1839+
/// Brings a live NavigationView menu-item collection into agreement with
1840+
/// <paramref name="newData"/> while reusing existing <see cref="WinUI.NavigationViewItem"/>
1841+
/// containers (matched by Tag) so their runtime <c>IsExpanded</c>/selection
1842+
/// state survives the re-render. Recurses into hierarchical children.
1843+
/// </summary>
1844+
private void ReconcileNavMenuItems(
1845+
global::System.Collections.Generic.IList<object> live,
1846+
NavigationViewItemData[]? oldData,
1847+
NavigationViewItemData[] newData)
1848+
{
1849+
// Fast path: the structure (order of headers + item Tags) is unchanged —
1850+
// the overwhelmingly common case, since menus are usually static. Update
1851+
// each container in place without detaching anything, which keeps every
1852+
// container's expansion, selection and animation state pristine.
1853+
if (NavStructureMatches(live, newData))
1854+
{
1855+
for (int i = 0; i < newData.Length; i++)
1856+
{
1857+
var data = newData[i];
1858+
if (data.IsHeader)
1859+
{
1860+
if (live[i] is WinUI.NavigationViewItemHeader h && !Equals(h.Content, data.Content))
1861+
h.Content = data.Content;
1862+
}
1863+
else if (live[i] is WinUI.NavigationViewItem nvi)
1864+
{
1865+
var oldItem = oldData is not null && i < oldData.Length ? oldData[i] : null;
1866+
UpdateNavItemInPlace(nvi, oldItem, data);
1867+
}
1868+
}
1869+
return;
1870+
}
1871+
1872+
// Structure changed (items added / removed / reordered): snapshot the
1873+
// existing containers by Tag so matches can be reused, then rebuild the
1874+
// collection in the new order. Reused containers retain their IsExpanded.
1875+
var reusable = new global::System.Collections.Generic.Dictionary<string, WinUI.NavigationViewItem>();
1876+
foreach (var item in live)
1877+
if (item is WinUI.NavigationViewItem nvi && nvi.Tag is string tag)
1878+
reusable[tag] = nvi;
1879+
1880+
var oldByTag = new global::System.Collections.Generic.Dictionary<string, NavigationViewItemData>();
1881+
if (oldData is not null)
1882+
foreach (var d in oldData)
1883+
if (!d.IsHeader)
1884+
oldByTag[d.Tag ?? d.Content] = d;
1885+
1886+
live.Clear();
1887+
foreach (var data in newData)
1888+
{
1889+
if (data.IsHeader)
1890+
{
1891+
live.Add(new WinUI.NavigationViewItemHeader { Content = data.Content });
1892+
continue;
1893+
}
1894+
1895+
var key = data.Tag ?? data.Content;
1896+
if (reusable.TryGetValue(key, out var nvi))
1897+
UpdateNavItemInPlace(nvi, oldByTag.GetValueOrDefault(key), data);
1898+
else
1899+
nvi = CreateNavItem(data);
1900+
live.Add(nvi);
1901+
}
1902+
}
1903+
1904+
/// <summary>True when the live collection already matches the new data in
1905+
/// count and in the sequence of headers / item Tags.</summary>
1906+
private static bool NavStructureMatches(
1907+
global::System.Collections.Generic.IList<object> live,
1908+
NavigationViewItemData[] newData)
1909+
{
1910+
if (live.Count != newData.Length) return false;
1911+
for (int i = 0; i < newData.Length; i++)
1912+
{
1913+
var data = newData[i];
1914+
if (data.IsHeader)
1915+
{
1916+
if (live[i] is not WinUI.NavigationViewItemHeader) return false;
1917+
}
1918+
else
1919+
{
1920+
if (live[i] is not WinUI.NavigationViewItem nvi) return false;
1921+
if ((nvi.Tag as string) != (data.Tag ?? data.Content)) return false;
1922+
}
1923+
}
1924+
return true;
1925+
}
1926+
1927+
/// <summary>Updates a reused NavigationViewItem's Content/Icon (only when
1928+
/// changed) and reconciles its children, leaving IsExpanded untouched.</summary>
1929+
private void UpdateNavItemInPlace(WinUI.NavigationViewItem nvi, NavigationViewItemData? oldData, NavigationViewItemData data)
1930+
{
1931+
if (!Equals(nvi.Content, data.Content)) nvi.Content = data.Content;
1932+
1933+
var newTag = data.Tag ?? data.Content;
1934+
if (!Equals(nvi.Tag, newTag)) nvi.Tag = newTag;
1935+
1936+
// Re-resolve the icon only when the icon data actually changed (IconData
1937+
// records compare by value), so the FontIcon/SymbolIcon isn't reallocated
1938+
// on every render.
1939+
bool iconChanged = oldData is null
1940+
|| !Equals(oldData.IconElement, data.IconElement)
1941+
|| oldData.Icon != data.Icon;
1942+
if (iconChanged)
1943+
{
1944+
var icon = ResolveIcon(data.IconElement, data.Icon);
1945+
if (icon is not null) nvi.Icon = icon;
1946+
else if (nvi.Icon is not null) nvi.Icon = null;
1947+
}
1948+
1949+
if (data.Children is { Length: > 0 } children)
1950+
ReconcileNavMenuItems(nvi.MenuItems, oldData?.Children, children);
1951+
else if (nvi.MenuItems.Count > 0)
1952+
nvi.MenuItems.Clear();
1953+
}
1954+
18381955
private UIElement? UpdateTitleBar(TitleBarElement o, TitleBarElement n, WinUI.TitleBar titleBar, Action requestRerender)
18391956
{
18401957
titleBar.Title = n.Title;

src/Reactor/Core/V1Protocol/Descriptor/Descriptors/NavigationViewDescriptor.cs

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,9 @@ internal static class NavigationViewDescriptor
123123

124124
private static void ApplyMenuAndSelection(WinUI.NavigationView control, NavigationViewElement? oldElement, NavigationViewElement element)
125125
{
126-
if (oldElement is null || !ReferenceEquals(oldElement.MenuItems, element.MenuItems))
126+
if (oldElement is null)
127127
{
128+
// Mount: build fresh.
128129
control.MenuItems.Clear();
129130
foreach (var item in element.MenuItems)
130131
{
@@ -133,6 +134,13 @@ private static void ApplyMenuAndSelection(WinUI.NavigationView control, Navigati
133134
: CreateNavItem(item));
134135
}
135136
}
137+
else if (!ReferenceEquals(oldElement.MenuItems, element.MenuItems))
138+
{
139+
// Update: reconcile in place so reused containers keep IsExpanded.
140+
// Mirrors Reconciler.ReconcileNavMenuItems (legacy arm) — see the note
141+
// there on why a clear-and-rebuild collapses hierarchical items.
142+
ReconcileMenuItems(control.MenuItems, oldElement.MenuItems, element.MenuItems);
143+
}
136144

137145
if (oldElement is null
138146
|| oldElement.SelectedTag != element.SelectedTag
@@ -142,6 +150,107 @@ private static void ApplyMenuAndSelection(WinUI.NavigationView control, Navigati
142150
}
143151
}
144152

153+
private static void ReconcileMenuItems(
154+
global::System.Collections.Generic.IList<object> live,
155+
NavigationViewItemData[]? oldData,
156+
NavigationViewItemData[] newData)
157+
{
158+
if (StructureMatches(live, newData))
159+
{
160+
for (int i = 0; i < newData.Length; i++)
161+
{
162+
var data = newData[i];
163+
if (data.IsHeader)
164+
{
165+
if (live[i] is WinUI.NavigationViewItemHeader h && !Equals(h.Content, data.Content))
166+
h.Content = data.Content;
167+
}
168+
else if (live[i] is WinUI.NavigationViewItem nvi)
169+
{
170+
var oldItem = oldData is not null && i < oldData.Length ? oldData[i] : null;
171+
UpdateNavItemInPlace(nvi, oldItem, data);
172+
}
173+
}
174+
return;
175+
}
176+
177+
var reusable = new global::System.Collections.Generic.Dictionary<string, WinUI.NavigationViewItem>();
178+
foreach (var item in live)
179+
if (item is WinUI.NavigationViewItem nvi && nvi.Tag is string tag)
180+
reusable[tag] = nvi;
181+
182+
var oldByTag = new global::System.Collections.Generic.Dictionary<string, NavigationViewItemData>();
183+
if (oldData is not null)
184+
foreach (var d in oldData)
185+
if (!d.IsHeader)
186+
oldByTag[d.Tag ?? d.Content] = d;
187+
188+
live.Clear();
189+
foreach (var data in newData)
190+
{
191+
if (data.IsHeader)
192+
{
193+
live.Add(new WinUI.NavigationViewItemHeader { Content = data.Content });
194+
continue;
195+
}
196+
197+
var key = data.Tag ?? data.Content;
198+
if (reusable.TryGetValue(key, out var nvi))
199+
UpdateNavItemInPlace(nvi, oldByTag.GetValueOrDefault(key), data);
200+
else
201+
nvi = CreateNavItem(data);
202+
live.Add(nvi);
203+
}
204+
}
205+
206+
private static bool StructureMatches(
207+
global::System.Collections.Generic.IList<object> live,
208+
NavigationViewItemData[] newData)
209+
{
210+
if (live.Count != newData.Length) return false;
211+
for (int i = 0; i < newData.Length; i++)
212+
{
213+
var data = newData[i];
214+
if (data.IsHeader)
215+
{
216+
if (live[i] is not WinUI.NavigationViewItemHeader) return false;
217+
}
218+
else
219+
{
220+
if (live[i] is not WinUI.NavigationViewItem nvi) return false;
221+
if ((nvi.Tag as string) != (data.Tag ?? data.Content)) return false;
222+
}
223+
}
224+
return true;
225+
}
226+
227+
private static void UpdateNavItemInPlace(WinUI.NavigationViewItem nvi, NavigationViewItemData? oldData, NavigationViewItemData data)
228+
{
229+
if (!Equals(nvi.Content, data.Content)) nvi.Content = data.Content;
230+
231+
var newTag = data.Tag ?? data.Content;
232+
if (!Equals(nvi.Tag, newTag)) nvi.Tag = newTag;
233+
234+
bool iconChanged = oldData is null
235+
|| !Equals(oldData.IconElement, data.IconElement)
236+
|| oldData.Icon != data.Icon;
237+
if (iconChanged)
238+
{
239+
var icon = data.IconElement is not null
240+
? Reconciler.ResolveIconForDescriptor(data.IconElement)
241+
: data.Icon is not null
242+
? Reconciler.ResolveIconForDescriptor(new SymbolIconData(data.Icon))
243+
: null;
244+
if (icon is not null) nvi.Icon = icon;
245+
else if (nvi.Icon is not null) nvi.Icon = null;
246+
}
247+
248+
if (data.Children is { Length: > 0 } children)
249+
ReconcileMenuItems(nvi.MenuItems, oldData?.Children, children);
250+
else if (nvi.MenuItems.Count > 0)
251+
nvi.MenuItems.Clear();
252+
}
253+
145254
private static WinUI.NavigationViewItem CreateNavItem(NavigationViewItemData data)
146255
{
147256
var item = new WinUI.NavigationViewItem { Content = data.Content, Tag = data.Tag ?? data.Content };

tests/Reactor.AppTests.Host/FixtureRegistry.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,17 @@ internal static class FixtureRegistry
6161
// Collections
6262
"ListView_TypedRendering",
6363

64+
// TreeView expand/collapse (mirrors ReactorGallery Basic TreeView —
65+
// E2E repro/guard for the item-body-collapse bug)
66+
"TreeView_BasicTextTree",
67+
6468
// Navigation
6569
"Navigation_TabSwitching",
6670

71+
// NavigationView hierarchical expand/collapse (E2E repro/guard for the
72+
// re-render rebuild-clobber that collapsed expanded categories)
73+
"NavigationView_Hierarchical",
74+
6775
// Observable
6876
"Observable_UseObservable_Rerender",
6977
"Observable_UseObservable_ExternalMutation",
@@ -205,9 +213,15 @@ internal static class FixtureRegistry
205213
// Collections
206214
"ListView_TypedRendering" => CollectionFixtures.ListViewTyped(ctx),
207215

216+
// TreeView expand/collapse
217+
"TreeView_BasicTextTree" => TreeViewE2EFixtures.BasicTextTree(ctx),
218+
208219
// Navigation
209220
"Navigation_TabSwitching" => NavigationFixtures.TabSwitching(ctx),
210221

222+
// NavigationView hierarchical expand/collapse
223+
"NavigationView_Hierarchical" => NavigationViewE2EFixtures.HierarchicalNav(ctx),
224+
211225
// Observable
212226
"Observable_UseObservable_Rerender" => ObservableFixtures.UseObservable_Rerender(ctx),
213227
"Observable_UseObservable_ExternalMutation" => ObservableFixtures.UseObservable_ExternalMutation(ctx),
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
using Microsoft.UI.Reactor;
2+
using Microsoft.UI.Reactor.Core;
3+
using Microsoft.UI.Xaml.Controls;
4+
using static Microsoft.UI.Reactor.Factories;
5+
6+
namespace Microsoft.UI.Reactor.AppTests.Host.Fixtures;
7+
8+
/// <summary>
9+
/// E2E fixtures for hierarchical <c>NavigationView</c> expand/collapse, mirroring
10+
/// the ReactorGallery shell shape: a stateful component that holds the selected
11+
/// tag, re-renders on every selection, and rebuilds its <c>MenuItems</c> array
12+
/// fresh each render (the condition that made the old clear-and-rebuild update
13+
/// path collapse expanded categories on every re-render).
14+
///
15+
/// PaneDisplayMode is pinned to <c>Left</c> so the pane stays open with text
16+
/// labels (addressable by UIA Name) and children render inline when expanded,
17+
/// regardless of the host window size.
18+
/// </summary>
19+
internal static class NavigationViewE2EFixtures
20+
{
21+
internal class HierarchicalNavComponent : Component
22+
{
23+
public override Element Render()
24+
{
25+
var (selectedTag, setSelectedTag) = UseState("alpha-1");
26+
27+
// Rebuilt fresh every render — exactly the pattern the Gallery uses
28+
// and the one that exposed the rebuild-clobber bug.
29+
var menuItems = new[]
30+
{
31+
NavItem("Alpha", tag: "alpha") with
32+
{
33+
Children = new[]
34+
{
35+
NavItem("Alpha-1", tag: "alpha-1"),
36+
NavItem("Alpha-2", tag: "alpha-2"),
37+
}
38+
},
39+
NavItem("Bravo", tag: "bravo") with
40+
{
41+
Children = new[]
42+
{
43+
NavItem("Bravo-1", tag: "bravo-1"),
44+
}
45+
},
46+
};
47+
48+
return NavigationView(
49+
menuItems,
50+
content: TextBlock($"Selected: {selectedTag}").AutomationId("NavSelectedTag")
51+
) with
52+
{
53+
SelectedTag = selectedTag,
54+
PaneDisplayMode = NavigationViewPaneDisplayMode.Left,
55+
IsSettingsVisible = false,
56+
OnSelectedTagChanged = tag =>
57+
{
58+
if (tag != null) setSelectedTag(tag);
59+
},
60+
};
61+
}
62+
}
63+
64+
internal static Element HierarchicalNav(RenderContext ctx) =>
65+
Component<HierarchicalNavComponent>();
66+
}

0 commit comments

Comments
 (0)