Skip to content

Commit 2895837

Browse files
committed
Wave C: detect the Shell flyout-template contract by concept, not by name
The proposed API has changed shape twice while this adapter sat here: the internal GetBindableObjectWithFlyoutItemTemplate, then a three-method contract, and now a single resolve-style call. Each time the expiry test named members explicitly it silently stopped detecting anything - worse than having no test, because a green build then implies the adapter is still needed. The detector now matches the concept: any new public Shell member about a flyout item template. It is covered by table-driven tests proving it fires for the resolve-style shape, the three-method shape and plausible alternatives, while ignoring pre-existing members - so it is verified to trigger rather than merely assumed to. dotnet/maui#37862 remains OPEN and under design. No shape is adopted; ShellTemplateResolver's implementation is untouched. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent a6f5d4a commit 2895837

3 files changed

Lines changed: 84 additions & 34 deletions

File tree

docs/waves/wave-c.md

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,19 @@ Two items are not fully resolved and should not be described as "done":
9191
`IShellController.GetFlyoutItemDataTemplate`. Note the shape differs from the internal helper
9292
Wave C reimplemented, so adoption is a rewrite of the adapter rather than a rename.
9393

94-
**The PR is open, so nothing here adopts it.** The adapter stays provisional until the API is
95-
merged *and* present in a referenced package. What did change is the expiry test: it now watches
96-
the three proposed members instead of `GetBindableObjectWithFlyoutItemTemplate`. Watching the
97-
internal name would never have fired — upstream is not planning to publish it — and the adapter
98-
would have quietly become permanent, which is precisely the rot the expiry tests exist to
99-
prevent.
94+
**The PR is open and still being designed, so nothing here adopts it.** The adapter stays
95+
provisional until the design merges *and* ships in a referenced package.
96+
97+
The expiry test is deliberately **name-agnostic**, and that is worth explaining. The proposed
98+
API has now changed shape twice while the adapter sat here: the internal
99+
`GetBindableObjectWithFlyoutItemTemplate`, then a three-method contract
100+
(`IsFlyoutItemTemplateSet` / `GetFlyoutItemTemplateSource` / `GetFlyoutItemTemplateProperty`),
101+
and now a single resolve-style call. Each time the test named members explicitly it silently
102+
stopped detecting anything — which is worse than no test, because a green build then *implies*
103+
the adapter is still needed. It now matches the concept (any new public `Shell` member about a
104+
flyout item template) and is itself covered by table-driven tests proving it fires for the
105+
resolve-style shape, the three-method shape and plausible alternatives, while ignoring
106+
pre-existing members.
100107

101108
2. **`Toolbar.DrawerToggleVisible` (request 0009).** `IToolbar` publishes `BackButtonVisible` and
102109
`IsVisible` but not `DrawerToggleVisible`, which is the third member of the same concept. Wave C

