Skip to content

Fix Android layout jump when navigating with IME open and NavBarIsVisible=false#34621

Open
jpd21122012 wants to merge 2 commits intodotnet:mainfrom
jpd21122012:fix/34584-Shell-NavBar-SafeAreaEdges
Open

Fix Android layout jump when navigating with IME open and NavBarIsVisible=false#34621
jpd21122012 wants to merge 2 commits intodotnet:mainfrom
jpd21122012:fix/34584-Shell-NavBar-SafeAreaEdges

Conversation

@jpd21122012
Copy link
Contributor

Description

On Android, navigating from a page with the soft keyboard (IME) open to a page with Shell.NavBarIsVisible="False" causes the destination page to initially render under the status bar and then jump into the correct position.

This behavior is more noticeable when the destination page performs heavier UI work, and in some cases the layout may remain incorrectly positioned.

Root Cause

During ShellRenderer.SwitchFragment, the fragment transaction is committed while the IME is still visible. Android continues to report IME-related WindowInsets, causing the new layout to be measured with incorrect top insets.

Once the IME state stabilizes, the layout is corrected, resulting in a visible jump.

Fix

Dismiss the soft keyboard before performing the fragment transaction:

  • Detect IME visibility using IsSoftInputShowing
  • Call HideSoftInput() prior to FragmentTransaction

This ensures that WindowInsets are stable before the new layout is measured.

Result

  • Eliminates layout jump when navigating with keyboard open
  • Ensures correct layout positioning from initial render
  • Improves Shell navigation consistency on Android

Testing

Added a UI test (Issue34584) that:

  • Opens the keyboard by focusing an Entry
  • Navigates to a page with Shell.NavBarIsVisible="False"
  • Verifies that content is laid out below the status bar (Y > 0)

Note: The test validates final layout correctness, not visual animation.

Related Issues

Copilot AI review requested due to automatic review settings March 24, 2026 19:56
@github-actions
Copy link
Contributor

github-actions bot commented Mar 24, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34621

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34621"

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an Android Shell navigation layout jump when transitioning while the soft keyboard (IME) is visible—especially when navigating to destinations with Shell.NavBarIsVisible=false.

Changes:

  • Dismisses the soft keyboard before committing the fragment transaction in ShellRenderer.SwitchFragment.
  • Adds a HostApp repro page for issue #34584 (keyboard open → navigate to NavBar hidden page).
  • Adds an Android UI test intended to validate the destination content is not laid out under the status bar.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellRenderer.cs Attempts to hide IME before fragment replacement to stabilize insets/layout.
src/Controls/tests/TestCases.HostApp/Issues/Issue34584.cs Adds a Shell-based repro page with an Entry + navigation to a NavBar-hidden destination.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34584.cs Adds an Android UI test to verify final element Y-position is below the status bar after navigation.

Update the test to trigger navigation from Entry.Completed instead of a button tap,
ensuring the soft keyboard (IME) remains visible at the moment navigation occurs.

This makes the test deterministic and accurately reproduces the original issue scenario.
@github-actions
Copy link
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The test exercises the correct code path and the fix is Android-only with a matching platform guard, but the assertion is weak and there are minor issues (unused HostApp element, redundant null check, missing edge cases).

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34621 — Fix Android layout jump when navigating with IME open and NavBarIsVisible=false
Test files evaluated: 1 (UI Test)
Fix files: 1 (ShellRenderer.cs)


Overall Verdict

⚠️ Tests need improvement

The test correctly exercises the fix's code path, but the assertion (Y > 0) is too weak to reliably catch regressions, and a NavigateButton element is defined in the HostApp but never used in the test.


1. Fix Coverage — ✅

The fix calls rootView.HideSoftInput() in SwitchFragment when the IME is showing. The test opens the keyboard (via App.Tap("Entry") + App.EnterText), triggers navigation via App.PressEnter() (which fires entry.Completed → GoToAsync), then checks the label's position on the destination page. This correctly exercises the patched code path and would fail if the fix were reverted (content would render at Y=0 under the status bar).


2. Edge Cases & Gaps — ⚠️

Covered:

  • Navigation with keyboard open via IME action (Enter key)
  • Destination page with NavBarIsVisible=False

Missing:

  • Navigation via button tapNavigateButton is defined in the HostApp but never used in the test. The fix in SwitchFragment should also apply when navigating by tapping a button with the keyboard still open; this path is untested.
  • Navigation without keyboard open — No regression check confirming normal navigation still works correctly after the fix.
  • Threshold for "under status bar" — The assertion Y > 0 would pass even at Y = 1px, which could still be visually wrong. A more meaningful assertion would check Y >= statusBarHeight (or use a screenshot comparison).

3. Test Type Appropriateness — ✅

Current: UI Test (Appium)
Recommendation: Appropriate. This fix requires native IME interaction (soft keyboard) and a full Shell navigation lifecycle with fragment switching. Device tests could theoretically cover part of this, but orchestrating the IME-active state during navigation is significantly easier with Appium. UI test is the right choice here.


