Fixed SafeAreaEdges cannot be set for Shell and the flyout menu collides with display notch and status bar in landscape mode#32295
Conversation
|
|
||
| [Test] | ||
| [Category(UITestCategories.SafeAreaEdges)] | ||
| public void ShellFlyoutShouldRespectSafeArea() |
There was a problem hiding this comment.
This test is failing on iOS:
at UITest.Appium.AppiumQuery.GetQueryBy(String token, String value) in /_/src/TestUtils/src/UITest.Appium/AppiumQuery.cs:line 198
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2557
at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 745
at UITest.Appium.HelperExtensions.WaitForFlyoutIcon(IApp app, String automationId, Boolean isShell) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2263
at UITest.Appium.HelperExtensions.TapFlyoutIcon(IApp app, String title, Boolean isShell, Boolean waitForFlyoutIcon) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2336
at UITest.Appium.HelperExtensions.TapFlyoutPageIcon(IApp app, String title) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2378
at Microsoft.Maui.TestCases.Tests.Issues.Issue32275.ShellFlyoutShouldRespectSafeArea() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32275.cs:line 22
at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
There was a problem hiding this comment.
is this screen shot correct? should there be this much space between the navigation bar and the flyout items?
There was a problem hiding this comment.
Yes @PureWeen , If the flyout items contains images or icons, they would render fine.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issue #32275 where the Shell flyout menu collides with the display notch and status bar in landscape mode on Android. The fix adds proper left inset handling to account for notches/cutouts that appear on the left side in landscape orientation.
Key Changes
- Added
leftInsetcalculation from system bars and display cutouts - Refactored padding logic to handle both AppBarLayout presence and footer scenarios
- Applied left inset padding to root view and background image
Reviewed Changes
Copilot reviewed 3 out of 7 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Issue32275.cs (test) | UI test that verifies flyout respects safe area in landscape mode |
| Issue32275.cs (host) | Test host app page demonstrating the flyout collision issue |
| ShellFlyoutTemplatedContentRenderer.cs | Core fix applying left inset padding to prevent notch collision |
| ShouldFlyoutTextWrapsInLandscape.png | Baseline screenshot for visual regression testing |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
626bd47 to
1e362b2
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/backport to release/10.0.1xx-sr1 |
|
Started backporting to |
|
@PureWeen backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Patch format detection failed.
Error: The process '/usr/bin/git' failed with exit code 128 |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Downloaded the nuget package, I have tested it and it is working fine, any idea when it get merged and released? |
|
/rebase |
1 similar comment
|
/rebase |
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & Validation📝 Review Session — Added template test scenario ·
|
| File:Line | Reviewer | Comment | Status |
|---|---|---|---|
| ShellFlyoutTemplatedContentRenderer.cs:138 | jsuarezruiz | Potential double topInset: v gets topInset AND AppBarLayout also gets INVESTIGATE | topInset |
| Issue32275.cs:18 | jsuarezruiz | Test failing on iOS with TapFlyoutPageIcon error | Author says fixed |
| ShellFlyoutTemplatedContentRenderer.cs:146 | Copilot | appbarLayout accessed without null BUG |
check |
| ShellFlyoutShouldRespectSafeArea.png | PureWeen | Too much space between navigation bar and flyout items? | Explained by author |
| (review) | PureWeen | Found crash in sandbox testing - CHANGES_ BLOCKING | REQUESTED |
Edge Cases to Check
- What happens when
appbarLayoutis null? - Does the double topInset cause excessive spacing?
- Does crash reproduce on current state of PR?
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #32295 | Add leftInset to flyout padding + restructure AppBarLayout inset PENDING (Gate) | ShellFlyoutTemplatedContentRenderer.cs (+33) |
Original PR - has active crash report from PureWeen | handling |
🚦 Gate — Test Verification
📝 Review Session — Added template test scenario · 5cfe7af
Result PASSED (with caveat):
Platform: android
Mode: Full Verification
- Tests FAIL without fix (ShellFlyoutShouldRespectSafeArea detected the bug)
- Tests PASS with fix (ShellFlyoutShouldRespectSafeArea passes with the fix)
Caveat
ShellFlyoutContentTemplateShouldRespectSafeArea (Issue32275_Template) fails due to a missing baseline snapshot for android platform. The PR includes snapshots for android-notch-36 and ios but NOT for the standard android snapshot directory for the Template test. This is a gap in the PR that should be addressed.
Test Details
| Test | Without Fix | With Fix |
|---|---|---|
| PASS | ||
| ` FAIL (missing FAIL (missing baseline snapshot) | snapshot) | ShellFlyoutContentTemplateShouldRespectSafeArea` |
The primary fix test validates correctly. The template test issue is a PR deficiency.
🔧 Fix — Analysis & Comparison
📝 Review Session — Added template test scenario · 5cfe7af
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #32295 | Add leftInset from system bars + display cutout to flyout root view padding and background image PASS (Gate) | ShellFlyoutTemplatedContentRenderer.cs (+33/-14) |
Original PR - PureWeen has CHANGES_REQUESTED due to crash |
Exhausted: No - Phase 3 (try-fix) skipped due to API rate limiting (429 errors after 2 attempts)
Selected Fix: PR's fix - Only validated option; Gate confirmed the core test passes
Skipped Try-Fix Details
Phase 3 was skipped because the task agent returned 429 rate limit errors on all 2 attempts.
Per autonomous execution rules: "If try-fix cannot run due to environment issues after one retry, skip the remaining try-fix models and proceed to Report."
Potential alternative approaches (not tested):
- Null-safe appbarLayout access:
(appbarLayout?.MeasuredHeight ?? 0) > 0- fixes the NullRef crash risk - Apply leftInset to FlyoutView instead of root view for minimal footprint
- Simplify duplicate if/else blocks into a single unified logic block
📋 Report — Final Recommendation
📝 Review Session — Added template test scenario · 5cfe7af
Final Recommendation: REQUEST CHANGES##
Summary
PR #32295 fixes a real regression where the Shell flyout menu collides with the display notch/status bar in landscape mode on Android. The core fix correctly adds leftInset handling to OnApplyWindowInsets. The primary test (ShellFlyoutShouldRespectSafeArea) passes with the fix. However, there are several code quality issues and an active crash report from PureWeen that must be addressed before this can be merged.
Root Cause
In ShellFlyoutTemplatedContentRenderer.OnApplyWindowInsets, the method calculated topInset and bottomInset from system bars and display cutouts but did not calculate leftInset. In landscape mode on devices with notches (which appear on the left or right), the flyout's padding didn't account for the cutout, causing the flyout to render over the notch area.
Fix Quality Assessment
What Works
- Core fix approach is correct: adding
leftInset = Math.Max(systemBars?.Left ?? 0, displayCutout?.Left ?? 0)and applying it to the root view and background image - Primary test
ShellFlyoutShouldRespectSafeAreapasses on Android with the fix - Test structure is appropriate (uses
TestShell,PlatformAffected.Android) - Background image also gets leftInset which prevents the bgImage from overlapping the notch
Issues Requiring Changes####
1. CRASH: PureWeen Requested Changes (BLOCKING)
PureWeen found a crash during sandbox testing and has an active CHANGES_REQUESTED review. A crash reproduction document was uploaded to the PR. This must be investigated and fixed.
2. NullReferenceException Risk (Copilot flagged this)
// CURRENT CODE (can throw if appbarLayout is null):
if (appbarLayout.MeasuredHeight > 0)
// FIX: Use null-safe access:
if ((appbarLayout?.MeasuredHeight ?? 0) > 0)FindDescendantView<AppBarLayout> may return null, causing an unhandled exception.
3. Missing Baseline Snapshot for Template Test
ShellFlyoutContentTemplateShouldRespectSafeArea fails locally because TestCases.Android.Tests/snapshots/android/ShellFlyoutContentTemplateShouldRespectSafeArea.png is missing. The PR only includes snapshots for android-notch-36 and ios but not the standard android snapshot for the template test.
4. Redundant Code
int flyoutViewBottomInset = 0;
if (FooterView is not null)
{
flyoutViewBottomInset = REDUNDANT: already 0 from initialization0; // 5. Non-Standard Preprocessor Symbols
#if TEST_FAILS_ON_CATALYST && TEST_FAILS_ON_WINDOWSCopilot flagged this as non-standard. Per UI testing guidelines, consider using standard symbols like #if !WINDOWS && !MACCATALYST or adding a comment explaining these custom symbols.
6. Code Duplication
The PR has identical if (appbarLayout.MeasuredHeight > 0) ... else ... block repeated twice (once for FooterView is not null, once for the else). This could be extracted into a helper or combined.
Test Coverage
ShellFlyoutShouldRespectSafeArea- Tests basic flyout safe area (Android, primary fix test)ShellFlyoutShouldRespectSafeArea- Also added for iOS
ShellFlyoutContentTemplateShouldRespectSafeArea- Tests custom template flyout (MISSING android baseline snapshot)-
ShouldFlyoutTextWrapsInLandscape- Snapshot modified (potentially affected by the change)-
VerifyShellFlyoutBackgroundImage- Snapshot modified (background image padding changed)-
Specific Changes Requested
- Fix the crash identified by PureWeen in sandbox testing - investigate the crash reproduction document
- Fix null reference on
appbarLayout.MeasuredHeight: change to(appbarLayout?.MeasuredHeight ?? 0) > 0 - Add missing snapshot: Add
ShellFlyoutContentTemplateShouldRespectSafeArea.pngtoTestCases.Android.Tests/snapshots/android/ - Remove redundant assignment: Remove the
flyoutViewBottomInset = 0;line in theif (FooterView is not null)block - Address jsuarezruiz's double topInset concern: Verify there is no 2x topInset with the new logic where both
vandappbarLayoutget padding
Platform Coverage
- Fix is Android-only (
ShellFlyoutTemplatedContentRenderer.csis Android-specific) - Tests correctly scoped to Android (
PlatformAffected.Android,TEST_FAILS_ON_CATALYST && TEST_FAILS_ON_WINDOWS)
📋 Expand PR Finalization Review
Title: ✅ Good
Current: Fixed SafeAreaEdges cannot be set for Shell and the flyout menu collides with display notch and status bar in landscape mode
Description: ✅ Good
- Starts with past tense "Fixed" — titles should describe what changed, not that it was fixed
- Missing
[Android]platform prefix — this fix is Android-only - Too long and reads like a copy of the issue title
- Does not convey the technical change (left/cutout inset addition)
✨ Suggested PR Description
[!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!
Root Cause
In .NET 10 RC2, the OnApplyWindowInsets override in ShellFlyoutTemplatedContentRenderer stopped applying the left inset to the flyout coordinator layout and background image. In landscape mode on devices with a display notch or camera cutout, the left inset is non-zero, causing the flyout menu content to render behind the notch/status bar. This was a regression from .NET 9 which correctly handled the left inset.
Description of Change
ShellFlyoutTemplatedContentRenderer.cs (Android):
-
Added
IHandleWindowInsetsguard: IfFlyoutViewimplementsIHandleWindowInsets, the method delegates tobase.OnApplyWindowInsets()and returns early. This allows flyout views that setSafeAreaEdgeson their own (viaFlyoutContentTemplate) to manage their own safe area insets without the framework double-applying them. -
Added
leftInsetcalculation: The left inset is now computed from the maximum ofsystemBars.LeftanddisplayCutout.Left, mirroring the existingtopInsetandbottomInsetlogic. -
Applied
leftInsetto CoordinatorLayout and background image: The root coordinator layout (v) and the background image (bgImage) now receiveleftInsetas their left padding, preventing content from rendering behind a notch in landscape mode. -
Restructured AppBarLayout-based inset distribution: The
appbarLayout.MeasuredHeightcheck (which determines whether AppBarLayout or the root view should absorb the top inset) was moved inside eachFooterViewbranch, ensuring consistent behaviour regardless of whether a footer is present.
New test cases:
Issue32275.cs/Issue32275_Template.cs(TestCases.HostApp): Two test Shell pages — one using default flyout items, one using a customFlyoutContentTemplatewithSafeAreaEdges = SafeAreaEdges.All.Issue32275.cs/Issue32275_Template.cs(TestCases.Shared.Tests): Screenshot-based UI tests in landscape mode validating that the flyout respects safe area insets in both scenarios.
Issues Fixed
Fixes #32275
Platforms Tested
- Android (primary — regression was Android-only)
- iOS (test runs on iOS via shared test project, but see note below)
- Windows (not applicable — test excluded via
TEST_FAILS_ON_WINDOWS) - Mac Catalyst (not applicable — test excluded via
TEST_FAILS_ON_CATALYST)
Note: An open review thread reports the iOS test (
ShellFlyoutShouldRespectSafeArea) is failing on iOS with aTapFlyoutPageIconerror. This should be resolved before merge.
Output
| Before | After |
|---|---|
![]() |
![]() |
Code Review: ✅ Passed
Code Review — PR #32295
🔴 Blocking Issue
iOS Test Failure (Unresolved Review Comment)
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32275.cs
Review thread: PRRT_kwDOD6PVWM5gUShC — IsResolved: false
The test ShellFlyoutShouldRespectSafeArea is failing on iOS with a TapFlyoutPageIcon / WaitForFlyoutIcon timeout. The test is wrapped in:
#if TEST_FAILS_ON_CATALYST && TEST_FAILS_ON_WINDOWSThis means it still runs on iOS (the condition excludes only Catalyst and Windows). Since the issue is Android-specific (the root cause is an Android landscape inset regression), the test should either:
Option A: Change the guard to also exclude iOS:
#if TEST_FAILS_ON_CATALYST && TEST_FAILS_ON_WINDOWS && TEST_FAILS_ON_IOSOption B: Fix the iOS test so it passes (add a new iOS snapshot).
The same unresolved issue applies to Issue32275_Template.cs (iOS is not excluded and likely fails the same way).
🟡 Medium Issues
1. Thread.Sleep Anti-Pattern in Template Test
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32275_Template.cs (line 24)
Thread.Sleep(1000); // Allow time for orientation change to take effect and UI to stabilize.Per the UI Testing Guidelines, Thread.Sleep should be replaced with VerifyScreenshot(retryTimeout: ...). The built-in retry mechanism handles animation/orientation settling more robustly:
// Replace Thread.Sleep(1000) + VerifyScreenshot() with:
VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2));🔵 Minor Issues
2. Redundant flyoutViewBottomInset = 0 Assignment
File: src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutTemplatedContentRenderer.cs
int flyoutViewBottomInset = 0; // initialized to 0
if (FooterView is not null)
{
flyoutViewBottomInset = 0; // redundant — already 0
...
}The assignment inside the if (FooterView is not null) block is redundant. It should be removed:
if (FooterView is not null)
{
// flyoutViewBottomInset is already 0; no assignment needed
...
}This was flagged by the Copilot reviewer (PRRC_kwDOD6PVWM6U6yiN) and the thread was marked as resolved/outdated, but the redundant assignment is still present in the current diff.
3. Non-Null-Safe appbarLayout.MeasuredHeight Access
File: src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutTemplatedContentRenderer.cs
var appbarLayout = v.FindDescendantView<AppBarLayout>((v) => true);
...
if (appbarLayout.MeasuredHeight > 0) // NullReferenceException if no AppBarLayout foundFindDescendantView may return null if no AppBarLayout exists in the view hierarchy. The access should use null-conditional:
if ((appbarLayout?.MeasuredHeight ?? 0) > 0)This was flagged by the Copilot reviewer (PRRC_kwDOD6PVWM6U6yh3) and marked resolved/outdated — but the non-null-safe access still appears in both the FooterView is not null and else branches in the current diff. In practice this may not crash often if AppBarLayout is almost always present, but it is a latent null dereference.
✅ Looks Good
IHandleWindowInsetsguard: Good design — allowsFlyoutContentTemplateviews that self-manage safe area insets to bypass the framework's inset application. Prevents double-padding.leftInsetcalculation pattern: Correctly mirrors the existingtopInset/bottomInsetpattern usingMath.Max(systemBars.Left, displayCutout.Left).FlyoutViewnull guard: The outerif (v is CoordinatorLayout && FlyoutView is not null)check ensures theFlyoutView.SetPadding(...)call (no null-conditional needed) is safe.- Snapshot updates:
ShouldFlyoutTextWrapsInLandscape.pngandVerifyShellFlyoutBackgroundImage.pngare updated, showing the author verified that existing tests still pass with the new left padding applied. - Two test scenarios: Both the default flyout and the custom template scenario are tested, covering the two code paths gated by
IHandleWindowInsets.
kubaflo
left a comment
There was a problem hiding this comment.
Hi! Could you please review the Ai Summary comment?


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
This pull request addresses an issue where the Shell flyout menu on Android devices could collide with the display notch or status bar, especially in landscape mode. The changes ensure the flyout menu and its background image respect the device's safe area insets, particularly the left inset for devices with a notch. Additionally, a new test case is added to verify this behavior.
Shell Flyout Safe Area Improvements:
ShellFlyoutTemplatedContentRenderer.csto include the left inset when setting padding for the flyout view and its background image, preventing overlap with display notches or status bars. [1] [2]Testing Enhancements:
Issue32275) to the test app to reproduce and verify the safe area issue with the Shell flyout in landscape mode on Android. [1] [2]TestCases.Shared.Teststo automatically validate that the flyout menu does not collide with the notch or status bar.Issues Fixed
Fixes #32275
Output
Screen.Recording.2025-11-26.at.2.34.49.PM.mov