[WIP][Android] Fix stale bottom safe area after Shell tab navigation with TabBarIsVisible=False - #36474
[WIP][Android] Fix stale bottom safe area after Shell tab navigation with TabBarIsVisible=False#36474praveenkumarkarunanithi wants to merge 3 commits into
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36474Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36474" |
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
| // so its padding isn't left stale. The IsViewTracked gate excludes plain, non-safe-area views. See #36269 | ||
| if (_isInsetListenerSet && | ||
| (_didSafeAreaEdgeConfigurationChange || | ||
| (changed && MauiWindowInsetListener.FindListenerForView(this)?.IsViewTracked(this) == true))) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Android Safe Area — Gating resize-triggered inset reapplication on IsViewTracked(this) misses safe-area-eligible views that initially computed zero padding. SafeAreaExtensions.ApplyAdjustedSafeAreaInsetsPx returns without tracking when all computed padding is zero, and only calls TrackView after nonzero padding is applied. Concrete scenario: a safe-area page initially lays out above the system bar with zero bottom padding, then resizes into the bar area when the TabBar is hidden; changed is true but the view was never tracked, so RequestApplyInsets is skipped and padding stays stale. Please gate on safe-area eligibility rather than prior nonzero padding, or track eligible zero-padding views. LayoutViewGroup has the same predicate pattern.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
Tests Failure Analysis
Test Failure Review: Not ready - click to expandOverall verdict: Not ready One failure ( Coverage: 169 checks total · 162 passing/neutral/skipped · 7 failing · 0 pending · 0 inaccessible · 2 unmapped · 9 unexplained build legs · 0 unaccounted failing checks · 1 aborted failing checks · 0 canceled-build checks · 0 device-test unverified · 13 unattributed · 1 regressed-vs-base. Deterministic ceiling: Not ready — 2 failing check(s) have no inspectable AzDO build evidence; 9 failed build leg(s) produced no extractable failure; 13 failure(s) could not be attributed deterministically; 1 failing check(s) did not finish cleanly (macOS UITests Controls Shell cancelled); 1 leg/failure(s) are red on the PR but GREEN on the most recent completed base build.
Recommended actionA human reviewer should inspect the Evidence detailsBuild: maui-pr-uitests 1502162 — failed/completed on refs/pull/36474/merge. Baseline build: maui-pr-uitests 1499834 — failed/completed on refs/heads/main (2026-07-08). Baseline log inspection was partial: only 8 of 34 failed build log entries were read, so the baseline failure list may be incomplete. Device tests: maui-pr-devicetests 1502163 — succeeded. All 7 Helix jobs confirmed clean (0 failed work items each). maui-pr (main build): 1502161 — succeeded. All build, pack, and Helix unit test legs green. PR scope: 2 changed files — Unexplained failed build legs (9): Controls ViewBaseTests/VisualStateManager/Window, Controls Page/Performance/Picker/ProgressBar, Controls Shell, Controls CarouselView, Controls Layout, Controls (API 30) Editor/Effects/Essentials/FlyoutPage/Focus/Fonts/Frame/Gestures/GraphicsView, Controls CollectionView. These produced no extractable failure (likely build breaks or unreadable logs); each leg's log should be opened to confirm they are infra-only. Unmapped failing checks (2): Known-issue matchers loaded: 1. ci-scan matchers loaded (main branch history): 41 — 10 failures matched ci-scan, 8 regressions demoted to Needs human investigation by ci-scan (recurring pattern found but leg is regressed vs base, so dismissal refused). |
kubaflo
left a comment
There was a problem hiding this comment.
Is this ready for review?
Tests Failure Analysis
Test Failure Review: Needs human investigation - click to expandOverall verdict: Needs human investigation — but no PR-caused regressions. Against several recent
Coverage: 169 checks · 162 passing · 7 failing · 0 pending · 2 unmapped · 10 unexplained build legs · 1 aborted · 0 device-test unverified · 14 unattributed · 0 regressed-vs-base. Deterministic ceiling: Needs human investigation — 10 unexplained legs, 14 unattributed, 1 aborted; 0 regressed-vs-base. Builds (this PR): maui-pr-uitests 1502162 (failed) · maui-pr-devicetests 1502163 (succeeded). Base sampling ( Scope: platform/android · area-safearea · partner/syncfusion. WIP draft. Base branch: main. Recommended actionNo genuine base-branch regression was isolated, but some legs could not be attributed — a human should glance at the Uncertain items above before merging. |
This comment has been minimized.
This comment has been minimized.
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the ai's suggestions?
Addressed the AI concerns. |
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the latest suggestions?
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 4 findings
See inline comments for details.
| // HasSafeAreaRegions is evaluated last so it only runs for changed, inset-tracked views. See #36269. | ||
| if (_isInsetListenerSet && | ||
| (_didSafeAreaEdgeConfigurationChange || | ||
| (changed && SafeAreaExtensions.HasSafeAreaRegions(CrossPlatformLayout)))) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[major] Performance-Critical Path / Safe Area — This gate is much broader than "safe-area views": Layout's SafeAreaEdgesDefaultValueCreator() returns SafeAreaEdges.Container (src/Controls/src/Core/Layout/Layout.cs:373), and Layout.GetSafeAreaRegionsForEdge maps Default/Container to SafeAreaRegions.Container whenever IgnoreSafeArea is false. So HasSafeAreaRegions returns true for essentially every Grid/StackLayout/FlexLayout in an app, not just ones that set SafeAreaEdges. _isInsetListenerSet is also set unconditionally in OnAttachedToWindow via TrySetMauiWindowInsetListener (no ShouldSetMauiWindowInsetListener eligibility check), so the guard collapses to "any LayoutViewGroup whose bounds changed". Concrete scenario: a CollectionView/ScrollView page whose inner layouts resize as content loads, or any animation that triggers relayout, now schedules a full RequestApplyInsets traversal on every bounds change; each dispatch re-runs ApplyAdjustedSafeAreaInsetsPx per tracked view (new DisplayMetrics() + GetRealMetrics + GetLocationOnScreen per view) and can call SetPadding, forcing another measure/layout. Consider narrowing to views that can actually receive padding — e.g. ISafeAreaView2.HasExplicitSafeAreaEdges (already used by MauiWindowInsetListener.HasExplicitSafeAreaEdges) or the view already being tracked by the listener — so ordinary resizes of default layouts don't pay for an inset dispatch.
| // HasSafeAreaRegions is evaluated last so it only runs for changed, inset-tracked views. See #36269. | ||
| if (_isInsetListenerSet && | ||
| (_didSafeAreaEdgeConfigurationChange || | ||
| (changed && SafeAreaExtensions.HasSafeAreaRegions(CrossPlatformLayout)))) |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Layout Measure-Arrange — Requesting insets from inside OnLayout whenever changed is true creates a layout→padding→layout feedback path: the inset callback calls view.SetPadding(...), and OnMeasure here adds PaddingTop/Bottom to the measured size, so for a content-sized safe-area view (wrap-content, Grid Auto row, or VerticalOptions=Center) the new padding changes the view's own bounds, which produces another changed == true layout that re-requests insets. The overlap math clamps via Math.Min(..., bottom) so it should converge, but for a vertically centered safe-area layout the growth moves both viewTop and viewBottom further into the inset regions and re-enters the loop at least once more per resize. Please verify with a centered / Auto-sized layout that has SafeAreaEdges set that this settles in one extra pass and does not ping-pong on devices where the inset is larger than the free space.
| return false; | ||
| } | ||
|
|
||
| return GetSafeAreaView(crossPlatformLayout)?.IgnoreSafeArea == false; |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Logic and Correctness / Performance — The legacy fallback returns true for any ICrossPlatformLayout that implements only ISafeAreaView with IgnoreSafeArea == false (the default), but ApplyAdjustedSafeAreaInsetsPx does nothing for those views: when GetSafeAreaView2(layout) is null it takes the else branch and returns windowInsets unchanged without ever calling SetPadding. So for legacy/custom ISafeAreaView-only layouts this branch schedules an inset dispatch that can never produce padding — pure overhead on every bounds change, and it is the permissive default rather than the conservative one. Returning false when safeAreaView2 is null would match what the inset path can actually do.
|
|
||
| var grid = new Grid | ||
| { | ||
| SafeAreaEdges = SafeAreaEdges.All, |
There was a problem hiding this comment.
🔍 AI-Generated Review (multi-model)
[moderate] Regression Prevention / Test Coverage — The fixture cannot discriminate the new guard. SafeAreaEdges = SafeAreaEdges.All on this Grid is not what makes the new HasSafeAreaRegions(...) branch fire — Layout's default SafeAreaEdges is already Container, so GetSafeAreaRegionsForEdge returns non-None for all four edges with or without this line, and the screenshot would pass identically if the guard were simply changed with no HasSafeAreaRegions check at all. There is therefore no test for the negative case the guard exists to protect (a layout with SafeAreaEdges="None" / IgnoreSafeArea=true being resized must NOT trigger RequestApplyInsets), nor for the reverse transition (showing the TabBar again must remove the bottom padding). Please add a case that exercises a non-safe-area sibling layout and a hide→show round trip so a future change to the gate is actually caught.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@praveenkumarkarunanithi — new AI review results are available based on commit
36cbe75.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ✅ PASSED
Platform: ANDROID · Base: main · Merge base: 1749485a
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🖥️ Issue36269 Issue36269 |
✅ FAIL — 2400s | ✅ PASS — 561s |
🔴 Without fix — 🖥️ Issue36269: FAIL ✅ · 2400s
Error-relevant lines (filtered from the build log):
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Mono.AndroidTools.Internal.AdbOutputParsing.CheckInstallSuccess(String output, String packageName) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Mono.AndroidTools.AndroidDevice.<>c__DisplayClass105_0.<InstallPackage>b__0(Task`1 t) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: --- End of stack trace from previous location --- [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at AndroidDeviceExtensions.PushAndInstallPackageAsync(AndroidDevice device, PushAndInstallCommand command, CancellationToken token) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.InstallPackage(Boolean installed) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.RunInstall() [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
Build FAILED.
at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 retryDelay, Nullable`1 retryTimeout, Int32 cropLeft, Int32 cropRight, Int32 cropTop, Int32 cropBottom, Double tolerance) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 296
at Microsoft.Maui.TestCases.Tests.Issues.Issue36269.SafeAreaBottomPaddingIsAppliedWhenTabBarHiddenAtRuntime() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36269.cs:line 23
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
🟢 With fix — 🖥️ Issue36269: PASS ✅ · 561s
(no coded error found; showing last 1200 chars)
A total of 1 test files matched the specified pattern.
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.11] Discovering: Controls.TestCases.Android.Tests
[xUnit.net 00:00:00.37] Discovered: Controls.TestCases.Android.Tests
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 08/10/2026 22:47:04 FixtureSetup for Issue36269(Android)
>>>>> 08/10/2026 22:47:06 SafeAreaBottomPaddingIsAppliedWhenTabBarHiddenAtRuntime Start
>>>>> 08/10/2026 22:47:12 SafeAreaBottomPaddingIsAppliedWhenTabBarHiddenAtRuntime Stop
Passed SafeAreaBottomPaddingIsAppliedWhenTabBarHiddenAtRuntime [6 s]
NUnit Adapter 4.5.0.0: Test execution complete
Results File: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue36269.trx
Test Run Successful.
Total tests: 1
Passed: 1
Total time: 21.2897 Seconds
>>> TRX_RESULT_FILE: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue36269.trx
📁 Fix files reverted (3 files)
src/Core/src/Platform/Android/ContentViewGroup.cssrc/Core/src/Platform/Android/LayoutViewGroup.cssrc/Core/src/Platform/Android/SafeAreaExtensions.cs
📋 Pre-Flight — Context & Validation
Issue: #36269 - [Android] SafeAreaEdges break with tab navigation and Shell.TabBarIsVisible="False"
PR: #36474 - [WIP][Android] Fix stale bottom safe area after Shell tab navigation with TabBarIsVisible=False
Platform: Android (reported on Android 16; regression starts in MAUI 10.0.70)
Review commit: a73ec00ef8 (squashed PR commit), based on 1749485acc
Files changed: 3 implementation files, 2 UI-test source files, 2 Android snapshots
Problem and Reproduction
When a Shell tab page with SafeAreaEdges.All starts above the navigation bar and the tab bar is hidden at runtime, the page can be measured at a reduced height during tab transitions. Its bottom safe-area padding is recalculated to zero, then remains stale when the page grows back into the navigation-bar inset because a plain bounds change does not trigger another inset dispatch.
The issue reproduces after repeated navigation between two Shell tabs. Calling OnPropertyChanged(nameof(SafeAreaEdges)) is a reported workaround.
PR Fix
The PR changes both Android layout hosts:
ContentViewGroup.OnLayoutLayoutViewGroup.OnLayout
On a bounds change, each requests window insets again when its cross-platform layout is safe-area eligible. A new SafeAreaExtensions.HasSafeAreaRegions helper checks ISafeAreaView2 regions or legacy ISafeAreaView.IgnoreSafeArea.
This supersedes an earlier reviewed predicate based on MauiWindowInsetListener.IsViewTracked(this). That predicate was rejected because a safe-area-eligible view whose first computed padding is zero is not tracked, which is the concrete failing state.
Direct Diff Findings
- The current fix performs safe-area eligibility checks and can dispatch
RequestApplyInsetsfrom the AndroidOnLayouthot path whenever bounds change. - Both
ContentViewGroupandLayoutViewGroupneed equivalent behavior becauseSafeAreaEdgescan be set on pages or layouts. - Alternative candidates must avoid merely relocating the same resize predicate. They should use a different root-cause strategy, such as preserving/recomputing eligible zero-inset state or invalidating at the Shell/tab-bar lifecycle transition.
- Preserve all pre-existing dirty
.github/andeng/changes in the worktree; they are review infrastructure changes, not candidate changes.
Validation Inputs
Primary test (only):
pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform android -TestFilter "Issue36269"Gate evidence already proves this UI test fails without the PR fix and passes with it. Do not rerun gate verification.
Mandatory regression tests: None. The regression cross-reference result is CLEAN with zero overlaps/reverts.
Existing PR Candidate
| Candidate | Approach | Gate result | Implementation files |
|---|---|---|---|
| PR | Re-request insets from both Android layout hosts on safe-area-eligible bounds changes | PASS | ContentViewGroup.cs, LayoutViewGroup.cs, SafeAreaExtensions.cs |
The dedicated expert comparison is deferred to STEP 5b as required; no separate expert agent was launched during this pre-flight.
🔬 Code Review — Deep Analysis
Code Review — PR #36474
Independent Assessment
What this changes: On Android, ContentViewGroup and LayoutViewGroup now request a new window-inset dispatch whenever their bounds change and their cross-platform view reports any safe-area region. This lets overlap-based padding be recalculated after Shell changes a page's size. The PR adds an Android screenshot UI test for the hidden-TabBar scenario.
Inferred motivation: The existing inset callback computes padding from the view's current screen bounds, but a later bounds-only layout does not naturally rerun that callback. A page can therefore retain padding computed while it was shorter.
Expert Findings
- Major — hot-path scope is too broad.
Layoutdefaults toSafeAreaEdges.Container, soHasSafeAreaRegionsis true for ordinary layouts. Outside the listener's special suppression cases (such as recycler items and descendants ofMauiScrollView), every bounds change can now schedule a full inset traversal, including screen-metric and location queries. The fix should distinguish an explicitly safe-area-enabled or already-tracked view from an ordinary default layout. - Moderate — layout feedback needs a bound. The inset callback calls
SetPadding, while the affectedOnMeasureimplementations include padding in measured size. Content-sized or centered views can therefore take repeated layout → inset → padding passes; the current overlap clamp suggests convergence, but that adjacent case is not demonstrated. - Moderate — the legacy predicate is ineffective.
HasSafeAreaRegionsreturns true for anISafeAreaView-only implementation withIgnoreSafeArea == false, butApplyAdjustedSafeAreaInsetsPxapplies padding only throughISafeAreaView2. Those legacy-only dispatches cannot change padding. - Moderate — the test covers only the positive transition. The fixture proves the reported hide-TabBar regression, but does not discriminate the predicate's negative case or verify a hide → show round trip.
Blast Radius and Failure Modes
- Runs for ordinary instances: Yes for default
Layoutinstances that receive an inset listener; the new predicate treats the defaultContainerbehavior as active safe-area use. - Startup impact: No new startup path or static initialization.
- Static/shared state: None added.
- Attach/detach and configuration: Existing listener cleanup and
_didSafeAreaEdgeConfigurationChangereset paths remain symmetric. - Null/default state: Null cross-platform layouts are rejected; default layouts are the over-broad case rather than a null-safety failure.
- Repeated dispatch: A bounds change requests insets after arrange; a padding mutation can schedule another measure/layout. No unbounded loop is proven, but centered/Auto-sized layouts remain an unverified adjacent scenario.
- External output contract: Not applicable; the diff does not classify external tool output.
Test and CI Evidence
- Trusted Gate: passed — the regression test fails without the submitted fix and passes with it.
- Broader required-check status could not be queried because GitHub CLI authentication is unavailable.
Verdict: NEEDS_CHANGES
Confidence: low
The resize-triggered inset recomputation directly addresses the reproduced bug, but the predicate places expensive inset work on a much broader Android layout path than the PR description implies. A focused refinement should retain the proven Shell fix while narrowing dispatch eligibility and avoiding ineffective legacy-only requests.
🛠️ Try-Fix — Analysis & Comparison
Alternative Fix Candidates — PR #36474
Budget: 2 of at most 2 candidates completed
Primary test: pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform android -TestFilter "Issue36269"
Mandatory regression tests: None (regression-check is CLEAN)
Candidate 1 — Symmetric TabBar visibility inset dispatch
Model: claude-opus-5
Result: PASS
Files changed: src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRenderer.cs (+9/-5)
Self-review: 1 moderate finding; 0 critical/major
Artifacts: CustomAgentLogsTmp/PRState/36474/PRAgent/try-fix/attempt-1/
Narrative: CustomAgentLogsTmp/PRState/36474/PRAgent/try-fix-1/content.md
Approach
ShellItemRenderer.UpdateTabBarVisibility already posted RequestApplyInsets after a Gone -> Visible TabBar transition. Candidate 1 makes that behavior symmetric by posting the request whenever TabBar visibility actually changes, including Visible -> Gone.
The hypothesis is that hiding the TabBar is the specific lifecycle event that invalidates content bounds. The inset dispatch around that transition can observe the page's shorter, pre-transition bounds and calculate zero bottom overlap. Because an all-zero result is not tracked, no later correction occurs. Posting a second request after the visibility-triggered layout lets the dispatch observe settled full-height bounds.
This differs from the PR's broad resize strategy: it changes one Shell transition site rather than adding safe-area eligibility work to both Android layout-host OnLayout hot paths, and it does not use IsViewTracked.
Isolated diff
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRenderer.cs
index 4e68cad025..f8fe056970 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRenderer.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRenderer.cs
@@ -541,11 +541,15 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
_bottomView.Visibility = showTabs ? ViewStates.Visible : ViewStates.Gone;
- // After a Gone → Visible TabBar transition Android does not automatically re-dispatch
- // window insets to child views, leaving the displayed page with stale bottom padding
- // that creates empty space above the TabBar. Re-request insets after the layout pass
- // so safe-area padding is recalculated against the updated content bounds.
- if (wasGone && showTabs && DisplayedPage.Handler is IPlatformViewHandler { PlatformView: { } platformView })
+ // Either TabBar transition changes the height of the content area, and Android does not
+ // automatically re-dispatch window insets to child views afterwards. That leaves the
+ // displayed page with safe-area padding computed against its pre-transition bounds:
+ // stale bottom padding creating empty space above a restored TabBar (Gone → Visible), or
+ // no bottom padding at all once the page grows down into the navigation bar
+ // (Visible → Gone, issue #36269). Re-request insets after the transition's layout pass
+ // so safe-area padding is recalculated against the settled content bounds.
+ var isGone = !showTabs;
+ if (wasGone != isGone && DisplayedPage.Handler is IPlatformViewHandler { PlatformView: { } platformView })
{
platformView.Post(() =>
{Test result
The one permitted implementation/test pass succeeded; no correction or retest was used.
Total tests: 1
Passed: 1
Total time: 21.2812 Seconds
Result: SUCCESS
The test tapped HideTabBarButton, found BottomEdgeLabel, and completed the VerifyScreenshot comparison against the Android snapshot.
Failure/risk analysis
There was no test failure. Inline self-review recorded one moderate regression-prevention concern: this repair is keyed to an actual TabBar visibility change. If repeated tab navigation can produce the stale inset while the TabBar remains hidden, UpdateTabBarVisibility may not be re-entered, so that path might remain unfixed even though the committed primary test passes.
The baseline was restored through EstablishBrokenBaseline.ps1 -Restore; the candidate diff is no longer present and all pre-existing review-infrastructure changes remain intact.
Candidate 2 — Derive bottom ownership from Shell chrome state
Model: gpt-5.6-sol
Result: FAIL
Files changed: SafeAreaExtensions.cs (+38/-4), MauiWindowInsetListener.cs (+41/-0)
Self-review: 2 findings (1 major, 1 moderate)
Artifacts: CustomAgentLogsTmp/PRState/36474/PRAgent/try-fix/attempt-2/
Narrative: CustomAgentLogsTmp/PRState/36474/PRAgent/try-fix-2/content.md
Approach
Candidate 2 cached the real material BottomNavigationView visibility and retained measured height at the root inset listener. During descendant safe-area calculation, it attempted to preserve the full bottom inset when Shell's bottom navigation was hidden, layout was pending, and the safe-area view filled the ancestor chain to navigationlayout_content.
This differed from both prior strategies: it added neither the PR's resize-triggered OnLayout re-dispatch nor candidate 1's posted TabBar visibility-transition dispatch. The state was intended to remain applicable during repeated navigation while the TabBar was already hidden.
The final isolated diff added WillFillShellContentAfterBottomNavigationRemoval to SafeAreaExtensions and Shell bottom-navigation state/discovery to MauiWindowInsetListener. The exact diff is preserved in try-fix/attempt-2/fix.diff and reproduced in try-fix-2/content.md.
Test result
The first implementation inferred state from the fragment container. After it failed, the one permitted focused correction recursively located the actual BottomNavigationView and included its retained height in the pending-layout tolerance.
Initial: Failed 1/1 — snapshot difference 9.52% — 27.8172s
Retest: Failed 1/1 — snapshot difference 9.52% — 28.6609s
Both runs built and deployed, tapped HideTabBarButton, found BottomEdgeLabel, and failed only at VerifyScreenshot.
Failure/risk analysis
The identical visual failure shows that the state-based branch did not activate effectively in the real Shell hierarchy. Either usable BottomNavigationView state did not reach the shared listener before descendant dispatch, or the vertically-filling pending-layout predicate rejected the actual page path. A further diagnostic cycle would exceed the correction budget.
Inline self-review recorded a major correctness finding for the empirically ineffective branch and a moderate Android performance finding for recursively traversing the bottom-tab subtree on every root inset dispatch.
The baseline was restored through EstablishBrokenBaseline.ps1 -Restore, the remaining candidate-only patch was removed, and all pre-existing review-infrastructure changes remain intact.
📝 PR Finalize — Recommended Title & Description
Assessment: ✏️ Recommend updating — the title still carries a [WIP] prefix, and the description incorrectly says no test or new method was added while the submitted PR includes an Android UI test, two screenshot baselines, and HasSafeAreaRegions.
Recommended title
[Android] SafeArea: Reapply insets after Shell TabBar resize
Recommended description
### Root Cause
On Android, a page's bottom safe-area padding is derived from its current height—how far it extends into the navigation bar area. During Shell tab navigation with `Shell.TabBarIsVisible="false"`, the page (which shares the Shell pager) is briefly remeasured at a reduced height as the TabBar animates back in on the other tab. While it is shorter, its bottom padding is recalculated to zero.
This zeroing is triggered by PR #34324, which added a `RequestApplyInsets` dispatch when the TabBar transitions from hidden to visible to fix the related empty-space issue #33703. That dispatch runs while the returning page is still measured at the reduced height, so its bottom padding is recomputed to zero.
When the page grows back to full height, Android does not recompute safe-area padding on a plain resize—only on an inset dispatch, a `SafeAreaEdges` change, or a configuration change. The stale zero padding therefore persists and content slides under the navigation bar.
### Description of Change
When an Android safe-area view is laid out at new bounds, `ContentViewGroup` and `LayoutViewGroup` request Android to reapply window insets so overlap-based padding is recalculated against the updated size.
The request is gated by the existing inset-listener state and by the cross-platform layout's effective safe-area regions. The new internal `SafeAreaExtensions.HasSafeAreaRegions` helper supports both `ISafeAreaView2` regions and the legacy `ISafeAreaView.IgnoreSafeArea` contract. Applying the change to both platform groups covers `SafeAreaEdges` configured on a page or on a layout. No public API is added.
### Issues Fixed
Fixes #36269
### Testing
- Added the Android-only `Issue36269.SafeAreaBottomPaddingIsAppliedWhenTabBarHiddenAtRuntime` UI screenshot test.
- Added Android and Android notch/API 36 screenshot baselines.
Tested the behavior on:
- [x] Android
- [ ] Windows
- [ ] iOS
- [ ] Mac
### Regression Details
Regressed by PR #34324
### Output Video
Before Issue Fix | After Issue Fix
|---|---|
| <video width="40" height="60" alt="Before Fix" src="https://github.com/user-attachments/assets/b6d4cae8-32c6-473d-9663-a83d285479cd"> | <video width="50" height="40" alt="After Fix" src="https://github.com/user-attachments/assets/97bceb3e-1581-426e-ab56-e944e12dded4"> |
🏁 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Winner: pr-plus-reviewer
The submitted fix correctly targets the stale-inset mechanism and its regression test passes, but expert review found that its resize predicate treats the default Layout safe-area behavior as active and can trigger a full Android inset traversal on ordinary bounds changes. pr-plus-reviewer retains the general resize-based repair needed for repeated navigation while limiting redispatch to explicit active safe-area configurations or views already tracked as padded. Its required Android Issue36269 validation passed.
| Rank | Candidate | Regression result | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
PASS | Preserves the PR's general fix, removes ineffective legacy-only redispatches, excludes explicit None, and narrows default-layout work to already-tracked views. Best correctness/performance balance. |
| 2 | pr |
PASS (trusted Gate) | Fixes the reproduced failure and covers both Android layout hosts, but performs potentially expensive inset redispatches for a broad set of default layouts. Expert verdict: changes needed. |
| 3 | try-fix-1 |
PASS | Efficient one-file symmetric Shell TabBar visibility dispatch, but it is tied to an actual visibility transition and may miss the issue's repeated-navigation path while the TabBar is already hidden. |
| 4 | try-fix-2 |
FAIL | Both focused runs produced the same 9.52% screenshot difference; it is empirically ineffective and adds recursive hierarchy traversal. Per the ranking rule, it remains below every passing candidate. |
Expert Review Disposition
- Addressed in the winner: over-broad default-layout redispatch; ineffective
ISafeAreaView-only fallback; explicitSafeAreaEdges.Noneexclusion. - Residual uncertainty: centered/Auto-sized layouts may require more than one layout/inset pass, though no non-converging transition was proven.
- Coverage gap: the submitted UI test proves the hide transition but does not separately exercise a negative no-safe-area case or hide/show round trip.
Because the winning refinement is not present in the submitted PR HEAD, the PR should be updated before approval.
📱 UI Tests — SafeAreaEdges,ViewBaseTests
Detected UI test categories: SafeAreaEdges,ViewBaseTests
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
SafeAreaEdges |
92/102 (10 ⚠ new baseline) | — |
ViewBaseTests |
118/119 (1 skipped) ✓ | — |
🔍 AI analysis of failures — PR-related vs unrelated
🔍 AI-generated triage (GitHub Copilot CLI) — a heuristic judgement of whether each deep UI test failure is connected to this PR's changes. Verify before relying on it.
Likely unrelated: the failures appear pre-existing, flaky, or infrastructure.
- ● Unrelated — Missing Android SafeAreaEdges visual baselines (~10 tests): every failure stops at “Baseline snapshot not yet created,” and the PR neither changes these tests nor removes or edits their named snapshots, so no rendering comparison was performed.
Strongest signal: all failures are missing-baseline errors rather than visual mismatches, despite the PR’s Android safe-area code overlap.
⚠️ SafeAreaEdges — 10 new snapshot tests need a baseline PNG
These tests call VerifyScreenshot but their baseline image isn't committed yet (brand-new snapshot tests get their baseline added separately by a maintainer). There's nothing to compare against, so this is not a regression — download the drop-deep-uitests artifact, confirm the rendering, and commit the baseline PNG.
ToolbarExtendsAllTheWayLeftAndRight_NavigationPageVerifyDefaultFlyoutItemsRenderingToolbarExtendsAllTheWayLeftAndRight_FlyoutPageVerifyCustomFlyoutContentWithHeaderFooterVerifyCustomFlyoutContentTemplateWithHeaderFooterVerifyCustomFlyoutContentRenderingVerifyFlyoutWithHeaderFooterVerifyCustomFlyoutContentTemplateRenderingLayoutShouldBeCorrectOnFirstNavigationToolbarExtendsAllTheWayLeftAndRight_Shell
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)
🧭 Next Steps — reviewer patch required (pr-plus-reviewer)
The reviewer-enhanced candidate won, so the submitted PR still needs those changes.
Why: The refined PR preserves the validated resize-based fix while narrowing inset redispatch to explicit active safe-area configurations or already-tracked padded views. It passed the required Android Issue36269 test and avoids the raw PR's confirmed broad hot-path cost.
Apply PRAgent/pr-plus-reviewer/reviewer.patch from the CopilotLogs
artifact (or follow the report's Required submitted-PR change), push the update, and run
the review again.
kubaflo
left a comment
There was a problem hiding this comment.
What's the status of this one?
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
On Android, a page's bottom safe area padding is derived from its current height—how far it extends into the navigation bar area. During Shell tab navigation with
Shell.TabBarIsVisible="false", the page (which shares the Shell pager) is briefly re-measured at a reduced height as the TabBar animates back in on the other tab, and while it is shorter, its bottom padding is recalculated to zero.This zeroing is triggered by PR #34324, which added a
RequestApplyInsetsdispatch when the TabBar transitions from hidden to visible (to fix the related empty-space issue, #33703). That dispatch runs while the returning page is still measured at the reduced height, so its bottom padding is recomputed to zero.When the page grows back to full height, Android does not recompute safe area padding on a plain resize—only on an inset dispatch, a
SafeAreaEdgeschange, or a configuration change. As a result, the stale zero padding persists and the content slides under the navigation bar.Description of Change
When a safe area view is re-laid out at a new size, the fix requests Android to re-apply window insets during the layout pass so the bottom padding is recalculated against the updated height.
The fix is scoped to run only for views that actually use safe area, so ordinary pages and normal size changes are unaffected. It reuses the existing layout and window inset handling, introducing no new method or public API.
Because
SafeAreaEdgescan be applied either to a page or to a layout, the same change is applied to bothContentViewGroupandLayoutViewGroup.Issues Fixed
Fixes #36269
Tested the behaviour in the following platforms
Note
The test couldn't be added — the bug needs real-device timing the harnesses can't reproduce (UI tests settle the navigation, so padding never goes stale; device tests aren't edge-to-edge, so the inset never reaches the page).
Regression Details:
Regressed by PR #34324
Output Video
BeforeFix.42.mov
AfterFix.49.mov