src/Maui.Tizen.Controls.Navigation/Adapters/UpstreamApiRequests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ public static class UpstreamApiRequests
4242
new(
4343
"MAUI-TIZEN-API-0001",
4444
"Microsoft.Maui.Controls.Shell.GetBindableObjectWithFlyoutItemTemplate(BindableObject)",
45-
"Shell.IsFlyoutItemTemplateSet / GetFlyoutItemTemplateSource / GetFlyoutItemTemplateProperty (proposed in dotnet/maui#37862, open)",
45+
"A public flyout item template contract on Shell (dotnet/maui#37862, OPEN and still being designed - currently moving toward a single resolve-style call returning the final DataTemplate)",
4646
nameof(ShellTemplateResolver),
47-
"Any backend rendering a flyout must resolve which element owns the item template. Only partially reproducible off-tree: the MenuShellItem redirect cannot be expressed at all because MenuShellItem and its MenuItem property are both internal, so bare MenuItems in a flyout fall back to the shell-level template. Upstream dotnet/maui#37862 proposes a public contract with a different shape to the internal helper; it is OPEN, so the adapter stays provisional and the API must not be baked in before it merges and ships in a referenced package."),
47+
"Any backend rendering a flyout must resolve which element owns the item template. Only partially reproducible off-tree: the MenuShellItem redirect cannot be expressed at all because MenuShellItem and its MenuItem property are both internal, so bare MenuItems in a flyout fall back to the shell-level template. Upstream dotnet/maui#37862 is OPEN and its shape is still moving (internal helper -> three-method contract -> single resolve-style call). The adapter stays provisional and no shape is baked in until the design merges and ships in a referenced package; the expiry test matches the concept rather than any proposed name."),
4848
new(
4949
"MAUI-TIZEN-API-0002",
5050
"Microsoft.Maui.Controls.ViewExtensions.FindParentOfType<T>(Element, bool)",

tests/Maui.Tizen.SourceTests/WaveCUpstreamExpiryTests.cs

Lines changed: 69 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -59,48 +59,47 @@ public void ToolbarDrawerToggleAdapterExpiresWhenIToolbarPublishesTheProperty()
5959
/// bare <c>MenuItem</c> in a flyout therefore falls back to the shell-level template.
6060
/// </para>
6161
/// <para>
62-
/// dotnet/maui#37862 ("Add public Shell flyout item template contract for external backends")
63-
/// is open and proposes a different shape to the internal helper Wave C reimplemented:
64-
/// <c>Shell.IsFlyoutItemTemplateSet</c>, <c>Shell.GetFlyoutItemTemplateSource</c> and
65-
/// <c>Shell.GetFlyoutItemTemplateProperty</c>, used alongside the already-public
66-
/// <c>IShellController.GetFlyoutItemDataTemplate</c>.
62+
/// DELIBERATELY NAME-AGNOSTIC. The proposed API has already changed shape twice while this
63+
/// adapter sat here: first as the internal <c>GetBindableObjectWithFlyoutItemTemplate</c>, then
64+
/// as a three-method contract (<c>IsFlyoutItemTemplateSet</c>,
65+
/// <c>GetFlyoutItemTemplateSource</c>, <c>GetFlyoutItemTemplateProperty</c>), and it is now
66+
/// being redesigned again toward a single resolve-style call. Each time this test named the
67+
/// members explicitly it silently stopped detecting anything, which is worse than having no
68+
/// test at all - a green build implies the adapter is still needed when it may not be.
6769
/// </para>
6870
/// <para>
69-
/// So this test watches for the <em>proposed</em> members, not the internal one. Watching
70-
/// <c>GetBindableObjectWithFlyoutItemTemplate</c> - which is what an earlier revision did -
71-
/// would never have fired, because upstream is not planning to publish that name at all, and
72-
/// the adapter would have quietly become permanent.
71+
/// So it matches on the <em>concept</em> instead: any new public member on <see cref="Shell"/>
72+
/// that talks about a flyout item template. That fires for a resolve-style API, for the
73+
/// three-method shape, or for whatever the review settles on, without needing to be revised
74+
/// every time the design moves.
7375
/// </para>
7476
/// <para>
75-
/// The adapter stays provisional until the API is merged AND available in a referenced package;
76-
/// this test firing is the signal to adopt it, not a reason to bake it in early.
77+
/// Firing is the signal to adopt, not permission to bake the API in early: the adapter stays
78+
/// provisional until the design is merged AND present in a referenced package.
7779
/// </para>
7880
/// </remarks>
7981
[Fact]
80-
public void ShellTemplateResolverExpiresWhenShellPublishesTheFlyoutTemplateContract()
82+
public void ShellTemplateResolverExpiresWhenShellPublishesAFlyoutTemplateContract()
8183
{
8284
var shell = NeutralMaui.Controls.GetType("Microsoft.Maui.Controls.Shell");
8385

8486
Assert.NotNull(shell);
8587

86-
string[] proposed =
87-
{
88-
"IsFlyoutItemTemplateSet",
89-
"GetFlyoutItemTemplateSource",
90-
"GetFlyoutItemTemplateProperty",
91-
};
92-
93-
var landed = proposed
94-
.Where(name => shell!.GetMethod(name, BindingFlags.Public | BindingFlags.Static) is not null)
88+
var landed = shell!
89+
.GetMembers(BindingFlags.Public | BindingFlags.Static | BindingFlags.Instance)
90+
.Select(m => m.Name)
91+
.Where(IsNewFlyoutTemplateContractMember)
92+
.Distinct(StringComparer.Ordinal)
93+
.OrderBy(n => n, StringComparer.Ordinal)
9594
.ToList();
9695

9796
Assert.True(
9897
landed.Count == 0,
99-
"Shell now publishes " + string.Join(", ", landed) + " (dotnet/maui#37862). Re-point "
100-
+ "Adapters/ShellTemplateResolver.cs at the public contract alongside "
101-
+ "IShellController.GetFlyoutItemDataTemplate, delete the MenuShellItem workaround "
102-
+ "and its documented behaviour gap, and remove MAUI-TIZEN-API-0001 from "
103-
+ "Adapters/UpstreamApiRequests.cs.");
98+
"Shell now publishes a flyout item template contract (" + string.Join(", ", landed)
99+
+ ", dotnet/maui#37862). Rewrite Adapters/ShellTemplateResolver.cs onto it - the shape "
100+
+ "differs from the internal helper, so this is a rewrite rather than a rename - "
101+
+ "delete the MenuShellItem workaround and its documented behaviour gap, and remove "
102+
+ "MAUI-TIZEN-API-0001 from Adapters/UpstreamApiRequests.cs.");
104103
}
105104

