[net11.0] Expose toolbar drawer-toggle visibility on IToolbar - #37863
[net11.0] Expose toolbar drawer-toggle visibility on IToolbar#37863Redth wants to merge 2 commits into
Conversation
External platform backends implement toolbar handlers against
`Microsoft.Maui.IToolbar`, which exposed `BackButtonVisible`, `IsVisible`
and `Title` but not the drawer (flyout / "hamburger") toggle state. That
made it impossible to render or update the drawer affordance without
forking the state into a private side table.
There were two gaps:
1. No read contract. `DrawerToggleVisible` only existed on the
Controls-level `Microsoft.Maui.Controls.Toolbar` type, not on the
`IToolbar` contract handlers receive.
2. No change notification. `ShellToolbar` and `NavigationPageToolbar`
assigned the `_drawerToggleVisible` backing field directly, so neither
`PropertyChanged` nor `Handler.UpdateValue` ever fired. Built-in
platforms only refreshed the drawer icon as a side effect of a
`BackButtonVisible` change, so a drawer-only transition (for example a
`Shell.FlyoutBehavior` or `FlyoutPage.FlyoutLayoutBehavior` change) was
silently dropped.
Changes:
- Add `bool IToolbar.DrawerToggleVisible { get; }` as a default interface
member returning `false`, keeping existing implementations source and
binary compatible. `Toolbar` already declares a public virtual
get/set property, so it implements the new member implicitly with no
binary change. netstandard2.0 declares it without a default, matching
the existing `IRefreshView.IsRefreshEnabled` pattern.
- Raise the missing notification from `ShellToolbar` and
`NavigationPageToolbar` via a new `Toolbar.NotifyPropertyChanged`
helper. The backing field is still assigned before `BackButtonVisible`
notifies and the drawer notification is raised after it, so back-button
precedence in the shared navigation slot is preserved and backends never
observe an inconsistent intermediate state.
- Register a `DrawerToggleVisible` mapping for Android and Tizen, the two
built-in platforms that render the drawer icon in the navigation slot.
Adds `ToolbarDrawerToggleTests` covering a fake external backend that only
consumes `IToolbar`, Shell and NavigationPage/FlyoutPage visibility
transitions, back-vs-drawer precedence, no-op suppression, and per-window
isolation. Four of the new tests fail without the notification fix.
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 -- 37863Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37863" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR extends the core Microsoft.Maui.IToolbar contract to expose drawer/flyout toggle visibility (DrawerToggleVisible) for external platform backends, and updates Controls toolbars to reliably notify handler mappers when the computed drawer-toggle state changes (e.g., flyout behavior/layout transitions that previously didn’t trigger updates).
Changes:
- Added
IToolbar.DrawerToggleVisible(with a DIM fallback for non-NETSTANDARD2_0) and updated Core PublicAPI baselines accordingly. - Ensured
ShellToolbarandNavigationPageToolbarnotifyDrawerToggleVisiblechanges (via a newToolbar.NotifyPropertyChangedhelper to preserve ordering). - Added unit tests covering external-backend mapping/ordering semantics, plus a design doc describing ownership and notification guarantees.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Core/src/Core/IToolbar.cs | Adds DrawerToggleVisible to the handler contract (DIM where supported). |
| src/Core/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt | Public API baseline update for the new IToolbar.DrawerToggleVisible getter. |
| src/Core/src/PublicAPI/netstandard/PublicAPI.Unshipped.txt | Public API baseline update for the new IToolbar.DrawerToggleVisible getter. |
| src/Core/src/PublicAPI/net/PublicAPI.Unshipped.txt | Public API baseline update for the new IToolbar.DrawerToggleVisible getter. |
| src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt | Public API baseline update for the new IToolbar.DrawerToggleVisible getter. |
| src/Core/src/PublicAPI/net-tizen/PublicAPI.Unshipped.txt | Public API baseline update for the new IToolbar.DrawerToggleVisible getter. |
| src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt | Public API baseline update for the new IToolbar.DrawerToggleVisible getter. |
| src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt | Public API baseline update for the new IToolbar.DrawerToggleVisible getter. |
| src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt | Public API baseline update for the new IToolbar.DrawerToggleVisible getter. |
| src/Controls/src/Core/Toolbar/Toolbar.cs | Adds NotifyPropertyChanged helper so derived toolbars can preserve update ordering while still notifying handler/subscribers. |
| src/Controls/src/Core/ShellToolbar.cs | Computes drawer-toggle visibility and explicitly notifies DrawerToggleVisible when it changes. |
| src/Controls/src/Core/NavigationPage/NavigationPageToolbar.cs | Computes drawer-toggle visibility and explicitly notifies DrawerToggleVisible when it changes. |
| src/Controls/src/Core/Toolbar/Toolbar.Mapper.cs | Registers a mapper entry for IToolbar.DrawerToggleVisible (Android/Tizen) to re-evaluate the shared navigation slot. |
| src/Controls/src/Core/Toolbar/Toolbar.Android.cs | Adds MapDrawerToggleVisible mapping to UpdateBackButton on Android. |
| src/Controls/src/Core/Toolbar/Toolbar.Tizen.cs | Adds MapDrawerToggleVisible mapping to UpdateBackButton on Tizen. |
| src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt | Public API baseline update for the new Android Toolbar.MapDrawerToggleVisible methods. |
| src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt | Public API baseline update for the new Tizen Toolbar.MapDrawerToggleVisible methods. |
| src/Controls/tests/Core.UnitTests/ToolbarDrawerToggleTests.cs | Adds unit tests validating contract exposure + external-backend notifications and ordering invariants. |
| docs/design/ToolbarDrawerToggle.md | Documents ownership, precedence, and notification behavior for DrawerToggleVisible. |
| /// <para>Changes to this value raise a handler update for the <c>DrawerToggleVisible</c> property name, so | ||
| /// platform backends can map it just like any other toolbar property.</para> | ||
| /// <para>The default implementation returns <see langword="false"/> so that existing | ||
| /// <see cref="IToolbar"/> implementations remain source and binary compatible.</para> |
Addresses review feedback on the previous commit.
The previous approach added `DrawerToggleVisible` to `IToolbar` as a
default interface member. That is a source break on netstandard2.0, where
default interface members are unsupported and the member is therefore
abstract, so any existing external `IToolbar` implementer fails with
CS0535. The `IRefreshView.IsRefreshEnabled` precedent does not justify
introducing a new break.
Replaced with `Microsoft.Maui.IToolbarDrawerToggleVisible`, an optional
read-only capability interface implemented by `Microsoft.Maui.Controls.Toolbar`
and consumed via pattern matching:
toolbar is IToolbarDrawerToggleVisible { DrawerToggleVisible: true }
`IToolbar` gains no members on any target framework, so the change is
purely additive everywhere. This mirrors `ISwipeItemMenuItemIconColor`,
which was added alongside `ISwipeItemMenuItem` for the same reason.
Verified with an external-implementer compile matrix. A type implementing
only the pre-existing `IToolbar` members compiles against the new Core on
both netstandard2.0 and net11.0, and fails with CS0535 on netstandard2.0
against the previous design.
Also fixes three contract issues found in review:
- Removed the platform-backend write in Tizen's `ShellView`, which did
`DrawerToggleVisible = DrawerToggleVisible && FlyoutBehavior == Flyout`.
That contradicted the documented framework ownership, was redundant
because `ShellToolbar` already folds `FlyoutBehavior` into the value,
latched to false since it could never restore true, and compared against
the raw bindable property rather than the effective flyout behavior. The
method now only re-renders the navigation slot.
- Reordered `ShellToolbarTracker.ApplyToolbarChanges` to assign
`BackButtonVisible` before `DrawerToggleVisible`. With a drawer mapping
now registered on Android, forwarding the drawer value first would map
the shared navigation slot against a stale back-button value.
- Dropped the incorrect mutual-exclusivity claim from the docs and tests.
On Windows `ShellToolbar` derives the drawer toggle purely from
`FlyoutBehavior`, so both values can be true simultaneously; the back
button merely takes precedence when rendering. The guarantee the
framework actually provides is ordering, and the test is renamed to
`DrawerToggleValueIsCurrentWhenBackButtonMapperRuns` to say so.
Docs now record actual built-in consumption: Android and Tizen read the
value from `ToolbarExtensions.UpdateBackButton`; Windows, iOS and
MacCatalyst render the flyout toggle from the flyout/navigation view and
never read it, which is why the mapping is registered only for the first
two.
Tests add an external implementer that omits the capability interface
(compiles, reports no drawer toggle) and one that opts into it.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Pushed Blocker confirmed and fixed. The previous head added Replaced with an optional read-only capability interface, Compile matrix, legacy implementer vs
Contract issues, all fixed:
Validation:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/Controls/tests/Core.UnitTests/ToolbarDrawerToggleTests.cs:290
LegacyExternalToolbarimplementsIElement.Parent/IElement.Handleras non-nullable, but both members are nullable onIElementand this stub returns null / is unset. This can produce nullable-mismatch warnings and makes the test double’s contract misleading. Align the annotations withIElement(IElement?/IElementHandler?).
This issue also appears on line 302 of the same file.
public string Title => string.Empty;
public IElement Parent => null;
public IElementHandler Handler { get; set; }
src/Controls/tests/Core.UnitTests/ToolbarDrawerToggleTests.cs:306
CapableExternalToolbarimplementsIElement.Parent/IElement.Handleras non-nullable, butIElementdeclares both as nullable and this test double returns null / leaves Handler unset. Align the nullability to avoid interface implementation warnings and to accurately model external implementations.
public string Title => string.Empty;
public IElement Parent => null;
public IElementHandler Handler { get; set; }
| Element.Toolbar.PropertyChanged += (s, e) => | ||
| { | ||
| if (e.PropertyName == "BackButtonVisible") | ||
| { | ||
| UpdateDrawerToggleVisible(); | ||
| RefreshNavigationSlot(); |
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
External platform backends implement toolbar handlers against
Microsoft.Maui.IToolbar. That contract exposedBackButtonVisible,IsVisibleandTitle, but nothing about the drawer (flyout / "hamburger") toggle — so a backend could not render or update the drawer affordance without forking the state into a private side table.There were two gaps:
DrawerToggleVisibleonly existed on the Controls-levelMicrosoft.Maui.Controls.Toolbartype, not on the contract handlers are handed.ShellToolbarandNavigationPageToolbarassign the_drawerToggleVisiblebacking field directly, so neitherPropertyChangednorHandler.UpdateValueever fired. Built-in platforms only refreshed the drawer icon as a side effect of aBackButtonVisiblechange, so a drawer-only transition was silently dropped.Gap 2 is a latent framework bug, not just an extensibility problem. Cases that did not update before this PR:
Shell.FlyoutBehaviortoggling betweenFlyoutandDisabled/Lockedwith the navigation stack unchanged.FlyoutPage.FlyoutLayoutBehaviorchanging betweenSplitandPopoveron a tablet.ShellViewassigningToolbar.DrawerToggleVisible— the setter calledHandler.UpdateValue("DrawerToggleVisible")against a mapper with no such key.API: optional capability interface
Microsoft.Maui.Controls.Toolbarimplements it. External backends consume it by pattern matching:Why not a member on
IToolbar. An earlier revision of this PR addedDrawerToggleVisibletoIToolbaras a default interface member. That is a source break on netstandard2.0, where DIMs are unsupported and the member is therefore abstract — every existing externalIToolbarimplementer fails withCS0535. A capability interface is purely additive on every TFM instead. This mirrorsISwipeItemMenuItemIconColor, whose doc comment cites exactly this rationale.The interface is intentionally not marked
[EditorBrowsable(Never)](unlikeISwipeItemMenuItemIconColor, which is a stopgap): external backend authors are the intended consumer and need to discover it.It is read-only because the value is computed and owned by the cross-platform layer.
Controls.Toolbarkeeps its shipped settable property, but the framework owns the value for the Shell andNavigationPagetoolbars.External-implementer compile matrix
A type implementing only the pre-existing
IToolbarmembers, compiled againstMicrosoft.Maui:IToolbar)CS0535: 'LegacyExternalToolbar' does not implement interface member 'IToolbar.DrawerToggleVisible'Reproduced locally by building
src/Core/src/Core.csprojfor each TFM and compiling a standalone library that references the resultingMicrosoft.Maui.dlland declares:IToolbar's member set is unchanged on every TFM, so there is nothing left to break; the PublicAPI diff contains noMicrosoft.Maui.IToolbar.*additions.Reliable notification
private protected Toolbar.NotifyPropertyChanged(string)helper (SetPropertyroutes through it).ShellToolbar.ApplyChangesandNavigationPageToolbar.UpdateBackButtoncompute the drawer value into a local, still assign the backing field beforeBackButtonVisiblenotifies, and raise theDrawerToggleVisiblenotification after it. This preserves the existing ordering contract that Android's animated back/drawer handling depends on.Back button precedence — not mutual exclusion
An earlier revision of this PR claimed the two are mutually exclusive. That was wrong and has been removed from the docs and tests. On Windows
ShellToolbarderives the drawer toggle purely fromFlyoutBehaviorwhileBackButtonVisibleis independent, so both can betrueat once.What the framework guarantees is precedence (they share one navigation slot, back wins at render time) and ordering (the drawer value is settled before
BackButtonVisiblenotifies). The test is renamedDrawerToggleValueIsCurrentWhenBackButtonMapperRunsto state the real invariant.Ownership fix — Tizen
ShellView.UpdateDrawerToggleVisible()did:A platform backend writing a framework-computed value, contradicting the documented ownership. It was also:
ShellToolbaralready foldsFlyoutBehaviorinto the value;X = X && condcan never restoretrue;Shell.FlyoutBehaviorbindable property rather than the effective behaviorShellToolbaruses.The write is removed; the method is renamed
RefreshNavigationSlot()and now only re-renders. The manual_toolbar?.UpdateBackButton(...)refresh is deliberately retained, because_toolbaris injected viaSetToolbar(MauiToolbar)and I could not verify locally that it is the same instance theToolbarHandlerdrives.Ordering fix — Android
ShellToolbarTracker.ApplyToolbarChangesforwardedDrawerToggleVisiblebeforeBackButtonVisible. That was inert before this PR (no mapping existed), but with a drawer mapping registered it would map the shared navigation slot against a stale back-button value. Reordered soBackButtonVisibleis assigned first.Built-in platform consumption
ToolbarExtensions.UpdateBackButtonpicks aDrawerArrowDrawable(Progress = 0) when back is hidden and the drawer toggle is visible. Shell additionally drives anActionBarDrawerTogglefromShellToolbarTracker.ToolbarExtensions.UpdateBackButtoninstalls a menu button;UpdateTitleIconavoids clearing the icon while it is visible.NavigationView.The mapping is registered for Android and Tizen only. Both it and the
BackButtonVisiblemapping callUpdateBackButton, so a change flipping both runs it twice — intentional and idempotent.Tests
src/Controls/tests/Core.UnitTests/ToolbarDrawerToggleTests.cs(11 tests):ExternalToolbarBackendHandler : ElementHandler<IToolbar, object>, written againstIToolbaronly, never referencingControls.Toolbar. It reads drawer state solely through the capability-interface pattern match, so anything it can do an external assembly can do.LegacyExternalToolbar— implements only the pre-existingIToolbarmembers, deliberately not the capability interface: compiles and reports no drawer toggle.CapableExternalToolbar— opts in and reports its value.FlyoutBehaviortransitions notify;Disabled → Lockeddoes not (no spurious updates).NavigationPageinsideFlyoutPage: push/pop plusFlyoutLayoutBehavior.Split → Popover.Four tests fail without the notification fix, verified by reverting
ShellToolbar.cs+NavigationPageToolbar.cs:Validation
Controls.Core.UnitTests: 6218 passed / 0 failed / 30 skipped.Core.csprojbuilds clean fornetstandard2.0andnet11.0;Controls.Core.csprojbuilds clean fornet11.0-android37.0(mapper + PublicAPI analyzers).NETSDK1139— workload not installed on this machine). The Tizen edits are a method rename with all three references updated plus removal of one assignment, and theToolbar.Tizen.csmapper mirrors the Android one that does compile — but CI must confirm the Tizen leg.Docs
docs/design/ToolbarDrawerToggle.md— why a separate interface, ownership table, precedence-not-exclusion, the ordering guarantee (including theApplyToolbarChangesassignment-order requirement), a copy-pasteable external handler, and the built-in consumption table.Issues Fixed
Unblocks external MAUI platform backends that currently need a private
ConditionalWeakTableadapter to track drawer toggle state.