-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Android] Fixed Label Overlapped by Android Status Bar When Using SafeAreaEdges="Container" in .NET MAUI #33014
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an Android-specific issue where labels and other UI elements could be overlapped by the status bar when using SafeAreaEdges=Container in .NET MAUI Shell navigation. The fix improves safe area inset calculation by detecting full-screen views and applying consistent insets during navigation, while also ensuring proper view tracking cleanup.
Key Changes
- Enhanced safe area calculation logic to detect full-screen views and apply reliable insets during Shell navigation
- Added view untracking when padding is reset to zero to prevent stale tracking
- Introduced comprehensive UI test coverage for the navigation scenario
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/Core/src/Platform/Android/SafeAreaExtensions.cs |
Core fix: Added full-screen view detection (95% threshold), improved position-based calculation conditions, and view untracking logic |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32941.cs |
NUnit UI test implementation with navigation verification via screenshot comparison |
src/Controls/tests/TestCases.HostApp/Issues/Issue32941.cs |
Test case UI with two pages demonstrating Shell navigation with SafeAreaEdges=Container |
src/Controls/tests/TestCases.Android.Tests/snapshots/android-notch-36/ShellContentShouldRespectSafeAreaEdges_After_Navigation.png |
Reference screenshot for automated visual verification |
| [Category(UITestCategories.Shell)] | ||
| [Category(UITestCategories.SafeAreaEdges)] |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple [Category] attributes detected. According to the UI testing guidelines, each test should have only ONE [Category] attribute. Please pick the most specific category that applies to this test.
Since this test is primarily about SafeAreaEdges behavior (which is the core issue being tested), consider using only [Category(UITestCategories.SafeAreaEdges)] and removing the Shell category.
| using System.Collections.ObjectModel; | ||
|
|
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unused import System.Collections.ObjectModel should be removed as it's not used anywhere in this file.
| using System.Collections.ObjectModel; |
| top = Math.Min(top - viewTop, top); | ||
| } | ||
| else | ||
| else if ((viewHeight > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews) |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition (viewHeight > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews contains redundant checks. The hasTrackedViews condition appears twice - once in the OR clause and once in the AND clause. This makes the logic unnecessarily complex and potentially confusing.
Consider simplifying to: hasTrackedViews && !isFullScreen since if hasTrackedViews is false, the entire AND expression would be false anyway, making the (viewHeight > 0 || hasTrackedViews) part redundant.
| bottom = Math.Min(viewBottom - bottomEdge, bottom); | ||
| } | ||
| else | ||
| else if ((viewHeight > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews) |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition (viewHeight > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews contains redundant checks. The hasTrackedViews condition appears twice - once in the OR clause and once in the AND clause. This makes the logic unnecessarily complex and potentially confusing.
Consider simplifying to: hasTrackedViews && !isFullScreen since if hasTrackedViews is false, the entire AND expression would be false anyway, making the (viewHeight > 0 || hasTrackedViews) part redundant.
| left = Math.Min(left - viewLeft, left); | ||
| } | ||
| else | ||
| else if ((viewWidth > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews) |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition (viewWidth > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews contains redundant checks. The hasTrackedViews condition appears twice - once in the OR clause and once in the AND clause. This makes the logic unnecessarily complex and potentially confusing.
Consider simplifying to: hasTrackedViews && !isFullScreen since if hasTrackedViews is false, the entire AND expression would be false anyway, making the (viewWidth > 0 || hasTrackedViews) part redundant.
| right = Math.Min(viewRight - rightEdge, right); | ||
| } | ||
| else | ||
| else if ((viewWidth > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews) |
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition (viewWidth > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews contains redundant checks. The hasTrackedViews condition appears twice - once in the OR clause and once in the AND clause. This makes the logic unnecessarily complex and potentially confusing.
Consider simplifying to: hasTrackedViews && !isFullScreen since if hasTrackedViews is false, the entire AND expression would be false anyway, making the (viewWidth > 0 || hasTrackedViews) part redundant.
|
|
||
| // A view is considered full-screen if it occupies most of the available content area | ||
| // Use a percentage-based threshold (95%) instead of fixed pixels to handle all screen sizes | ||
| bool isFullScreen = (viewWidth >= availableWidth * 0.95) && (viewHeight >= availableHeight * 0.95); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 95%?
Review Feedback: PR #33014 - Fixed Label Overlapped by Android Status Bar (BEFORE Code Quality Fixes)RecommendationRequired changes:
Recommended changes:
📋 Full PR Review Details (Original Code)SummaryPR #33014 fixes issue #32941 where labels were overlapped by the Android status bar when using Fix approach: Introduces "full-screen" view detection (≥95% of available screen space) to skip unreliable position-based optimization and always apply full safe area insets to full-screen views. Fix effectiveness: ✅ Successfully resolves the reported issue Code ReviewRoot Cause AnalysisThe problem occurs in // OLD: Position-based optimization attempted immediately
if (top > 0 && viewTop < top && viewTop >= 0)
top = Math.Min(top - viewTop, top); // Reduce inset if view doesn't overlap
else if (viewHeight > 0 || hasTrackedViews)
top = 0; // Remove inset entirelyThe bug: During Shell navigation to a page with
The Fix - AnalysisKey insight: ContentPages are typically full-screen views. For full-screen views, position-based optimization is unnecessary AND unreliable during navigation. The PR adds: // Detect full-screen views
var availableWidth = screenWidth - baseSafeArea.Left - baseSafeArea.Right;
var availableHeight = screenHeight - baseSafeArea.Top - baseSafeArea.Bottom;
bool isFullScreen = (viewWidth >= availableWidth * 0.95) && (viewHeight >= availableHeight * 0.95);
// Skip position optimization for full-screen views
if (top > 0 && hasTrackedViews && !isFullScreen && viewTop < top && viewTop >= 0)
{
top = Math.Min(top - viewTop, top);
}
else if ((viewHeight > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews)
{
top = 0;
}Why this works:
Assessment: ✅ The fix correctly addresses the root cause. The approach is sound and pragmatic. Code Quality Issues❌ Issues Requiring Fixes1. Redundant Boolean Conditions (4 instances)Location: Issue: else if ((viewHeight > 0 || hasTrackedViews) && !isFullScreen && hasTrackedViews)Problem: Why:
Fix: else if (hasTrackedViews && !isFullScreen)Impact: Low severity - code works correctly but is unnecessarily complex and confusing. 2. Magic Number: 0.95Location: Issue: bool isFullScreen = (viewWidth >= availableWidth * 0.95) && (viewHeight >= availableHeight * 0.95);Problem: The threshold value Questions:
Fix: // At class level
private const double FullScreenThreshold = 0.95; // 95%
// Add comment explaining: "Views occupying ≥95% of available screen space are
// considered full-screen (typically ContentPages) and should always receive
// full safe area insets to avoid timing issues during Shell navigation."
// In method
bool isFullScreen = (viewWidth >= availableWidth * FullScreenThreshold) &&
(viewHeight >= availableHeight * FullScreenThreshold);Impact: Medium severity - makes the code less maintainable and the choice less clear. 3. Unused ImportLocation: Issue: using System.Collections.ObjectModel;Problem: This namespace is imported but never used in the file. Fix: Remove the line. Impact: Low severity - code works but violates clean code principles. 4. Multiple
|
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NirmalKumarYuvaraj can you evaluate if this is a regression or an expected visual change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PureWeen , This is a regression. I will check and update
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
- Modified full-screen detection to only apply when hasTrackedViews=true This prevents TabbedPage content from being incorrectly treated as full-screen during initialization, which was breaking ListView ItemTapped events - Simplified position-based calculation logic by removing redundant hasTrackedViews checks This restores original behavior for non-full-screen views - Updated Issue32941 test from screenshot-based to position-based assertions Test now verifies label Y position is >= 20px to ensure it's not overlapped by status bar - Simplified Issue32941 HostApp implementation for better maintainability - Removed unnecessary screenshot snapshot file Fixes the breaking test TestIssue1763ItemTappedFiring while preserving the fix for dotnet#32941
- Replaced percentage-based full-screen detection with deterministic parent hierarchy analysis Views are now identified as top-level by checking if their parent implements ISafeAreaView2 - Parent-based logic: top-level if no parent OR parent doesn't implement ISafeAreaView2 OR (parent implements ISafeAreaView2 AND hasTrackedViews) - More reliable than 95% threshold, avoids edge cases with screen size calculations
…View/ViewCell - Created ISafeAreaIgnoredContainer marker interface in Core layer - ListView, TableView, ViewCell now implement ISafeAreaIgnoredContainer - SafeAreaExtensions checks parent hierarchy for this interface - Views inside these containers completely skip safe area logic - Clean architectural solution - no type name string comparisons needed This ensures ViewCells never apply safe area insets, similar to how ScrollView and CollectionView manage their own layout.
- Removed from all PublicAPI.Unshipped.txt files - Interface is internal so should not be in PublicAPI
- Check if view's parent or ancestors implement ISafeAreaIgnoredContainer - Don't skip safe area for the containers themselves, only their content - Fixes app crash on navigation
Instead of early return, set safe area values to 0 when inside ListView/TableView/ViewCell. This allows view tracking and padding reset to work correctly, fixing layout issues in ListView.
ViewCell should not implement ISafeAreaIgnoredContainer. Only ListView and TableView (the containers) need it. ViewCell is a template, not a container.
…content All pages where ListView or TableView is the direct Content need SafeAreaRegions.Container to work correctly with ISafeAreaIgnoredContainer. Affected pages: - Bugzilla58875 ListViewPage - Bugzilla32206 ContentPage32206 - Bugzilla52533 - Bugzilla43941 ContentPage43941 - Bugzilla33578 - Bugzilla32040 - Issue1028
Simplified the ISafeAreaIgnoredContainer hierarchy checking by removing the redundant direct parent check. The while loop already checks the parent on its first iteration, so the separate if statement was unnecessary. No functional change - same behavior with cleaner code.
Both pages have ListView/TableView as root content and need SafeAreaRegions.Container to work correctly with ISafeAreaIgnoredContainer implementation. - Issue5924: TableView as root content - ShellInsets ListViewPage: ListView as root content
Both tests have ListView as root content and need SafeAreaRegions.Container to work correctly with ISafeAreaIgnoredContainer implementation. - Issue1658: ListView with context actions and gesture recognizer - Issue6258: ListView with context actions icon
Move the ISafeAreaIgnoredContainer logic from SafeAreaExtensions to MauiWindowInsetListener.FindListenerForView(). This prevents inset listeners from being added to ListView/TableView children in the first place, rather than applying listeners and then zeroing out safe area. Benefits: - Cleaner architecture: listeners aren't added where they shouldn't be - More efficient: no unnecessary listener attachment or calculations - Clearer intent: early rejection at listener discovery phase
The ISafeAreaIgnoredContainer check now happens in FindListenerForView, so we no longer need this logic in SafeAreaExtensions. This simplifies the code and makes safe area calculation more straightforward.
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 33014Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 33014" |
- Add ISafeAreaIgnoredContainer interface to mark ListView and TableView - Prevent SafeArea inset listener from being applied to children of ignored containers - Add UI test for Issue33034 to validate TabbedPage with ListView behavior - Children of ListView/TableView should not have SafeArea insets applied
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 labels could be overlapped by the Android status bar when using
SafeAreaEdges=Containerin .NET MAUI Shell navigation. The changes improve how safe area insets are calculated and applied for full-screen views on Android, and introduce new test cases to verify the fix.Safe Area Insets Calculation Improvements (Android):
Test Coverage:
Issue32941) to verify that after navigation, the label is not overlapped by the Android status bar whenSafeAreaEdges=Containeris used.Issues Fixed
Fixes #32941
Output
Before.mov
After.mov