[net11.0] Add public Shell flyout item template contract for external backends - #37862
[net11.0] Add public Shell flyout item template contract for external backends#37862Redth wants to merge 3 commits into
Conversation
… backends Out-of-tree Shell backends (for example the Tizen backend that now lives outside this repository) have to decide whether a flyout item uses an application supplied `DataTemplate` or the backend's own platform default view. That decision requires resolving the object that actually carries the template, which is not always the flyout item itself: menu items are backed by a pair of objects (the internal `MenuShellItem` wrapper and the public `MenuItem`) and the template can be set on either one. The logic lived in the internal `Shell.GetBindableObjectWithFlyoutItemTemplate`, so external backends could not reproduce it and menu items incorrectly fell back to the platform default template. Expose the minimal contract publicly on `Shell`: * `GetFlyoutItemTemplateProperty` - the `BindableProperty` used for an item. * `GetFlyoutItemTemplateSource` - the object that carries the template, the style class, and the binding context for an item. * `IsFlyoutItemTemplateSet` - whether `GetFlyoutItemDataTemplate` returns an application supplied template or falls back to the default flyout cell. `MenuShellItem` stays internal. All existing call sites (Shell template resolution, default flyout cell style classes, and the in-box Tizen flyout adaptor) now go through the public helpers, so flyout item, menu item, and header template selection, binding contexts, and commands are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37862Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37862" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
github-actions
Bot
added
the
area-controls-shell
…l one The ShellTemplateResolver expiry test watched for Shell.GetBindableObjectWithFlyoutItemTemplate becoming public. Upstream is not planning to publish that name: dotnet/maui#37862 proposes a different shape - IsFlyoutItemTemplateSet, GetFlyoutItemTemplateSource and GetFlyoutItemTemplateProperty, used with the already-public IShellController.GetFlyoutItemDataTemplate. As written the test could never have fired, so the adapter would have quietly become permanent - exactly the rot these tests exist to prevent. It now watches the three proposed members and names the follow-up work when they land. #37862 is OPEN, so nothing adopts it here. ShellTemplateResolver's implementation is untouched and stays provisional until the API merges and ships in a referenced package. Only the detector, the request ledger and the wave doc change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a small set of new public Shell helper APIs that let out-of-tree Shell flyout backends resolve flyout item templates (and their template “source” object) the same way in-box backends do, especially for the MenuItem/MenuShellItem wrapper case.
Changes:
- Add public static
Shellhelpers:GetFlyoutItemTemplateProperty,GetFlyoutItemTemplateSource, andIsFlyoutItemTemplateSet. - Update in-box call sites (Shell template resolution, default flyout cell style-class sourcing, and Tizen flyout item template selection) to route through the new helpers.
- Add
ShellExternalFlyoutBackendTeststo validate the contract from the perspective of an external backend.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/tests/Core.UnitTests/ShellExternalFlyoutBackendTests.cs | Adds unit tests that simulate an external Shell flyout backend using the new public helper contract. |
| src/Controls/src/Core/Shell/Shell.cs | Introduces the new public helper APIs and switches internal template resolution to use them. |
| src/Controls/src/Core/Shell/BaseShellItem.cs | Uses the new template-source helper to select the style-class source for the default flyout cell. |
| src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt | Records the new public APIs for netstandard PublicAPI validation. |
| src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt | Records the new public APIs for net PublicAPI validation. |
| src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt | Records the new public APIs for net-windows PublicAPI validation. |
| src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt | Records the new public APIs for net-tizen PublicAPI validation. |
| src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt | Records the new public APIs for net-maccatalyst PublicAPI validation. |
| src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt | Records the new public APIs for net-ios PublicAPI validation. |
| src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt | Records the new public APIs for net-android PublicAPI validation. |
| src/Controls/src/Core/Handlers/Shell/Tizen/ShellFlyoutItemAdaptor.cs | Updates Tizen flyout template selection to rely on Shell.IsFlyoutItemTemplateSet. |
| /// <summary> | ||
| /// Gets the <see cref="BindableProperty"/> that Shell uses to look up the flyout <see cref="DataTemplate"/> | ||
| /// for <paramref name="flyoutItem"/>. | ||
| /// </summary> | ||
| /// <param name="flyoutItem">A flyout item produced by <see cref="IShellController.GenerateFlyoutGrouping"/>.</param> | ||
| /// <returns> | ||
| /// <see cref="MenuItemTemplateProperty"/> for menu items, otherwise <see cref="ItemTemplateProperty"/>. | ||
| /// </returns> | ||
| /// <exception cref="ArgumentNullException"><paramref name="flyoutItem"/> is <see langword="null"/>.</exception> | ||
| public static BindableProperty GetFlyoutItemTemplateProperty(BindableObject flyoutItem) | ||
| { | ||
| if (flyoutItem is null) | ||
| throw new ArgumentNullException(nameof(flyoutItem)); | ||
|
|
||
| return flyoutItem is IMenuItemController ? MenuItemTemplateProperty : ItemTemplateProperty; | ||
| } |
There was a problem hiding this comment.
Good catch on the original revision — that concern is resolved by the redesign in 37f4a9c, which restored the helper rather than removing it.
Shell.GetBindableObjectWithFlyoutItemTemplate still exists with the identical signature, just internal and now documented:
internal static BindableObject GetBindableObjectWithFlyoutItemTemplate(BindableObject bo)So both shipped binary profiles still resolve it — verified against the artifacts that Microsoft.Maui.Controls.targets actually imports:
$ strings maui.aotprofile | grep -c GetBindableObjectWithFlyoutItemTemplate
1
$ strings maui-sc.aotprofile | grep -c GetBindableObjectWithFlyoutItemTemplate
1
IShellController.GetFlyoutItemDataTemplate is also unchanged in signature, so its profile entry (maui.aotprofile.txt:2056, maui-sc.aotprofile.txt:2575) stays valid too.
Two notes for the record:
- The
.txtfiles are human-readable dumps, not build inputs —Controls.Build.Tasks.csprojpacksnuget\**withExclude="nuget\**\*.aotprofile.txt", andMicrosoft.Maui.Controls.targetsimports the binarymaui.aotprofile/maui-sc.aotprofile. Hand-editing the.txtwould have no effect on profiled AOT. - The new
ResolveFlyoutItemTemplateis not in the profiles, which is expected: the profiles are regenerated from a recorded run viasrc/ProfiledAot, not maintained by hand. It's a small non-virtual static on a path that is already only hit while building flyout items.
…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>
Replaces the three algorithm-piece methods with one nullable-annotated,
result-oriented entry point:
public static DataTemplate? ResolveFlyoutItemTemplate(Shell? shell, BindableObject flyoutItem)
It returns the final application-defined template, or null meaning "use your
own platform-native default". Rationale for the change:
* `Get*` on Shell reads as an attached-property accessor, which these were not.
* Exposing the property/source/is-set trio froze the internal lookup levels as
public contract.
* The documented binding context was wrong. The built-in Android, iOS, and
Windows backends all bind created content to the flyout item itself
(`View.BindingContext = context` / `_content.BindingContext = bo`), not to the
object that carried the template, which for `ShellContent.MenuItems` is the
parent `ShellContent`.
`MenuShellItem` and the paired-object lookup stay internal.
Null handling is explicit: an item that sets the template property wins even
when the value is null (so an app can opt one item out of a Shell level
template), and a null or null-bound value is reported as "no template" rather
than producing a true/null mismatch or an NRE in platform code.
`IShellController.GetFlyoutItemDataTemplate` now composes the new method with
the default flyout cell, so the in-box path never hands null to platform code.
The Windows backend is dogfooded through the public API, and the in-box Tizen
adaptor now resolves and null-checks in one step.
Tests grow to 23 and add binding coverage: Text/Command bound through
`ShellContent.MenuItems` and through Shell-level `MenuItemTemplate`, flyout
selection invoking the command, item vs Shell template precedence, explicit
null, null-bound, `DataTemplateSelector`, headers/footers and their templates,
Shell resolution when the argument is omitted, unparented items, and argument
validation.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds the remaining flyout item shape an external backend can be handed: the bare MenuItem whose template is set on its (internal) MenuShellItem wrapper, reachable publicly as MenuItem.Parent, plus the no-template fallback for the same shape. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Redesigned — ready for a fresh reviewThe three-step API from the first revision is gone. Head is now Single nullable result-oriented API. One method, algorithm details internal: public static DataTemplate? ResolveFlyoutItemTemplate(Shell? shell, BindableObject flyoutItem)
Public Bind to the flyout item, not the template source. Confirmed correct: for Explicit/bound null is "no application template". An item that sets the property wins even when the value is null (so an app can opt one item out of a Shell-level template), and null or null-bound resolves to No Dogfooded on Windows. Nullable annotations + PublicAPI. Method is in a Tests — 25, public API only.
The AOT-profile comment is answered inline — the helper was restored with an identical signature, so the shipped binary profiles still resolve it. |
| // An explicitly set template wins even when its value is null, which lets an application opt a single item | ||
| // out of a Shell level template. A null value is reported as "no template" so callers fall back safely. | ||
| BindableObject templateSource = GetBindableObjectWithFlyoutItemTemplate(flyoutItem); |
…tion bug Introduces ShellFlyoutTemplateResolution.ResolveFlyoutItemTemplate, whose signature is deliberately identical to the redesigned upstream API in dotnet/maui#37862 so adoption becomes a one-line body swap. Nothing is bound to the open PR. Building the seam surfaced a real bug. The call sites passed a PRE-RESOLVED template owner to IShellController.GetFlyoutItemDataTemplate, but that method re-derives the owner itself and picks the BindableProperty from its argument's own type. Handing it the owner therefore selected ItemTemplateProperty where MenuItemTemplateProperty was authored, silently dropping MenuItemTemplate for flyout menu items - and also skipped the internal MenuShellItem branch that only that method can reach. The raw item is now passed, which is both correct and what upstream does. The seam also returns DataTemplate? rather than a template-or-default, because GetFlyoutItemDataTemplate never returns null; calling it unguarded would have replaced Tizen's own flyout item view with MAUI's generic cell for every app that never authored a template. Records Core's published shapes (163677d) including the TizenNaviPage rename and the ITizenToolbarContainer push contract, and notes that TizenShellView's pull-based toolbar wiring must be reworked on rebase since SetToolbar disposes the toolbar it replaces. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Controls/src/Core/Shell/Shell.cs:788
ResolveFlyoutItemTemplatechecksGetBindableObjectWithFlyoutItemTemplate(flyoutItem)first. For aMenuIteminShellContent.MenuItems,GetBindableObjectWithFlyoutItemTemplatereturns the parentShellContentwhen it hasMenuItemTemplatePropertyset, which means a template explicitly set on theMenuItemitself (including an explicitnullto opt out) will be ignored. If the flyout item has the template property set, it should win before consulting the paired-object lookup.
BindableProperty bp = flyoutItem is IMenuItemController ? MenuItemTemplateProperty : ItemTemplateProperty;
// An explicitly set template wins even when its value is null, which lets an application opt a single item
// out of a Shell level template. A null value is reported as "no template" so callers fall back safely.
BindableObject templateSource = GetBindableObjectWithFlyoutItemTemplate(flyoutItem);
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
Out-of-tree Shell backends need to decide whether a flyout item uses an application-supplied
DataTemplateor the backend's own platform-native presentation. That decision requires resolving the object that actually carries the template, which is not always the flyout item itself:MenuItemadded toShell.Itemsis wrapped in the internalMenuShellItem. WhenShell.MenuItemTemplateis set on theMenuItem, the wrapper never has the property set — the template lives onMenuShellItem.MenuItem.MenuIteminShellContent.MenuItemsappears in the flyout grouping directly, while the template may be set on its parentShellContent.Both branches lived in the internal
Shell.GetBindableObjectWithFlyoutItemTemplate. An external backend can only inspect the flyout item it is handed, so it concludes no template is set and incorrectly falls back to its platform default. This is a concrete blocker for the Tizen backend now maintained out-of-tree (Redth/Maui.Tizen, Wave C).New API
One result-oriented, nullable-annotated method:
Returns the final application-defined template, or
nullmeaning use your own platform-native default.MenuShellItemand the paired-object lookup stay internal, andIShellControlleris unchanged (no breaking interface addition).Precedence matches the built-in backends exactly: a template set on the item wins over one set on the Shell;
MenuItemTemplatePropertyis used for menu items andItemTemplatePropertyfor everything else.Binding the created content to
flyoutItemis what all three built-in backends do — iOSUIContainerCell.BindingContext = context, AndroidView.BindingContext = value, Windows_content.BindingContext = bo.Null semantics
An item that explicitly sets the template property wins even when the value is null, so an app can opt a single item out of a Shell-level template. A null or null-bound value is reported as "no template" rather than producing a
true+nullmismatch or an NRE in platform code.IShellController.GetFlyoutItemDataTemplateis nowResolveFlyoutItemTemplate(this, bo) ?? BaseShellItem.CreateDefaultFlyoutItemCell(bo), so the in-box path never handsnullto platform code. Previously an explicitly-null template returnednullhere, which produced a blank Windows item and an NRE on Tizen.Dogfooding
The Windows backend (
ShellFlyoutItemView) now goes through the public API and falls back to the default cell, preserving behavior. The in-box Tizen adaptor resolves and null-checks in one step.BaseShellItem.CreateDefaultFlyoutItemCellcontinues to use the internal helper for its style-class source.Changes since the first revision
Addressing review feedback, the original three methods (
GetFlyoutItemTemplateProperty,GetFlyoutItemTemplateSource,IsFlyoutItemTemplateSet) were removed in favor of the single method above, because:Get*onShellreads as an attached-property accessor, which these were not.ShellContentforShellContent.MenuItems) instead of the flyout item.Issues Fixed
Unblocks external Shell backends (Tizen) from replicating in-box flyout item template selection.
Tests
ShellExternalFlyoutBackendTests(25 tests) drives aFakeExternalShellFlyoutBackendrestricted to public API only, mirroring an out-of-tree adaptor:null; Shell-levelItemTemplate/MenuItemTemplate; item-level template winning over Shell-level;MenuItemTemplateset on theMenuItemand found through theMenuShellItemwrapper; set on the wrapper and found through the bareMenuItem; set on the parentShellContentand found through theMenuItemTextandCommandbound throughShellContent.MenuItems,Text/Commandbound through Shell-levelMenuItemTemplate,Titlebound for shell items, and flyout selection actually invoking theMenuItem.CommandDataTemplateSelectorreturned as-is and resolved by the callerFlyoutHeader,FlyoutFooter,FlyoutHeaderTemplate,FlyoutFooterTemplate, and headers not appearing as flyout itemsnullargument validationResults:
Controls.Core.UnitTestsControls.Xaml.UnitTestsControls.Corenet11.0+netstandard2.0buildnet11.0-windows*andnet11.0-tizen*cannot be compiled on this macOS box (MakePri.exe/ unrecognized tizen platform identifier), so those two backend edits are review-validated only; CI covers them.