4. Convention Compliance — ⚠️

  • ✅ Class named Issue34584, inherits _IssuesUITest, correct [Category(UITestCategories.Shell)]
  • [Issue()] attribute present on HostApp page with PlatformAffected.Android
  • WaitForElement used before interactions
  • ⚠️ #if ANDROID wraps the entire class — This is an accepted pattern in this codebase (e.g., Issue32041AdjustPan.cs, Material3CheckBoxDefaultAppearanceTest.cs) when a fix is strictly Android-only. Acceptable here.
  • ⚠️ NavigateButton AutomationId defined in HostApp but never referenced in the test. Either the test should use it (see Edge Cases above) or it should be removed to avoid confusion.
  • ⚠️ TestShell base class is used correctly; HostApp follows the standard pattern.

5. Flakiness Risk — ✅ Low

  • WaitForElement("TargetLabel") is correctly placed before the assertion
  • App.EnterText + App.PressEnter() is a reliable way to trigger IME-based navigation
  • No Task.Delay / Thread.Sleep
  • No screenshot comparison, so no cursor blink risk
  • Minor concern: App.PressEnter() behavior can vary slightly across Android versions/IME implementations, but this is low risk given the test uses standard Appium APIs

6. Duplicate Coverage — ✅ No duplicates

No existing test covers the specific scenario of Shell navigation with IME active and NavBarIsVisible=False. The similar Shell tests found are for unrelated scenarios.


7. Platform Scope — ✅

The fix is in Android/ShellRenderer.cs (Android-only), the test is guarded with #if ANDROID. Platform scope is correctly matched.


8. Assertion Quality — ⚠️

var label = App.FindElement("TargetLabel");
Assert.That(label, Is.Not.Null);          // ❌ Redundant — WaitForElement already ensures presence

var labelRect = label.GetRect();
Assert.That(labelRect.Y, Is.GreaterThan(0),   // ⚠️ Weak — catches Y=0 but not Y=1 or Y=5
    "TargetLabel should not be at Y=0 (would be under the status bar)");

The first assertion is redundant — WaitForElement + FindElement would already throw if the element doesn't exist. The second assertion is minimal: it catches the specific Y=0 regression but a value of Y=1 or Y=5 would pass, even though content might still be partially under the status bar. Consider either:

  • Using Is.GreaterThanOrEqualTo(statusBarApproximateHeight) with a conservative lower bound
  • Adding VerifyScreenshot() with retryTimeout as a visual check

9. Fix-Test Alignment — ✅

  • Fix: ShellRenderer.SwitchFragment hides soft input before switching fragments
  • Test: Opens Entry (raises IME), navigates via IME action (Enter), verifies destination label is positioned correctly

The test targets exactly what the fix addresses.


Recommendations

  1. Remove Assert.That(label, Is.Not.Null) — it's redundant and adds noise.
  2. Strengthen the Y assertion — use a more meaningful threshold such as Is.GreaterThan(50) (approximate status bar height in dp) or add VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2)) for visual regression coverage.
  3. Use or remove NavigateButton — either add a second test method that navigates via button tap with the keyboard open (covering that code path), or remove the NavigateButton from Issue34584_MainPage to avoid dead code.

Note

🔒 Integrity filtering filtered 3 items

Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

PureWeen added a commit that referenced this pull request Mar 25, 2026
## Summary

Enables the copilot-evaluate-tests gh-aw workflow to run on fork PRs by
adding `forks: ["*"]` to the `pull_request` trigger and removing the
fork guard from `Checkout-GhAwPr.ps1`.

## Changes

1. **copilot-evaluate-tests.md**: Added `forks: ["*"]` to opt out of
gh-aw auto-injected fork activation guard. Scoped `Checkout-GhAwPr.ps1`
step to `workflow_dispatch` only (redundant for other triggers since
platform handles checkout).

2. **copilot-evaluate-tests.lock.yml**: Recompiled via `gh aw compile` —
fork guard removed from activation `if:` conditions.

3. **Checkout-GhAwPr.ps1**: Removed the `isCrossRepository` fork guard.
Updated header docs and restore comments to accurately describe behavior
for all trigger×fork combinations (including corrected step ordering).

4. **gh-aw-workflows.instructions.md**: Updated all stale references to
the removed fork guard. Documented `forks: ["*"]` opt-in, clarified
residual risk model for fork PRs, and updated troubleshooting table.

## Security Model

Fork PRs are safe because:
- Agent runs in **sandboxed container** with all credentials scrubbed
- Output limited to **1 comment** via `safe-outputs: add-comment: max:
1`
- Agent **prompt comes from base branch** (`runtime-import`) — forks
cannot alter instructions
- Pre-flight check catches missing `SKILL.md` if fork isn't rebased on
`main`
- No workspace code is executed with `GITHUB_TOKEN` (checkout without
execution)

## Testing

- ✅ `workflow_dispatch` tested against fork PR #34621
- ✅ Lock.yml statically verified — fork guard removed from `if:`
conditions
- ⏳ `pull_request` trigger on fork PRs can only be verified post-merge
(GitHub Actions reads lock.yml from default branch)

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community ✨ Community Contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shell page without NavBar jumping when navigating with keyboard open

2 participants