[Material 3] Fix Material 2 color flash in AppBar when switching tabs for the first time#35117
Conversation
…tnet#34994) > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Description Integrates the standalone `code-review` skill into the `pr-review` orchestrator so that deep code analysis happens during Pre-Flight and its findings feed into Try-Fix exploration. ### Changes **Pre-Flight (Phase 1)** — `.github/pr-review/pr-preflight.md` - Added Part B (Step 7): Invokes code-review skill as an independent sub-agent - Independence-first principle preserved — sub-agent receives only the PR number, not Part A context - Outputs both `content.md` (context + code review summary) and `code-review.md` (full review) **Try-Fix (Phase 2)** — `.github/skills/pr-review/SKILL.md` - Try-fix prompt template now includes `code_review_findings` (errors, warnings, failure modes, blast radius) - Findings are advisory hints — inform approach, not a mandatory checklist - Only root-cause-relevant findings included to avoid distraction **Report (Phase 3)** — `.github/pr-review/pr-report.md` - Code Review row added to Phase Status table - Hard gate: `NEEDS_CHANGES` verdict blocks `APPROVE` regardless of Gate/Try-Fix - New "Code Review Impact on Try-Fix" section **AI Summary** — `.github/scripts/post-ai-summary-comment.ps1` - Full code-review output shown as separate "🔬 Code Review — Deep Analysis" collapsible section - Gracefully skipped if `code-review.md` does not exist ### Design decisions - The `code-review` skill itself is NOT modified — stays standalone and independently invokable - The `try-fix` skill is NOT modified — already accepts hints - Code review runs as a sub-agent to prevent anchoring bias (independence-first) - Report hard gate prevents approving PRs with unresolved code review errors --------- Co-authored-by: Copilot <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 -- 35117Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35117" |
There was a problem hiding this comment.
Pull request overview
Fixes the initial AppBar “Material 2 color flash” on Android Shell (handler path) by making the default toolbar colors respect Material 3 when enabled, instead of using M2-only static defaults.
Changes:
- Updated Android
Shelltoolbar default background/foreground/title colors to switch based onRuntimeFeature.IsMaterial3Enabled. - Converted the previous
static readonlycolor fields into computed properties so the values aren’t fixed at type initialization.
🤖 AI Summary
📊 Review Session —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35117 | Convert static readonly fields to computed properties with M3 branching via RuntimeFeature.IsMaterial3Enabled |
⏳ PENDING (Gate skipped — no tests) | Shell.cs |
Original PR |
🔬 Code Review — Deep Analysis
Code Review — PR #35117
Independent Assessment
What this changes: Three Android-only static properties in Shell.cs that define default toolbar colors are updated to be M3-aware. DefaultForegroundColor and DefaultTitleColor change from static readonly fields (evaluated once at type init, permanently Colors.White) to computed properties that call ResolveThemeColor with M3-branching logic. DefaultBackgroundColor was already a computed property, but it now also branches on RuntimeFeature.IsMaterial3Enabled.
Inferred motivation: Without this fix, when no explicit ShellAppearance is set and the user switches tabs, UpdateToolbarAppearanceFeatures stamps the hard-coded M2 defaults (Colors.White, #2c3e50) onto the Toolbar, causing a visible M2-color flash before any M3 override can apply.
Reconciliation with PR Narrative
Author claims: The fix brings Shell.cs in line with ShellRenderer.cs (the legacy compatibility renderer path), which already checks RuntimeFeature.IsMaterial3Enabled. Root cause is correctly identified.
Agreement: The root cause analysis is accurate. The static readonly field evaluation is exactly why the flash happened — once is at startup (M2), and the M3 check never runs again.
Disagreement — one value does not match ShellRenderer.cs: The PR description says "to match ShellRenderer.cs," but the M2 (non-M3) light-theme value for DefaultForegroundColor differs:
| File | Non-M3 light DefaultForegroundColor |
|---|---|
ShellRenderer.cs line 96 |
Colors.Black |
Shell.cs (this PR) |
Colors.White |
DefaultTitleColor and DefaultBackgroundColor match exactly. Only DefaultForegroundColor diverges. The PR preserves the original Shell.cs value (Colors.White), which is what existed before the PR. It does not introduce the discrepancy — but it also doesn't fix it.
Findings
⚠️ Warning — DefaultForegroundColor non-M3 light mismatch vs ShellRenderer.cs
Shell.cs:828 uses Colors.White for the non-M3 light theme foreground; ShellRenderer.cs:96 uses Colors.Black. The PR description explicitly claims parity with ShellRenderer.cs, but this one value is inconsistent. At least one of the two files is visually wrong: the non-M3 light DefaultBackgroundColor is #2c3e50 (dark teal/blue), so white icons on that background are correct and Colors.Black in ShellRenderer.cs is likely the pre-existing bug. The PR's Colors.White is the more defensible value — but this should be confirmed by the author rather than assumed, since the PR description implies intentional mirroring of ShellRenderer.cs.
💡 Suggestion — Color.FromArgb allocates on every property access
Color is a reference type (sealed class) in .NET MAUI. The six Color.FromArgb(...) calls now execute on every UpdateToolbarAppearanceFeatures invocation (every tab switch, every appearance change). Before this PR, DefaultForegroundColor and DefaultTitleColor were static readonly fields — allocated once. Consider caching the four M3 colors as static readonly and referencing them inside the expressions, or using Colors constants where they exist. This is a minor allocation concern, not a hot-path issue.
💡 Suggestion — No automated test for the regression
This is a visible rendering bug (M2 flash on tab switch). The fix is straightforward, but there's no test to guard against regression. A UI/screenshot test that verifies toolbar BarBackground/BarTextColor immediately after first tab switch in M3 mode would prevent the bug from silently returning.
Devil's Advocate
-
Could the
Colors.WhitevsColors.Blackdiscrepancy actually matter? In the new handler path (Shell.cs),Toolbar.IconColoris set fromDefaultForegroundColor. If someone builds an app with M2 (non-M3) on Android in light theme using the new handler, their toolbar icons get white on a dark background — correct. But if they use the legacy renderer (ShellRenderer.cs), they get black on dark — likely a pre-existing bug in that path. This PR doesn't make things worse; the new handler path was always white. -
Is the computed property change safe? Converting
static readonlyfields to computed properties means the colors are now dynamically evaluated. This is actually an improvement:AppThemechanges at runtime (light/dark switching) will now be reflected on next tab switch. Not a regression. -
CI is fully green — all required checks pass, including integration and unit tests across all platforms.
Verdict: NEEDS_DISCUSSION
Confidence: medium
Summary: The fix correctly identifies and resolves the root cause of the M3 color flash. CI is green, the logic is sound, and the M3 color values match ShellRenderer.cs exactly. The discussion point is the DefaultForegroundColor non-M3 light value: the PR preserves Colors.White (original Shell.cs) rather than adopting Colors.Black (ShellRenderer.cs). The author should clarify whether this is intentional or an oversight — if Colors.Black in ShellRenderer.cs is itself a pre-existing bug (which is likely given the dark background), then Colors.White here is correct and this PR is LGTM. That confirmation would close the loop.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Pre-computed static color table — allocate all 12 M2/M3 × light/dark color variants as static readonly fields once at type init |
1 file | Eliminates per-call Color.FromArgb allocations | |
| 2 | try-fix (claude-sonnet-4.6) | Delegate to ShellRenderer.DefaultBackgroundColor/ForegroundColor/TitleColor directly — removes duplication |
1 file | Clean architecture; single source of truth | |
| 3 | try-fix (gpt-5.3-codex) | Guard null/fallback stamping in UpdateToolbarAppearanceFeatures — only apply defaults if Toolbar properties are null |
❌ FAIL (build error, wrong TFM) | 1 file | Different root cause hypothesis |
| 4 | try-fix (gpt-5.4) | Skip default color stamping entirely in M3 mode (no appearance set) — let platform handle M3 defaults | 1 file | Alternative behavioral approach | |
| PR | PR #35117 | Convert static readonly fields to computed properties with inline RuntimeFeature.IsMaterial3Enabled checks |
⏳ PENDING (Gate skipped — no tests) | 1 file | Original PR; matches ShellRenderer.cs pattern |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | Solution space well-covered; PR fix is correct approach; Attempt 2 (delegate to ShellRenderer) noted as cleanest alternative |
Exhausted: Yes
Selected Fix: PR's fix — correct root cause identification, matches existing ShellRenderer.cs pattern, simple 1-file change. Attempt 2 (delegate to ShellRenderer) is architecturally cleaner but introduces a cross-type dependency on the compatibility renderer. PR fix is more self-contained and pragmatic for a bug fix targeting the handler path.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #35116, 1 file changed |
| Code Review | NEEDS_DISCUSSION (medium) | 0 errors, 1 warning, 2 suggestions |
| Gate | No tests detected in PR | |
| Try-Fix | ✅ COMPLETE | 4 attempts, 0 passing (all BLOCKED — no device; 1 FAIL — build config error) |
| Report | ✅ COMPLETE |
Code Review Impact on Try-Fix
The code review DefaultForegroundColor M2 light mismatch (Colors.White in Shell.cs vs Colors.Black in ShellRenderer.cs) guided Attempt 4 (gpt-5.4) to try a different approach entirely. The suggestion about Color.FromArgb allocation on every call led Attempt 1 to propose a pre-computed color table. No model found a verifiable passing fix due to missing device/emulator.
Summary
The PR fixes a real, verified bug (M2 color flash on first tab switch in M3 mode) by making three #if ANDROID default color properties in Shell.cs M3-aware. The root cause analysis is correct and the approach mirrors the pattern already used in ShellRenderer.cs. All CI checks pass. The recommendation is REQUEST CHANGES due to one discussion item: the code review flagged that the PR's DefaultForegroundColor M2 light value (Colors.White) differs from ShellRenderer.cs (Colors.Black). This was already raised in a (now-resolved) review thread, and the author's explanation (Colors.White has better contrast on the dark #2c3e50 M2 background) is compelling. A brief confirmation from the author in the PR description or comments would close this cleanly.
No automated tests were added. The Gate was skipped because no tests were detected. Adding a regression test (even a unit test for DefaultBackgroundColor/DefaultForegroundColor/DefaultTitleColor values when M3 is enabled/disabled) is recommended.
Root Cause
DefaultForegroundColor and DefaultTitleColor in Shell.cs (the new Android handler path) were static readonly fields initialized once at type load, always returning M2 colors (Colors.White). DefaultBackgroundColor was a computed property but also always returned M2 colors. On every tab switch, UpdateToolbarAppearanceFeatures stamped these M2 defaults onto Toolbar.BarTextColor/BarBackground/IconColor when no explicit ShellAppearance was provided — causing a brief M2 color flash before M3 colors could override them.
Fix Quality
The PR fix is minimal (9 additions, 3 deletions in 1 file), directly addresses the root cause, and matches the pattern in ShellRenderer.cs. The conversion of static readonly fields to computed properties is the right approach. The only unresolved concern is the DefaultForegroundColor M2 light value discrepancy with ShellRenderer.cs. An architecturally cleaner alternative (Attempt 2: delegate to ShellRenderer.Default* properties) was identified but not verifiable without a device.
Selected Fix: PR's fix — pragmatic, self-contained, correct for the handler path.
Suggested action items before merge:
- Confirm
Colors.White(notColors.Black) is the correct M2 lightDefaultForegroundColorfor the handler path (contrast reasoning against#2c3e50background supports White; note discrepancy with ShellRenderer.cs) - Consider adding a unit test or at minimum a code comment documenting why
Colors.Whiteis used instead ofColors.Blackfor M2 light foreground
… for the first time (#35117) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details: When switching to a different tab for the first time, the AppBar briefly displays Material 2 (M2) colors before updating to the expected Material 3 (M3) colors. ### Root Cause: In Shell.cs (the new handler path), DefaultForegroundColor and DefaultTitleColor were static readonly fields fixed to Colors.White (M2-only), evaluated once at type initialization. DefaultBackgroundColor was computed but returned M2 colors regardless of M3 mode. All three were stamped onto Toolbar.BarBackground/BarTextColor/IconColor on every tab-switch when no explicit ShellAppearance was provided. ### Description of change: Updated the Shell.cs Android defaults to match ShellRenderer.cs — now they check RuntimeFeature.IsMaterial3Enabled and use M3 color values (#FEF7FF, #1D1B20) when M3 is enabled, falling back to the original M2 colors otherwise. ### Validated the behaviour in the following platforms - [x] Android - [ ] Windows - [ ] iOS - [ ] Mac ### Fixes Fixes #35116 ### Screenshots | Before | After | |---------|--------| | <video src="https://github.com/user-attachments/assets/2f17f2f1-21ff-4c94-9dbb-4fb2fe113a75"> | <video src="https://github.com/user-attachments/assets/8f3f6124-7b1b-467a-8d0e-a7eec0a25a33"> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… for the first time (#35117) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details: When switching to a different tab for the first time, the AppBar briefly displays Material 2 (M2) colors before updating to the expected Material 3 (M3) colors. ### Root Cause: In Shell.cs (the new handler path), DefaultForegroundColor and DefaultTitleColor were static readonly fields fixed to Colors.White (M2-only), evaluated once at type initialization. DefaultBackgroundColor was computed but returned M2 colors regardless of M3 mode. All three were stamped onto Toolbar.BarBackground/BarTextColor/IconColor on every tab-switch when no explicit ShellAppearance was provided. ### Description of change: Updated the Shell.cs Android defaults to match ShellRenderer.cs — now they check RuntimeFeature.IsMaterial3Enabled and use M3 color values (#FEF7FF, #1D1B20) when M3 is enabled, falling back to the original M2 colors otherwise. ### Validated the behaviour in the following platforms - [x] Android - [ ] Windows - [ ] iOS - [ ] Mac ### Fixes Fixes #35116 ### Screenshots | Before | After | |---------|--------| | <video src="https://github.com/user-attachments/assets/2f17f2f1-21ff-4c94-9dbb-4fb2fe113a75"> | <video src="https://github.com/user-attachments/assets/8f3f6124-7b1b-467a-8d0e-a7eec0a25a33"> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… for the first time (#35117) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details: When switching to a different tab for the first time, the AppBar briefly displays Material 2 (M2) colors before updating to the expected Material 3 (M3) colors. ### Root Cause: In Shell.cs (the new handler path), DefaultForegroundColor and DefaultTitleColor were static readonly fields fixed to Colors.White (M2-only), evaluated once at type initialization. DefaultBackgroundColor was computed but returned M2 colors regardless of M3 mode. All three were stamped onto Toolbar.BarBackground/BarTextColor/IconColor on every tab-switch when no explicit ShellAppearance was provided. ### Description of change: Updated the Shell.cs Android defaults to match ShellRenderer.cs — now they check RuntimeFeature.IsMaterial3Enabled and use M3 color values (#FEF7FF, #1D1B20) when M3 is enabled, falling back to the original M2 colors otherwise. ### Validated the behaviour in the following platforms - [x] Android - [ ] Windows - [ ] iOS - [ ] Mac ### Fixes Fixes #35116 ### Screenshots | Before | After | |---------|--------| | <video src="https://github.com/user-attachments/assets/2f17f2f1-21ff-4c94-9dbb-4fb2fe113a75"> | <video src="https://github.com/user-attachments/assets/8f3f6124-7b1b-467a-8d0e-a7eec0a25a33"> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… for the first time (#35117) <!-- Please let the below note in for people that find this PR --> > [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details: When switching to a different tab for the first time, the AppBar briefly displays Material 2 (M2) colors before updating to the expected Material 3 (M3) colors. ### Root Cause: In Shell.cs (the new handler path), DefaultForegroundColor and DefaultTitleColor were static readonly fields fixed to Colors.White (M2-only), evaluated once at type initialization. DefaultBackgroundColor was computed but returned M2 colors regardless of M3 mode. All three were stamped onto Toolbar.BarBackground/BarTextColor/IconColor on every tab-switch when no explicit ShellAppearance was provided. ### Description of change: Updated the Shell.cs Android defaults to match ShellRenderer.cs — now they check RuntimeFeature.IsMaterial3Enabled and use M3 color values (#FEF7FF, #1D1B20) when M3 is enabled, falling back to the original M2 colors otherwise. ### Validated the behaviour in the following platforms - [x] Android - [ ] Windows - [ ] iOS - [ ] Mac ### Fixes Fixes #35116 ### Screenshots | Before | After | |---------|--------| | <video src="https://github.com/user-attachments/assets/2f17f2f1-21ff-4c94-9dbb-4fb2fe113a75"> | <video src="https://github.com/user-attachments/assets/8f3f6124-7b1b-467a-8d0e-a7eec0a25a33"> | --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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!
Issue Details:
When switching to a different tab for the first time, the AppBar briefly displays Material 2 (M2) colors before updating to the expected Material 3 (M3) colors.
Root Cause:
In Shell.cs (the new handler path), DefaultForegroundColor and DefaultTitleColor were static readonly fields fixed to Colors.White (M2-only), evaluated once at type initialization. DefaultBackgroundColor was computed but returned M2 colors regardless of M3 mode. All three were stamped onto Toolbar.BarBackground/BarTextColor/IconColor on every tab-switch when no explicit ShellAppearance was provided.
Description of change:
Updated the Shell.cs Android defaults to match ShellRenderer.cs — now they check RuntimeFeature.IsMaterial3Enabled and use M3 color values (#FEF7FF, #1D1B20) when M3 is enabled, falling back to the original M2 colors otherwise.
Validated the behaviour in the following platforms
Fixes
Fixes #35116
Screenshots
35116_BeforeFix.mov
35116_AfterFix.mov