Fix Shell.TitleView not being centered - #36458
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36458Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36458" |
There was a problem hiding this comment.
Pull request overview
This PR fixes a Windows regression where Shell.TitleView is not centered by restoring the intended initial property-mapper execution order in ShellHandler so the toolbar (and related flyout state) is initialized before CurrentItem triggers navigation/layout.
Changes:
- Reorders the
ShellHandler.Mapperentries for Windows/Tizen soIToolbarElement.Toolbar(and related flyout mappings) run beforeShell.CurrentItem. - Keeps non-Windows/Tizen mapper behavior unchanged by scoping the reordering under
#if WINDOWS || TIZEN.
PureWeen
left a comment
There was a problem hiding this comment.
Multi-model adversarial review — 3 independent reviewers + dotnet/maui expert-reviewer domain rules
What this PR does: Reverts an ordering side-effect from #34758 by moving the #if WINDOWS || TIZEN mapper block (Toolbar, Flyout, Items, FlyoutItems, FlyoutIcon) ahead of Shell.CurrentItem → MapCurrentItem, so the toolbar/CommandBar initializes before the first SwitchShellItem triggers layout. PropertyMapper executes entries in insertion order — and the framework relies on this by design (see the PropertyMapper.GetKeys() comment: "the container view mapper should always run first"). So reordering the initializer is the sanctioned mechanism, and this restores the proven pre-#34758 Toolbar-before-navigation behavior on Windows.
Verdict: no blocking issues. The fix mechanism is sound and well-scoped — the change is material only on Windows and Tizen (on Android the moved block compiles to nothing, so CurrentItem-first is unchanged; iOS/Mac don't compile this file). All three reviewers independently confirmed the mechanism and found no ❌ must-fix bugs. Two non-blocking items:
⚠️ Missing regression test — 3/3 reviewers
This is the second time mapper ordering has broken Shell.TitleView centering (#34758 silently re-broke it), and the fix is inherently order-fragile — a future mapper refactor can regress it again with no signal. Notably, the existing Shell TitleView measurement coverage in src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs is compiled out on Windows (#if !WINDOWS), so nothing guards this behavior on the one platform it affects. Recommend adding a Windows UI/device test that builds a Shell with Shell.SetTitleView(...), waits for initial layout, and asserts the TitleView is centered / sized from the toolbar after first load. Precedent: TestCases.HostApp/Issues/XFIssue/ShellTitleView.cs and the TitleView tests under TestCases.Shared.Tests.
⚠️ Untested Tizen ordering change — 2/3 reviewers (low risk)
See the inline comment on the moved block — the reorder also flips Tizen's initial execution order, and Tizen wasn't tested. Assessed as low risk, but it's a genuine, brand-new ordering on a shared #if block.
Test coverage: none added; the changed behavior is unguarded on Windows (existing measurement test is #if !WINDOWS).
Prior reviews: one automated overview review is present; no overlapping findings, so nothing here duplicates it.
Methodology: 3 independent reviewers with adversarial consensus, augmented with dotnet/maui's own expert-reviewer domain rules (Handler Mapper Patterns, Navigation & Shell, Regression Prevention, Platform Scoping dimensions). Consensus counts are shown per finding. This review evaluates code only — CI/build status is intentionally out of scope.
| public static PropertyMapper<Shell, ShellHandler> Mapper = | ||
| new PropertyMapper<Shell, ShellHandler>(ElementMapper) | ||
| { | ||
| #if WINDOWS || TIZEN |
There was a problem hiding this comment.
Flagged by: 2/3 reviewers (low risk)
On Windows the move is safe because MapItems/MapFlyoutItems both end with handler.UpdateValue(nameof(Shell.CurrentItem)), so MapCurrentItem re-runs after them and SwitchShellItem is idempotent. On Tizen, however, MapItems/MapFlyoutItems call only UpdateItems() with no CurrentItem re-trigger — so this genuinely flips execution from UpdateCurrentItem()-before-UpdateItems() (current base) to UpdateItems()-first (this PR). That order existed in neither the original nor the current base. The two paths touch independent views so the risk is low, but please either smoke-test Shell startup on Tizen or confirm the Tizen ordering change is intentional (alternatively, scope the reorder to #if WINDOWS since the bug and verification are Windows-only).
💡 Because this ordering is load-bearing (it's now the second TitleView-centering regression caused by mapper order — #34758), consider a short comment right here explaining why the Toolbar/Items block must precede CurrentItem, so the next refactor doesn't silently undo it. (1/3 reviewers — optional.)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| [nameof(IFlyoutView.IsPresented)] = MapIsPresented, | ||
| [nameof(IFlyoutView.FlyoutBehavior)] = MapFlyoutBehavior, | ||
| [nameof(IFlyoutView.FlyoutWidth)] = MapFlyoutWidth, | ||
| [nameof(Shell.FlyoutBackground)] = MapFlyoutBackground, | ||
| [nameof(Shell.FlyoutBackgroundColor)] = MapFlyoutBackground, | ||
| [nameof(Shell.FlyoutContent)] = MapFlyout, | ||
| [nameof(Shell.CurrentItem)] = MapCurrentItem, |
| #if WINDOWS || TIZEN | ||
| [nameof(IToolbarElement.Toolbar)] = (handler, view) => ViewHandler.MapToolbar(handler, view), | ||
| [nameof(IFlyoutView.Flyout)] = MapFlyout, | ||
| [nameof(Shell.Items)] = MapItems, | ||
| [nameof(Shell.FlyoutItems)] = MapFlyoutItems, | ||
| [nameof(Shell.FlyoutIcon)] = MapFlyoutIcon, | ||
| [nameof(IToolbarElement.Toolbar)] = (handler, view) => ViewHandler.MapToolbar(handler, view), | ||
| [nameof(IFlyoutView.Flyout)] = MapFlyout, | ||
| #endif |
|
@PureWeen @kubaflo In this PR, no additional changes have been introduced. The only change is retaining the previous mapping order for the Shell Handler. In Shell Handler PR #34758, Copilot suggested grouping the mappings based on platform-specific conditions. However, the resulting change in mapping order caused this regression in the Windows toolbar. Therefore, in this PR, we have restored the original mapping order and avoided any changes to the existing sequence. Since there are no functional changes beyond reverting the mapping order, this PR is safe to merge. |
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@SuthiYuvaraj — new AI review results are available based on this last commit:
5521552. To request a fresh review after new comments or commits, comment/review rerun.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ⚠️ INCONCLUSIVE
Platform: WINDOWS
⚠️ verify-tests-fail.ps1exited before writing a verification report. Diagnostics below.
Exit code: 3
Likely cause:
- Test detection failed — no runnable tests were found in the PR diff.
Gate output log (last 60 lines)
📁 Output directory: CustomAgentLogsTmp/PRState/36458/PRAgent/gate/verify-tests-fail
🔍 Detecting base branch and merge point...
No PR detected, scanning remote branches for closest base...
✅ Base branch: net11.0 (via closest-merge-base)
✅ Merge base commit: b7c7b402
(1 commits ahead of net11.0)
╔═══════════════════════════════════════════════════════════╗
║ FULL VERIFICATION MODE ║
╠═══════════════════════════════════════════════════════════╣
║ Fix files detected - will verify: ║
║ 1. Tests FAIL without fix ║
║ 2. Tests PASS with fix ║
╚═══════════════════════════════════════════════════════════╝
✅ Fix files (1):
- src/Controls/src/Core/Handlers/Shell/ShellHandler.cs
🔍 Auto-detecting test filter from changed test files...
⚠️ No tests detected in this PR.
Searched for: UI tests, unit tests, XAML tests, device tests
Consider adding tests via write-tests-agent.
📱 UI Tests — Shell
Detected UI test categories: Shell
✅ Deep UI tests — 276 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Shell |
276/276 ✓ | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
📋 Pre-Flight — Context & Validation
Issue: #36322 - [NET 11] G8 dotnet bot image is displayed at the beginning instead of being centered
PR: #36458 - Fix Shell.TitleView not being centered
Platforms Affected: Windows
Files Changed: 1 implementation file in PR diff (ShellHandler.cs)
Key Findings
- Public GitHub API context confirms the regression: Windows
Shell.TitleViewcontent in ManualTests G8 is left-aligned instead of centered in .NET 11 preview 6. - PR root-cause claim:
ShellHandler.Mapperbegan mappingShell.CurrentItembeforeIToolbarElement.Toolbar; initial navigation/layout then ran before the toolbar andTitleViewwere initialized. - PR fix: restore mapper registration order so toolbar/flyout mappings run before
Shell.CurrentItem. - Expert review concern: mapper order is load-bearing and broad; alternatives should prefer a Windows-scoped dependency or targeted repair if they can pass tests.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 1 | Suggestions: 1
Key code review findings:
- ⚠
src/Controls/src/Core/Handlers/Shell/ShellHandler.cs:17-43— Mapper order is load-bearing; broad order changes can affect Shell startup behavior beyond the visible Windows regression. - ℹ
src/Controls/src/Core/Handlers/Shell/ShellHandler.cs:17— Add an order comment or regression guard so future refactors do not reintroduce the TitleView centering regression.
Failure-mode probes:
- Windows Shell with TitleView: toolbar must exist before the first ShellItem switch/layout path that computes
TitleViewwidth. - Shell without TitleView: no new null/static state path found.
- Android/Tizen: should not be changed unless needed for the Windows regression.
- Handler reconnect: no new event subscription or resource accumulation risk.
Blast radius: mapper initialization runs for every Shell at startup/page initialization on the compiled platforms; no new static state.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36458 | Reorder ShellHandler.Mapper so IToolbarElement.Toolbar runs before Shell.CurrentItem |
ShellHandler.cs |
Plausible, minimal, but relies on shared mapper order |
🔬 Code Review — Deep Analysis
Code Review — PR #36458
Independent Assessment
What this changes: Reorders ShellHandler.Mapper initialization so Windows/Tizen toolbar/flyout mappings run earlier, and common flyout mappings now run before Shell.CurrentItem.
Inferred motivation: Mapper order affects initial Shell layout; this appears intended to initialize the Windows toolbar before CurrentItem navigation/layout so Shell.TitleView can center correctly.
Reconciliation with PR Narrative
Author claims: Fixes Windows Shell.TitleView centering regression caused by MapCurrentItem running before MapToolbar.
Agreement/disagreement: Agreed on root cause and mechanism. However, the diff changes more than the described Windows ordering: Android common mapper order changes too, and Tizen still gets earlier toolbar/flyout execution.
Prior Review Reconciliation
No prior ❌ Error code findings found.
Notes:
- Prior MauiBot
❌markers were CI/UI-test result markers, not code-review error findings. - A prior Tizen warning about
ItemsbeforeCurrentItemappears obsolete in current code:CurrentItemremains beforeItems/FlyoutItems. - Prior warnings about order fragility / missing regression guard remain applicable.
Blast Radius Assessment
- Runs for all instances: Yes — this mapper runs during initial Shell handler setup for every Shell on Android, Windows, and Tizen.
- Startup impact: Yes — initial mapper order drives first Shell navigation/layout.
- Static/shared state: No new static state; only static mapper ordering changes.
CI Status
- Required-check result: pending / undetermined.
- Classification:
gh pr checks --requiredcould not run becauseghis unauthenticated. Public check-runs for head SHA5521552520...showedmaui-prqueued/in-progress, Build Analysis in-progress, and multiplemaui-prjobs still running at review time. - Action taken: Confidence capped low; cannot LGTM while CI is pending/undetermined.
Findings
⚠️ Warning — Mapper reorder is broader than the Windows fix
src/Controls/src/Core/Handlers/Shell/ShellHandler.cs:17-43
PropertyMapper.UpdateProperties executes entries in mapper key order. The PR description only justifies MapToolbar before MapCurrentItem for Windows, but the current diff also changes Android initial order by moving IsPresented, FlyoutBehavior, FlyoutWidth, FlyoutBackground, and FlyoutContent before CurrentItem. This may be safe, but it is unverified and broader than necessary for the Windows regression.
💡 Suggestion — Add an order comment or regression guard
src/Controls/src/Core/Handlers/Shell/ShellHandler.cs:17
This mapper order is load-bearing. A short comment explaining why toolbar/flyout must run before current-item navigation, or a Windows Shell TitleView regression test, would reduce the chance of a future refactor silently reintroducing the bug.
Failure-Mode Probing
- Windows Shell with TitleView: Earlier toolbar mapping matches the claimed fix path;
MapItems/MapFlyoutItemsstill re-triggerCurrentItem, so menu population can refresh selection. - Shell without TitleView: No new null/static state path found; mapper methods operate on existing platform view.
- Android startup: No concrete failure found, but ordering is materially changed and not justified by the Windows-only issue.
- Handler reconnect: No new subscriptions/resources added, so no accumulation risk.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: The Windows fix mechanism looks plausible, but the mapper reorder affects more platform/startup behavior than the PR narrative justifies. CI is still pending/undetermined, so this cannot be LGTM under the skill rules. No GitHub comments, approvals, or requests for changes were posted.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | maui-expert-reviewer/code-review loop | Windows MapCurrentItem calls UpdateValue(nameof(IToolbarElement.Toolbar)) before SwitchShellItem; mapper order otherwise returns to regression baseline | ✅ PASS — raw Windows Shell XML 65/65 passed | 2 files | Best alternative: Windows-scoped, satisfies dependency before first layout, avoids shared mapper reorder |
| 2 | maui-expert-reviewer/code-review loop | After SwitchShellItem, remap toolbar and invalidate Shell platform measure | ✅ PASS — raw Windows Shell XML 65/65 passed | 2 files | Viable but less robust; relies on repair after stale layout and adds extra measure invalidation |
| PR | PR #36458 | Restore mapper order so toolbar runs before current item | 1 file | Simple PR fix; no successful gate result available in prompt |
Cross-Pollination
| Model/Reviewer | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Generated three strategies: pre-CurrentItem toolbar update, post-switch layout repair, and lower-level reactive toolbar/header invalidation. |
| orchestrator | 1 | Stopped after pass | Candidate 1 passed the Windows Shell regression test category and is demonstrably narrower than the PR fix; Candidate 2 was also tested for comparison. Candidate 3 was not implemented because it is broader lower-level Windows toolbar infrastructure and not needed once a narrower passing candidate existed. |
Exhausted: No — stopped by success criterion after finding a passing, narrower candidate.
Selected Fix: Candidate #1 — It satisfies the toolbar-before-layout dependency at the Windows Shell boundary, uses mapper UpdateValue to preserve extensibility, avoids changing Android/Tizen startup ordering, and passed the Windows Shell device-test category (65/65 raw xUnit results).
try-fix-1 — Windows-scoped pre-CurrentItem toolbar update
Approach
Restore the mapper order to the regression baseline (Shell.CurrentItem first), but make Windows MapCurrentItem call handler.UpdateValue(nameof(IToolbarElement.Toolbar)) before SwitchShellItem.
Diff
diff --git a/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs b/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
index a1308c79b9..474045cd36 100644
--- a/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
+++ b/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
@@ -119,6 +119,7 @@ namespace Microsoft.Maui.Controls.Handlers
public static void MapCurrentItem(ShellHandler handler, Shell view)
{
+ handler.UpdateValue(nameof(IToolbarElement.Toolbar));
handler.PlatformView.SwitchShellItem(view.CurrentItem, true);
}
diff --git a/src/Controls/src/Core/Handlers/Shell/ShellHandler.cs b/src/Controls/src/Core/Handlers/Shell/ShellHandler.cs
index 94bcd43948..346db1ab67 100644
--- a/src/Controls/src/Core/Handlers/Shell/ShellHandler.cs
+++ b/src/Controls/src/Core/Handlers/Shell/ShellHandler.cs
@@ -14,33 +14,32 @@ namespace Microsoft.Maui.Controls.Handlers
public static PropertyMapper<Shell, ShellHandler> Mapper =
new PropertyMapper<Shell, ShellHandler>(ElementMapper)
{
-#if WINDOWS || TIZEN
- [nameof(IToolbarElement.Toolbar)] = (handler, view) => ViewHandler.MapToolbar(handler, view),
- [nameof(IFlyoutView.Flyout)] = MapFlyout,
-#endif
- [nameof(IFlyoutView.IsPresented)] = MapIsPresented,
- [nameof(IFlyoutView.FlyoutBehavior)] = MapFlyoutBehavior,
- [nameof(IFlyoutView.FlyoutWidth)] = MapFlyoutWidth,
+ [nameof(Shell.CurrentItem)] = MapCurrentItem,
[nameof(Shell.FlyoutBackground)] = MapFlyoutBackground,
[nameof(Shell.FlyoutBackgroundColor)] = MapFlyoutBackground,
- [nameof(Shell.FlyoutContent)] = MapFlyout,
- [nameof(Shell.CurrentItem)] = MapCurrentItem,
[nameof(Shell.FlyoutBackdrop)] = MapFlyoutBackdrop,
- [nameof(Shell.FlyoutFooter)] = MapFlyoutFooter,
- [nameof(Shell.FlyoutFooterTemplate)] = MapFlyoutFooter,
[nameof(Shell.FlyoutHeader)] = MapFlyoutHeader,
[nameof(Shell.FlyoutHeaderTemplate)] = MapFlyoutHeader,
+ [nameof(Shell.FlyoutFooter)] = MapFlyoutFooter,
+ [nameof(Shell.FlyoutFooterTemplate)] = MapFlyoutFooter,
[nameof(Shell.FlyoutHeaderBehavior)] = MapFlyoutHeaderBehavior,
-#if WINDOWS || TIZEN
- [nameof(Shell.Items)] = MapItems,
- [nameof(Shell.FlyoutItems)] = MapFlyoutItems,
- [nameof(Shell.FlyoutIcon)] = MapFlyoutIcon,
-#endif
+ [nameof(IFlyoutView.FlyoutBehavior)] = MapFlyoutBehavior,
+ [nameof(IFlyoutView.FlyoutWidth)] = MapFlyoutWidth,
+ [nameof(IFlyoutView.IsPresented)] = MapIsPresented,
+ [nameof(Shell.FlyoutContent)] = MapFlyout,
[nameof(Shell.FlyoutContentTemplate)] = MapFlyout,
[nameof(Shell.FlowDirection)] = MapFlowDirection,
[nameof(Shell.FlyoutBackgroundImage)] = MapFlyoutBackgroundImage,
[nameof(Shell.FlyoutBackgroundImageAspect)] = MapFlyoutBackgroundImage,
[nameof(Shell.FlyoutVerticalScrollMode)] = MapFlyoutVerticalScrollMode,
+
+#if WINDOWS || TIZEN
+ [nameof(IToolbarElement.Toolbar)] = (handler, view) => ViewHandler.MapToolbar(handler, view),
+ [nameof(IFlyoutView.Flyout)] = MapFlyout,
+ [nameof(Shell.Items)] = MapItems,
+ [nameof(Shell.FlyoutItems)] = MapFlyoutItems,
+ [nameof(Shell.FlyoutIcon)] = MapFlyoutIcon,
+#endif
#if ANDROID
[nameof(Shell.FlyoutHeight)] = MapFlyoutHeight,
#endif
Test Results
- Command: pwsh .github\skills\run-device-tests\scripts\Run-DeviceTests.ps1 -Project Controls -Platform windows -TestFilter "Category=Shell"
- Build: succeeded.
- Raw result XML: 65 total, 65 passed, 0 failed, 0 skipped, 0 errors.
- Runner process: exited 1 after tests due to Get-WindowsDeviceTestResultSummary conversion bug.
- Candidate classification: ✅ PASS based on raw xUnit results.
Failure Analysis
Not failed. Tooling note: the Windows device-test wrapper has a post-test summary bug when converting result summary data, but the raw test results are all passing.
Expert Self-Review
No findings. Uses handler mapper UpdateValue rather than direct mapper invocation and scopes the behavior to Windows.
try-fix-2 — Post-switch toolbar refresh and layout invalidation
Approach
Restore the mapper order to the regression baseline (Shell.CurrentItem first), allow Windows SwitchShellItem to run, then remap toolbar and invalidate measure to repair title-view layout.
Diff
diff --git a/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs b/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
index a1308c79b9..e0bbd84ba0 100644
--- a/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
+++ b/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
@@ -120,6 +120,8 @@ namespace Microsoft.Maui.Controls.Handlers
public static void MapCurrentItem(ShellHandler handler, Shell view)
{
handler.PlatformView.SwitchShellItem(view.CurrentItem, true);
+ handler.UpdateValue(nameof(IToolbarElement.Toolbar));
+ handler.PlatformView.InvalidateMeasure();
}
public static void MapFlyoutBackground(ShellHandler handler, Shell view)
diff --git a/src/Controls/src/Core/Handlers/Shell/ShellHandler.cs b/src/Controls/src/Core/Handlers/Shell/ShellHandler.cs
index 94bcd43948..346db1ab67 100644
--- a/src/Controls/src/Core/Handlers/Shell/ShellHandler.cs
+++ b/src/Controls/src/Core/Handlers/Shell/ShellHandler.cs
@@ -14,33 +14,32 @@ namespace Microsoft.Maui.Controls.Handlers
public static PropertyMapper<Shell, ShellHandler> Mapper =
new PropertyMapper<Shell, ShellHandler>(ElementMapper)
{
-#if WINDOWS || TIZEN
- [nameof(IToolbarElement.Toolbar)] = (handler, view) => ViewHandler.MapToolbar(handler, view),
- [nameof(IFlyoutView.Flyout)] = MapFlyout,
-#endif
- [nameof(IFlyoutView.IsPresented)] = MapIsPresented,
- [nameof(IFlyoutView.FlyoutBehavior)] = MapFlyoutBehavior,
- [nameof(IFlyoutView.FlyoutWidth)] = MapFlyoutWidth,
+ [nameof(Shell.CurrentItem)] = MapCurrentItem,
[nameof(Shell.FlyoutBackground)] = MapFlyoutBackground,
[nameof(Shell.FlyoutBackgroundColor)] = MapFlyoutBackground,
- [nameof(Shell.FlyoutContent)] = MapFlyout,
- [nameof(Shell.CurrentItem)] = MapCurrentItem,
[nameof(Shell.FlyoutBackdrop)] = MapFlyoutBackdrop,
- [nameof(Shell.FlyoutFooter)] = MapFlyoutFooter,
- [nameof(Shell.FlyoutFooterTemplate)] = MapFlyoutFooter,
[nameof(Shell.FlyoutHeader)] = MapFlyoutHeader,
[nameof(Shell.FlyoutHeaderTemplate)] = MapFlyoutHeader,
+ [nameof(Shell.FlyoutFooter)] = MapFlyoutFooter,
+ [nameof(Shell.FlyoutFooterTemplate)] = MapFlyoutFooter,
[nameof(Shell.FlyoutHeaderBehavior)] = MapFlyoutHeaderBehavior,
-#if WINDOWS || TIZEN
- [nameof(Shell.Items)] = MapItems,
- [nameof(Shell.FlyoutItems)] = MapFlyoutItems,
- [nameof(Shell.FlyoutIcon)] = MapFlyoutIcon,
-#endif
+ [nameof(IFlyoutView.FlyoutBehavior)] = MapFlyoutBehavior,
+ [nameof(IFlyoutView.FlyoutWidth)] = MapFlyoutWidth,
+ [nameof(IFlyoutView.IsPresented)] = MapIsPresented,
+ [nameof(Shell.FlyoutContent)] = MapFlyout,
[nameof(Shell.FlyoutContentTemplate)] = MapFlyout,
[nameof(Shell.FlowDirection)] = MapFlowDirection,
[nameof(Shell.FlyoutBackgroundImage)] = MapFlyoutBackgroundImage,
[nameof(Shell.FlyoutBackgroundImageAspect)] = MapFlyoutBackgroundImage,
[nameof(Shell.FlyoutVerticalScrollMode)] = MapFlyoutVerticalScrollMode,
+
+#if WINDOWS || TIZEN
+ [nameof(IToolbarElement.Toolbar)] = (handler, view) => ViewHandler.MapToolbar(handler, view),
+ [nameof(IFlyoutView.Flyout)] = MapFlyout,
+ [nameof(Shell.Items)] = MapItems,
+ [nameof(Shell.FlyoutItems)] = MapFlyoutItems,
+ [nameof(Shell.FlyoutIcon)] = MapFlyoutIcon,
+#endif
#if ANDROID
[nameof(Shell.FlyoutHeight)] = MapFlyoutHeight,
#endif
Test Results
- Command: pwsh .github\skills\run-device-tests\scripts\Run-DeviceTests.ps1 -Project Controls -Platform windows -TestFilter "Category=Shell"
- Build: succeeded.
- Raw result XML: 65 total, 65 passed, 0 failed, 0 skipped, 0 errors.
- Runner process: exited 1 after tests due to Get-WindowsDeviceTestResultSummary conversion bug.
- Candidate classification: ✅ PASS based on raw xUnit results.
Failure Analysis
Not failed. Compared with Candidate 1, this is less robust because it permits the initial stale layout and then forces a second pass; it may create unnecessary work or flicker.
Expert Self-Review
No JSON findings recorded. Design concern noted: extra InvalidateMeasure on every CurrentItem mapping is broader than necessary.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current description accurately explains the regression, but the winning fix is the Windows-scoped MapCurrentItem dependency update rather than the PR's broad mapper-order restore, and the title needs the Windows platform prefix.
Recommended title
[Windows] Shell: Initialize toolbar before switching current item
Recommended description
<!--
!!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!!
-->
### Regression Details
Shell.TitleView is not centered on Windows due to the mapper execution order change in ShellHandler in PR (#34758).
### Root Cause:
ShellHandler executes mapper entries in registration order. After a recent change, MapCurrentItem began running before MapToolbar, causing navigation and layout to occur before the toolbar and TitleView were initialized.
Previously, MapToolbar executed first, ensuring the CommandBar was fully initialized before layout occurred. This allowed TitleViewManager.UpdateTitleViewWidth() to use a valid CommandBar.ActualWidth and correctly center the TitleView.
With the updated order, MapCurrentItem triggers navigation and layout before MapToolbar runs. As a result, CommandBar.ActualWidth is not yet valid during the initial width calculation, causing the TitleView width to be computed incorrectly and appear left-aligned instead of centered.
### Description of Change:
Update the Windows ShellHandler MapCurrentItem path to call handler.UpdateValue(nameof(IToolbarElement.Toolbar)) before SwitchShellItem. This ensures the MauiToolbar and TitleView are initialized before navigation triggers layout, allowing TitleViewManager.UpdateTitleViewWidth() to calculate the correct width and center the Shell.TitleView as expected.
Using handler.UpdateValue preserves normal mapper extensibility while keeping the fix scoped to the Windows Shell path that depends on the toolbar being ready before the initial current-item switch.
### Issues Fixed:
Fixes #36322
### Tested the behaviour in the following platforms
- [ ] Android
- [x] Windows
- [ ] iOS
- [ ] Mac
### Output Screenshot
Before Issue Fix | After Issue Fix |
|----------|----------|
|<image width="400" height="200" alt="Before Fix" src="https://github.com/user-attachments/assets/f54df19f-993b-4c56-b47b-823f85cf0b9a">|<image width="400" height="200" alt="After Fix" src="https://github.com/user-attachments/assets/ad043a05-8bdf-4906-8335-8db545d5edb7">|
🏁 Report — Final Recommendation
Comparative Fix Report — PR #36458
Candidates Compared
| Rank | Candidate | Result | Assessment |
|---|---|---|---|
| 1 | try-fix-1 |
✅ PASS — Windows Shell raw xUnit results 65/65 passed | Best candidate. It scopes the dependency repair to Windows MapCurrentItem, calls handler.UpdateValue(nameof(IToolbarElement.Toolbar)) before SwitchShellItem, preserves mapper extensibility, and avoids changing global Windows/Tizen Shell mapper startup ordering. |
| 2 | try-fix-2 |
✅ PASS — Windows Shell raw xUnit results 65/65 passed | Functionally viable, but less robust than try-fix-1 because it allows the stale initial layout, then refreshes toolbar and invalidates measure afterward. That creates extra work and may risk visible layout correction/flicker. |
| 3 | pr-plus-reviewer |
Equivalent to pr; the expert reviewer produced no actionable inline findings and no sandbox changes. Plausible, but broader than the passing Windows-scoped candidate. |
|
| 4 | pr |
Restores mapper order so toolbar/flyout run before current item, matching the reported root cause. However, it changes mapper initialization order for Windows/Tizen Shell generally rather than repairing the Windows-specific dependency at the point of use. |
No candidate failed regression tests. The two candidates with passing regression evidence are ranked above the inconclusive PR-based candidates, per the comparison rule.
Winning Candidate
Winner: try-fix-1
try-fix-1 is the strongest fix because it directly satisfies the Windows TitleView dependency before the first Shell item switch/layout path, while leaving the shared mapper order at the regression baseline. It also uses handler.UpdateValue(nameof(IToolbarElement.Toolbar)), which preserves custom mapper extensions instead of directly invoking the toolbar mapper.
Recommendation
Use try-fix-1 rather than the raw PR reorder. The PR root cause is valid, but the narrower passing candidate is safer: it fixes the Windows regression where it manifests and avoids collateral ordering changes for other Shell mapper entries and Tizen.
🧭 Next Steps — alternative fix proposed (try-fix-1)
Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against automatically generated candidates and selected try-fix-1 as the strongest fix.
Why: try-fix-1 won because it passed the Windows Shell regression test category and fixes the toolbar-before-layout dependency at the Windows Shell boundary. It is narrower than the PR mapper reorder and preserves mapper extensibility by using handler.UpdateValue(nameof(IToolbarElement.Toolbar)).
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (try-fix-1)
diff --git a/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs b/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
index a1308c79b9..474045cd36 100644
--- a/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
+++ b/src/Controls/src/Core/Handlers/Shell/ShellHandler.Windows.cs
@@ -119,6 +119,7 @@ namespace Microsoft.Maui.Controls.Handlers
public static void MapCurrentItem(ShellHandler handler, Shell view)
{
+ handler.UpdateValue(nameof(IToolbarElement.Toolbar));
handler.PlatformView.SwitchShellItem(view.CurrentItem, true);
}
Regression Details
Shell.TitleView is not centered on Windows due to the mapper execution order change in ShellHandler in PR (#34758)
Root Cause:
ShellHandler executes mapper entries in registration order. After a recent change, MapCurrentItem began running before MapToolbar, causing navigation and layout to occur before the toolbar and TitleView were initialized.
Previously, MapToolbar executed first, ensuring the CommandBar was fully initialized before layout occurred. This allowed TitleViewManager.UpdateTitleViewWidth() to use a valid CommandBar.ActualWidth and correctly center the TitleView.
With the updated order, MapCurrentItem triggers navigation and layout before MapToolbar runs. As a result, CommandBar.ActualWidth is not yet valid during the initial width calculation, causing the TitleView width to be computed incorrectly and appear left-aligned instead of centered.
Description of Change:
Restore the execution order so that MapToolbar runs before MapCurrentItem. This ensures the MauiToolbar and TitleView are fully initialized before navigation triggers layout, allowing TitleViewManager.UpdateTitleViewWidth() to calculate the correct width and center the Shell.TitleView as expected.
Issues Fixed:
Fixes #36322
Tested the behaviour in the following platforms
Output Screenshot