diff --git a/CHANGELOG.md b/CHANGELOG.md index 959d7f7df..c7b451e4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -146,6 +146,27 @@ to land under these conventions; subsequent specs follow this shape. ### Fixed +- **`DropDownButton` / `SplitButton` / `ToggleSplitButton` `.Flyout` now + reconciles across re-renders.** Mount built each button's flyout once + and the corresponding Update paths never rebuilt it, so any dynamic + flyout content (a `MenuFlyoutContentElement` whose `Items` array is + state-derived, a `ContentFlyoutElement` whose subtree depends on + state, or a swapped flyout instance entirely) froze at first-mount + values. The matching X→null transition (a button whose flyout is + removed across renders) also left the stale flyout attached. + `UpdateDropDownButton`, `UpdateSplitButton`, and + `UpdateToggleSplitButton` now thread the old element + + `requestRerender` and call the existing `ApplyFlyoutAttachment` + helper, which already knows how to reuse the realized `FlyoutBase` + (so an already-open flyout stays open), clear+repopulate + `MenuFlyout.Items`, or reconcile content inside a `Flyout.Content` + child; an explicit `Flyout = null` branch handles the clear case. + Same class of bug as the recently-fixed CommandBar / TeachingTip + Content gap (microsoft-ui-reactor#343). Adds `FlyoutReconcileFixtures` + (six selftests covering text mutation, count mutation, + `ContentFlyoutElement` subtree mutation, and X→null clearing across + the three button kinds). + - **ListView / GridView / LazyVStack / LazyHStack now surface incremental WinUI deltas for keyed updates.** Previously, any `ItemCount` change rebuilt `ItemsSource` from `Enumerable.Range(...)`, which caused WinUI diff --git a/src/Reactor/Core/Reconciler.Update.cs b/src/Reactor/Core/Reconciler.Update.cs index 14b7aa55f..aec2e7ccd 100644 --- a/src/Reactor/Core/Reconciler.Update.cs +++ b/src/Reactor/Core/Reconciler.Update.cs @@ -124,12 +124,12 @@ public sealed partial class Reconciler => UpdateRepeatButton(o, n, rb), (ToggleButtonElement o, ToggleButtonElement n, WinPrim.ToggleButton tb) => UpdateToggleButton(o, n, tb), - (DropDownButtonElement, DropDownButtonElement n, WinUI.DropDownButton ddb) - => UpdateDropDownButton(n, ddb), + (DropDownButtonElement o, DropDownButtonElement n, WinUI.DropDownButton ddb) + => UpdateDropDownButton(o, n, ddb, requestRerender), (SplitButtonElement o, SplitButtonElement n, WinUI.SplitButton sb) - => UpdateSplitButton(o, n, sb), + => UpdateSplitButton(o, n, sb, requestRerender), (ToggleSplitButtonElement o, ToggleSplitButtonElement n, WinUI.ToggleSplitButton tsb) - => UpdateToggleSplitButton(o, n, tsb), + => UpdateToggleSplitButton(o, n, tsb, requestRerender), (RichEditBoxElement o, RichEditBoxElement n, WinUI.RichEditBox reb) => UpdateRichEditBox(o, n, reb), (TextFieldElement o, TextFieldElement n, TextBox tb) @@ -736,24 +736,32 @@ private static void RebuildRichTextBlocks(RichTextBlockElement n, WinUI.RichText return null; } - private UIElement? UpdateDropDownButton(DropDownButtonElement n, WinUI.DropDownButton ddb) + private UIElement? UpdateDropDownButton(DropDownButtonElement o, DropDownButtonElement n, WinUI.DropDownButton ddb, Action requestRerender) { if (ddb.Content as string != n.Label) ddb.Content = n.Label; SetElementTag(ddb, n); + if (n.Flyout is not null) + ApplyFlyoutAttachment(ddb, o.Flyout, n.Flyout, requestRerender); + else if (o.Flyout is not null) + ddb.Flyout = null; ApplySetters(n.Setters, ddb); return null; } - private UIElement? UpdateSplitButton(SplitButtonElement o, SplitButtonElement n, WinUI.SplitButton sb) + private UIElement? UpdateSplitButton(SplitButtonElement o, SplitButtonElement n, WinUI.SplitButton sb, Action requestRerender) { sb.Content = n.Label; SetElementTag(sb, n); if (o.OnClick is null && n.OnClick is not null) sb.Click += (s, _) => (GetElementTag((UIElement)s!) as SplitButtonElement)?.OnClick?.Invoke(); + if (n.Flyout is not null) + ApplyFlyoutAttachment(sb, o.Flyout, n.Flyout, requestRerender); + else if (o.Flyout is not null) + sb.Flyout = null; ApplySetters(n.Setters, sb); return null; } - private UIElement? UpdateToggleSplitButton(ToggleSplitButtonElement o, ToggleSplitButtonElement n, WinUI.ToggleSplitButton tsb) + private UIElement? UpdateToggleSplitButton(ToggleSplitButtonElement o, ToggleSplitButtonElement n, WinUI.ToggleSplitButton tsb, Action requestRerender) { SetElementTag(tsb, n); if (o.OnIsCheckedChanged is null && n.OnIsCheckedChanged is not null) @@ -769,6 +777,10 @@ private static void RebuildRichTextBlocks(RichTextBlockElement n, WinUI.RichText ChangeEchoSuppressor.BeginSuppress(tsb); tsb.IsChecked = n.IsChecked; } + if (n.Flyout is not null) + ApplyFlyoutAttachment(tsb, o.Flyout, n.Flyout, requestRerender); + else if (o.Flyout is not null) + tsb.Flyout = null; ApplySetters(n.Setters, tsb); return null; } diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/FlyoutReconcileFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/FlyoutReconcileFixtures.cs new file mode 100644 index 000000000..3fbf1711b --- /dev/null +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/FlyoutReconcileFixtures.cs @@ -0,0 +1,340 @@ +using Microsoft.UI.Reactor.AppTests.Host.SelfTest; +using Microsoft.UI.Reactor.Core; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using static Microsoft.UI.Reactor.Factories; +using WinPrim = Microsoft.UI.Xaml.Controls.Primitives; +using WinUI = Microsoft.UI.Xaml.Controls; + +namespace Microsoft.UI.Reactor.AppTests.Host.SelfTest.Fixtures; + +/// +/// Regression coverage for the Flyout-on-button reconcile fix +/// (companion to , which covers the +/// equivalent CommandBar / TeachingTip Content gap). +/// +/// DropDownButtonElement.Flyout, SplitButtonElement.Flyout, +/// and ToggleSplitButtonElement.Flyout were attached by Mount but +/// never reconciled by Update, so dynamic flyout content (the children +/// inside a ContentFlyoutElement, or a MenuFlyoutContentElement's +/// item list) stayed frozen at first-mount values across re-renders. +/// +/// Each fixture mounts a button whose flyout depends on a +/// UseState counter, bumps the counter via a sibling button, and +/// asserts the WinUI flyout content reflects the updated value. Before +/// the fix the post-update flyout equals the mount-time flyout; after +/// the fix it tracks the new state. The realized WinUI Flyout instance +/// stays the same — ApplyFlyoutAttachment reuses it via +/// UpdateFlyoutInPlace, which is what preserves an already-open +/// flyout across re-renders. +/// +internal static class FlyoutReconcileFixtures +{ + // ── DropDownButtonElement.Flyout (MenuFlyout, text-mutating) ────────── + + internal class DropDownButtonFlyoutUpdates(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(ctx => + { + var (tick, setTick) = ctx.UseState(0); + // MenuFlyout with a single item whose Text encodes the tick. + var ddb = new DropDownButtonElement("Menu") + { + Flyout = new MenuFlyoutContentElement( + [ + new MenuFlyoutItemData($"item-tick = {tick}"), + ]), + }; + return VStack( + Button("FlyoutReconcile_BumpDDB", () => setTick(tick + 1)), + ddb + ); + }); + + await Harness.Render(); + + var realized = H.FindControl(_ => true); + H.Check("FlyoutReconcile_DDB_Mounted", realized is not null); + + var initialFlyout = realized?.Flyout as WinUI.MenuFlyout; + H.Check("FlyoutReconcile_DDB_InitialFlyoutAttached", + initialFlyout is not null && initialFlyout.Items.Count == 1); + + var initialItem = initialFlyout?.Items[0] as WinUI.MenuFlyoutItem; + H.Check("FlyoutReconcile_DDB_InitialItemText", + initialItem is not null && initialItem.Text == "item-tick = 0"); + + // Bump three times — covers both reconcile-after-first-update + // and stable behaviour across repeated updates. + for (int i = 0; i < 3; i++) + { + H.ClickButton("FlyoutReconcile_BumpDDB"); + await Harness.Render(); + } + + var realizedAfter = H.FindControl(_ => true); + H.Check("FlyoutReconcile_DDB_SameInstance", + realized is not null && ReferenceEquals(realized, realizedAfter)); + + // ApplyFlyoutAttachment routes through UpdateFlyoutInPlace, so + // the WinUI MenuFlyout instance is preserved — an open flyout + // would stay open while its items mutate underneath it. + var flyoutAfter = realizedAfter?.Flyout as WinUI.MenuFlyout; + H.Check("FlyoutReconcile_DDB_FlyoutSameInstance", + initialFlyout is not null && ReferenceEquals(initialFlyout, flyoutAfter)); + + var itemAfter = flyoutAfter?.Items[0] as WinUI.MenuFlyoutItem; + H.Check("FlyoutReconcile_DDB_ItemTextReconciled", + itemAfter is not null && itemAfter.Text == "item-tick = 3"); + } + } + + // ── SplitButtonElement.Flyout (MenuFlyout, text-mutating) ───────────── + + internal class SplitButtonFlyoutUpdates(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(ctx => + { + var (tick, setTick) = ctx.UseState(0); + var sb = new SplitButtonElement("Save") + { + Flyout = new MenuFlyoutContentElement( + [ + new MenuFlyoutItemData($"split-tick = {tick}"), + ]), + }; + return VStack( + Button("FlyoutReconcile_BumpSB", () => setTick(tick + 1)), + sb + ); + }); + + await Harness.Render(); + + var realized = H.FindControl(_ => true); + H.Check("FlyoutReconcile_SB_Mounted", realized is not null); + + var initialFlyout = realized?.Flyout as WinUI.MenuFlyout; + var initialItem = initialFlyout?.Items[0] as WinUI.MenuFlyoutItem; + H.Check("FlyoutReconcile_SB_InitialItemText", + initialItem is not null && initialItem.Text == "split-tick = 0"); + + H.ClickButton("FlyoutReconcile_BumpSB"); + await Harness.Render(); + H.ClickButton("FlyoutReconcile_BumpSB"); + await Harness.Render(); + + var flyoutAfter = (H.FindControl(_ => true)?.Flyout) as WinUI.MenuFlyout; + H.Check("FlyoutReconcile_SB_FlyoutSameInstance", + initialFlyout is not null && ReferenceEquals(initialFlyout, flyoutAfter)); + var itemAfter = flyoutAfter?.Items[0] as WinUI.MenuFlyoutItem; + H.Check("FlyoutReconcile_SB_ItemTextReconciled", + itemAfter is not null && itemAfter.Text == "split-tick = 2"); + } + } + + // ── ToggleSplitButtonElement.Flyout (MenuFlyout, text-mutating) ─────── + + internal class ToggleSplitButtonFlyoutUpdates(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(ctx => + { + var (tick, setTick) = ctx.UseState(0); + var tsb = new ToggleSplitButtonElement("Bold") + { + Flyout = new MenuFlyoutContentElement( + [ + new MenuFlyoutItemData($"toggle-tick = {tick}"), + ]), + }; + return VStack( + Button("FlyoutReconcile_BumpTSB", () => setTick(tick + 1)), + tsb + ); + }); + + await Harness.Render(); + + var realized = H.FindControl(_ => true); + H.Check("FlyoutReconcile_TSB_Mounted", realized is not null); + + var initialFlyout = realized?.Flyout as WinUI.MenuFlyout; + var initialItem = initialFlyout?.Items[0] as WinUI.MenuFlyoutItem; + H.Check("FlyoutReconcile_TSB_InitialItemText", + initialItem is not null && initialItem.Text == "toggle-tick = 0"); + + H.ClickButton("FlyoutReconcile_BumpTSB"); + await Harness.Render(); + H.ClickButton("FlyoutReconcile_BumpTSB"); + await Harness.Render(); + H.ClickButton("FlyoutReconcile_BumpTSB"); + await Harness.Render(); + + var flyoutAfter = (H.FindControl(_ => true)?.Flyout) as WinUI.MenuFlyout; + H.Check("FlyoutReconcile_TSB_FlyoutSameInstance", + initialFlyout is not null && ReferenceEquals(initialFlyout, flyoutAfter)); + var itemAfter = flyoutAfter?.Items[0] as WinUI.MenuFlyoutItem; + H.Check("FlyoutReconcile_TSB_ItemTextReconciled", + itemAfter is not null && itemAfter.Text == "toggle-tick = 3"); + } + } + + // ── Items length changes are reconciled too ─────────────────────────── + + /// + /// Belt-and-braces: dynamic *count* changes go through + /// UpdateFlyoutInPlace's clear+repopulate path on + /// MenuFlyout.Items, not just text mutations on a stable item. + /// This is the original Radiant repro — a button whose flyout + /// enumerates a state-derived list. + /// + internal class DropDownButtonFlyoutItemsGrow(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(ctx => + { + var (count, setCount) = ctx.UseState(1); + var items = new global::Microsoft.UI.Reactor.Core.MenuFlyoutItemBase[count]; + for (int i = 0; i < count; i++) + items[i] = new MenuFlyoutItemData($"row-{i}"); + + var ddb = new DropDownButtonElement("Grow") + { + Flyout = new MenuFlyoutContentElement(items), + }; + return VStack( + Button("FlyoutReconcile_AddRow", () => setCount(count + 1)), + ddb + ); + }); + + await Harness.Render(); + + var realized = H.FindControl(_ => true); + var flyout0 = realized?.Flyout as WinUI.MenuFlyout; + H.Check("FlyoutReconcile_Grow_InitialOne", + flyout0 is not null && flyout0.Items.Count == 1); + + H.ClickButton("FlyoutReconcile_AddRow"); + await Harness.Render(); + H.ClickButton("FlyoutReconcile_AddRow"); + await Harness.Render(); + H.ClickButton("FlyoutReconcile_AddRow"); + await Harness.Render(); + + var flyoutN = (H.FindControl(_ => true)?.Flyout) as WinUI.MenuFlyout; + H.Check("FlyoutReconcile_Grow_Reconciled", + flyoutN is not null && flyoutN.Items.Count == 4); + } + } + + // ── ContentFlyoutElement subtree reconciles too ─────────────────────── + + /// + /// Non-menu flyout: a ContentFlyoutElement wraps an arbitrary + /// element subtree (here a TextBlock) so it should reconcile + /// through UpdateFlyoutInPlace's child-update path. + /// + internal class DropDownButtonContentFlyoutUpdates(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(ctx => + { + var (tick, setTick) = ctx.UseState(0); + var ddb = new DropDownButtonElement("Hint") + { + Flyout = new ContentFlyoutElement( + TextBlock($"content-tick = {tick}") + .Set(t => t.Name = "FlyoutReconcile_ContentText")), + }; + return VStack( + Button("FlyoutReconcile_BumpContent", () => setTick(tick + 1)), + ddb + ); + }); + + await Harness.Render(); + + var realized = H.FindControl(_ => true); + var initial = realized?.Flyout as WinUI.Flyout; + var initialTb = initial?.Content as TextBlock; + H.Check("FlyoutReconcile_Content_InitialText", + initialTb is not null && initialTb.Text == "content-tick = 0"); + + H.ClickButton("FlyoutReconcile_BumpContent"); + await Harness.Render(); + H.ClickButton("FlyoutReconcile_BumpContent"); + await Harness.Render(); + + var after = (H.FindControl(_ => true)?.Flyout) as WinUI.Flyout; + var afterTb = after?.Content as TextBlock; + H.Check("FlyoutReconcile_Content_FlyoutSameInstance", + initial is not null && ReferenceEquals(initial, after)); + H.Check("FlyoutReconcile_Content_TextReconciled", + afterTb is not null && afterTb.Text == "content-tick = 2"); + H.Check("FlyoutReconcile_Content_TextSameInstance", + initialTb is not null && ReferenceEquals(initialTb, afterTb)); + } + } + + // ── X → null transitions clear the realized flyout ──────────────────── + + /// + /// Symmetric to the X→X update path: when a button's flyout transitions + /// from non-null to null across renders, the realized WinUI control + /// should drop its .Flyout attachment too. Without this branch, + /// the user-observable button still pops the stale flyout on click. + /// + internal class DropDownButtonFlyoutClears(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(ctx => + { + var (hasFlyout, setHasFlyout) = ctx.UseState(true); + var ddb = new DropDownButtonElement("Maybe") + { + Flyout = hasFlyout + ? new MenuFlyoutContentElement( + [ + new MenuFlyoutItemData("item"), + ]) + : null, + }; + return VStack( + Button("FlyoutReconcile_Clear", () => setHasFlyout(false)), + ddb + ); + }); + + await Harness.Render(); + + var realized = H.FindControl(_ => true); + H.Check("FlyoutReconcile_Clear_InitiallyAttached", + realized?.Flyout is WinUI.MenuFlyout); + + H.ClickButton("FlyoutReconcile_Clear"); + await Harness.Render(); + + var after = H.FindControl(_ => true); + H.Check("FlyoutReconcile_Clear_ButtonSameInstance", + realized is not null && ReferenceEquals(realized, after)); + H.Check("FlyoutReconcile_Clear_FlyoutDetached", + after is not null && after.Flyout is null); + } + } +} diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index ac6a87066..1f429b988 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -564,6 +564,14 @@ internal static class SelfTestFixtureRegistry // Issue 343 — CommandBar/TeachingTip Content reconciliation "Issue343_CommandBarContentUpdates", "Issue343_TeachingTipContentUpdates", + // Companion to Issue 343 — Flyout reconciliation on + // DropDownButton / SplitButton / ToggleSplitButton. + "FlyoutReconcile_DropDownButtonFlyoutUpdates", + "FlyoutReconcile_SplitButtonFlyoutUpdates", + "FlyoutReconcile_ToggleSplitButtonFlyoutUpdates", + "FlyoutReconcile_DropDownButtonFlyoutItemsGrow", + "FlyoutReconcile_DropDownButtonContentFlyoutUpdates", + "FlyoutReconcile_DropDownButtonFlyoutClears", // Hooks coverage — FocusManager, UseFocus "HooksCov_FocusManagerRegistration", "HooksCov_FocusManagerNavigation", @@ -1430,6 +1438,12 @@ internal static class SelfTestFixtureRegistry // Issue 343 — CommandBar/TeachingTip Content reconciliation "Issue343_CommandBarContentUpdates" => new Issue343Fixtures.CommandBarContentUpdates(harness), "Issue343_TeachingTipContentUpdates" => new Issue343Fixtures.TeachingTipContentUpdates(harness), + "FlyoutReconcile_DropDownButtonFlyoutUpdates" => new FlyoutReconcileFixtures.DropDownButtonFlyoutUpdates(harness), + "FlyoutReconcile_SplitButtonFlyoutUpdates" => new FlyoutReconcileFixtures.SplitButtonFlyoutUpdates(harness), + "FlyoutReconcile_ToggleSplitButtonFlyoutUpdates" => new FlyoutReconcileFixtures.ToggleSplitButtonFlyoutUpdates(harness), + "FlyoutReconcile_DropDownButtonFlyoutItemsGrow" => new FlyoutReconcileFixtures.DropDownButtonFlyoutItemsGrow(harness), + "FlyoutReconcile_DropDownButtonContentFlyoutUpdates" => new FlyoutReconcileFixtures.DropDownButtonContentFlyoutUpdates(harness), + "FlyoutReconcile_DropDownButtonFlyoutClears" => new FlyoutReconcileFixtures.DropDownButtonFlyoutClears(harness), "ControlsCov_SearchManager" => new ControlsCoverageFixtures.SearchManagerExercise(harness), // Hooks coverage "HooksCov_FocusManagerRegistration" => new HooksCoverageFixtures.FocusManagerRegistration(harness),