106105
/// <summary>
@@ -151,4 +150,48 @@ public void WaveCDeclaresTheSecondaryActionSeamButDoesNotImplementIt()
151150
"Wave C must not implement IToolbarSecondaryActionPresenter; the alerts/gestures "
152151
+ "workstream owns the action-sheet presentation: " + string.Join(", ", implementations));
153152
}
153+
154+
/// <summary>Members that predate Wave C, so they cannot be the new contract.</summary>
155+
static readonly HashSet<string> PreexistingTemplateMembers = new(StringComparer.Ordinal)
156+
{
157+
"ItemTemplateProperty",
158+
"MenuItemTemplateProperty",
159+
"GetItemTemplate",
160+
"SetItemTemplate",
161+
"GetMenuItemTemplate",
162+
"SetMenuItemTemplate",
163+
};
164+
165+
/// <summary>
166+
/// The concept match used to detect a newly published flyout item template contract.
167+
/// </summary>
168+
internal static bool IsNewFlyoutTemplateContractMember(string memberName) =>
169+
(memberName.Contains("FlyoutItemTemplate", StringComparison.Ordinal)
170+
|| memberName.Contains("FlyoutItemDataTemplate", StringComparison.Ordinal))
171+
&& !PreexistingTemplateMembers.Contains(memberName);
172+
173+
/// <summary>
174+
/// Proves the detector actually fires. A detector is only worth having if it has been shown to
175+
/// trigger; the previous name-specific version passed happily while detecting nothing, which is
176+
/// exactly the failure this guards against.
177+
/// </summary>
178+
[Theory]
179+
// The single resolve-style API the design is currently moving toward.
180+
[InlineData("ResolveFlyoutItemTemplate", true)]
181+
// The three-method shape it is moving away from.
182+
[InlineData("IsFlyoutItemTemplateSet", true)]
183+
[InlineData("GetFlyoutItemTemplateSource", true)]
184+
[InlineData("GetFlyoutItemTemplateProperty", true)]
185+
// Anything else the review might land on.
186+
[InlineData("TryGetFlyoutItemTemplate", true)]
187+
[InlineData("GetFlyoutItemDataTemplate", true)]
188+
// Members that already existed must not trip it.
189+
[InlineData("ItemTemplateProperty", false)]
190+
[InlineData("MenuItemTemplateProperty", false)]
191+
[InlineData("GetItemTemplate", false)]
192+
[InlineData("CurrentItem", false)]
193+
[InlineData("FlyoutBehavior", false)]
194+
public void FlyoutTemplateContractDetectorMatchesTheConceptNotAName(string memberName, bool expected)
195+
=> Assert.Equal(expected, IsNewFlyoutTemplateContractMember(memberName));
196+
154197
}

0 commit comments

Comments
 (0)