[iOS] Fix stale bottom safe area after changing SafeAreaEdges with keyboard open#35083
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35083Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35083" |
There was a problem hiding this comment.
Pull request overview
Fixes an iOS safe-area regression where bottom padding can remain “stuck” after SafeAreaEdges is changed while the keyboard is open, by resetting keyboard-related safe-area state when unsubscribing from keyboard notifications and adding a UI test that reproduces the reported sequence.
Changes:
- Clear keyboard state during
UnsubscribeFromKeyboardNotifications()to prevent stale_isKeyboardShowing/_keyboardFramefrom affecting later safe area calculations. - Refactor keyboard-hide handling to reuse a shared
ClearKeyboardState()helper. - Add a UITest that toggles
SafeAreaEdges, shows/dismisses the keyboard, and verifies the layout returns to baseline without extra whitespace.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Core/src/Platform/iOS/MauiView.cs | Resets keyboard state on unsubscribe and centralizes keyboard-hide cleanup to avoid stale safe-area padding. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28986_ContentPage.cs | Adds a regression test covering the “toggle edges while keyboard open, then dismiss, then toggle back” sequence from the issue. |
| // Clear stale keyboard state so that re-subscribing later doesn't | ||
| // pick up a phantom keyboard frame from a previous session (#34846). | ||
| if (_isKeyboardShowing) | ||
| { | ||
| ClearKeyboardState(); | ||
| } |
There was a problem hiding this comment.
Clearing keyboard state during UnsubscribeFromKeyboardNotifications() fixes the stale-padding case when the keyboard is dismissed while unsubscribed, but it can regress the opposite flow: if SafeAreaEdges toggles back to a SoftInput/All mode while the keyboard is still visible, the view will re-subscribe without receiving a new WillShow notification, leaving _isKeyboardShowing false and _keyboardFrame empty, so no keyboard padding will be applied until the keyboard changes again. Consider keeping keyboard observers active until WillHide is observed (then unsubscribe), or keep the last keyboard frame and rehydrate _isKeyboardShowing on subscribe when a first responder exists.
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review the ai's summary?
🤖 AI Summary
📊 Review Session —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35083 | Clear _isKeyboardShowing/_keyboardFrame on unsubscribe (via ClearKeyboardState()); refactor OnKeyboardWillHide to call same helper |
✅ PASSED (Gate) | MauiView.cs, test file |
Original PR |
🔬 Code Review — Deep Analysis
Code Review — PR #35083
Independent Assessment
What this changes: MauiView.cs gets a new private ClearKeyboardState() helper extracted from OnKeyboardWillHide. That helper is then called inside UnsubscribeFromKeyboardNotifications() when _isKeyboardShowing is true at unsubscription time. A new UI test exercises the exact subscribe/change-edges/dismiss/re-subscribe cycle.
Inferred motivation: Something causes keyboard observer state (_isKeyboardShowing, _keyboardFrame) to go stale when the view unsubscribes while the keyboard is open. On re-subscription the stale frame produces phantom bottom padding.
Reconciliation with PR Narrative
Author claims: When SafeAreaEdges switches from a SoftInput-inclusive mode (e.g. All) to a non-SoftInput mode (e.g. Container) while the keyboard is visible, the view unsubscribes from iOS keyboard notifications without resetting _isKeyboardShowing/_keyboardFrame. The subsequent KeyboardWillHide event is never received. Switching back to All re-subscribes with stale state, applying an incorrect bottom padding equal to the old keyboard height.
Agreement/disagreement: The root cause analysis matches the code exactly. SubscribeToKeyboardNotifications() guards against double-subscription with an early return, so re-subscribing on a view that still has _isKeyboardShowing = true was the only path to the phantom padding. The fix is placed at the right callsite.
Findings
💡 Suggestion — macCatalyst coverage gap
The fix lives in MauiView.cs, which compiles for both iOS and macCatalyst (.ios.cs convention). The new test is #if IOS || ANDROID, so macCatalyst never exercises this path. macCatalyst external keyboards can open/close and UIKeyboard.WillHide notifications fire there too. If the same SafeAreaEdges + keyboard cycle is reproducible on Mac Catalyst, a separate macCatalyst UI test (or extending the #if to include MACCATALYST) would close the coverage gap.
💡 Suggestion — No wait between GridSetContainerButton and DismissKeyboard()
In SafeAreaNoWhiteSpaceAfterKeyboardDismissAndEdgeToggle, App.Tap("GridSetContainerButton") triggers UpdateKeyboardSubscription → UnsubscribeFromKeyboardNotifications → ClearKeyboardState → SetNeedsLayout. The layout pass is async; App.DismissKeyboard() fires immediately after without a RetryAssert. In practice this works because the Appium tap is synchronous and the main-thread layout is fast, but if this test becomes flaky on slower CI devices, inserting a App.WaitForElement("ContentGrid") between the two taps would make the sequencing deterministic.
Devil's Advocate
Challenge my approval:
- Could
ClearKeyboardState()callingSetNeedsLayout()insideUnsubscribeFromKeyboardNotifications()cause an extra layout pass in the normal (keyboard-not-showing) unsubscription path? No — the guardif (_isKeyboardShowing)ensures this only fires when the keyboard was actually open. - Could there be a window where
OnKeyboardWillHidefires between the observer removal and the_isKeyboardShowingcheck? iOS notifications are dispatched on the main thread; sinceUnsubscribeFromKeyboardNotificationsalso runs on the main thread, there is no race. - Does the refactoring change
OnKeyboardWillHidebehavior? The extractedClearKeyboardState()body is bit-for-bit identical to the oldOnKeyboardWillHidebody. Behaviour is preserved.
Challenge my suggestions:
- The macCatalyst gap is real but low-priority: hardware keyboard interaction on macCatalyst is a different UX than an on-screen keyboard dismiss, and CI for macCatalyst SafeArea tests is expensive. It is reasonable to defer to a follow-up.
- The wait concern in the test is theoretical; all five existing tests in the same class use the same pattern without flake reports.
Verdict: LGTM
Confidence: high
Summary: The fix is minimal, correctly placed, and addresses the root cause without touching unrelated code paths. The ClearKeyboardState() extraction is a clean refactor with no behavioral change to OnKeyboardWillHide. All CI checks pass. The two observations above are low-risk suggestions, not blockers.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Guard read-site with IsSubscribedToKeyboardNotifications property; stale state ignored when unsubscribed; unconditional clear on BLOCKED |
1 file | Appium/XCTest timeout (OneTimeSetUp not a code failure | 60s) unsubscribe |
| 2 | try-fix (claude-sonnet-4.6) | Drain-mode WillHide keep WillHide observer alive when unsubscribing while keyboard is showing; self-removes on WillHide fire; also fixes reverse edge case PASS | 1 file | More complex lifecycle; handles keyboard-still-visible re-subscribe case | |
| 3 | try-fix (gpt-5.3-codex) | Re-hydrate keyboard state on subscribe from / | PASS | 1 file | Depends on global keyboard tracker; handles both stale and still-visible cases |
| 4 | try-fix (gpt-5.4) | Subscription-versioned keyboard state stamped with subscription generation, stale state self-invalidates PASS | 1 file | Most complex approach; versioning overhead | |
| 5 | try-fix (claude-opus-4.6 cross-poll) | Always-subscribed observers while in gate keyboard padding at read-site with | window PASS | 1 file | Eliminates problem class; more complex subscription model |
| 6 | try-fix (claude-sonnet-4.6 cross-poll) | UIKeyboardLayoutGuide live compute overlap from live guide FAIL | 1 file | Guide requires active constraints; always CGRect.Zero without them | frame |
| PR | PR #35083 | Clear / on unsubscribe (conditional ) via helper; refactor to reuse same helper | PASSED (Gate) | 2 files | Original minimal and targeted |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| ran as attempt 5 | |||
| ran as attempt | 6 | ||
| covered by attempt 6 | |||
| gpt-5.4 | 2 | N/A | (not queried merged with round 2) |
| claude-opus-4.6 | 3 | Yes | First-responder ground-truth check at read- redundant given 4 passing alternatives |
| gpt-5.4 | 3 | Yes | UIKeyboardWillChangeFrame-driven more complex, no clear advantage |
Exhausted: Yes (3 cross-poll rounds; remaining ideas add complexity without quality improvement for this specific bug)
Selected Fix: PRs Simplest (10 net lines), correctly targeted at root cause, minimal blast radius, Gate PASSED, code review LGTM (high confidence). Alternatives 5 all pass but add unnecessary complexity.2 fix
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #34846, PR #35083, 2 files changed (1 impl + 1 test), prior Copilot inline review imported |
| Code Review | LGTM (high) | 0 errors, 0 warnings, 2 low-priority suggestions |
| Gate | ✅ PASSED | ios — tests FAIL without fix, PASS with fix |
| Try-Fix | ✅ COMPLETE | 6 attempts: 1 blocked (infra), 4 passing alternatives, 1 failed; 3 cross-poll rounds exhausted |
| Report | ✅ COMPLETE |
Code Review Impact on Try-Fix
Code review returned LGTM with zero errors and two low-priority suggestions (macCatalyst test coverage gap; minor sequencing concern in test). Since there were no ❌ errors to target, the hints field in try-fix prompts focused on the reverse-edge-case concern noted by the prior Copilot inline reviewer (keyboard still visible when re-subscribing). This guided attempts 2 and 3 to explicitly address that edge case with drain-mode and re-hydration approaches. None of the error-free code review findings changed the final recommendation.
Summary
PR #35083 fixes a stale keyboard state bug on iOS where changing SafeAreaEdges from All/SoftInput to Container/non-SoftInput while the keyboard is open causes phantom bottom whitespace. The PR's fix is minimal (10 net lines), correctly targeted, and passes both the Gate and 4 of 5 independently-explored alternative approaches. The code review found no correctness issues. Multi-model try-fix exploration confirmed the PR's approach is the simplest of all passing candidates.
Root Cause
When UnsubscribeFromKeyboardNotifications() is called with _isKeyboardShowing = true (i.e., keyboard was open when SafeAreaEdges changed to a non-SoftInput mode), the view stops receiving UIKeyboard.WillHideNotification. The cached state _isKeyboardShowing = true and _keyboardFrame (non-empty) persist. On re-subscribing to SoftInput mode, SubscribeToKeyboardNotifications() guards against double-subscription but does not re-hydrate state — it just adds observers for future notifications. GetAdjustedSafeAreaInsets() reads the stale _isKeyboardShowing = true and applies phantom bottom padding.
Fix Quality
The PR's fix is excellent quality:
- Targeted: adds one
if (_isKeyboardShowing)guard block inUnsubscribeFromKeyboardNotifications()callingClearKeyboardState() - Clean refactor:
OnKeyboardWillHideis simplified to a one-liner callingClearKeyboardState()— identical behavior, reduced duplication - No regression risk: the guard ensures
SetNeedsLayout()is only called when the keyboard was actually showing at unsubscription time - Well-tested: new
SafeAreaNoWhiteSpaceAfterKeyboardDismissAndEdgeToggletest reproduces exact reported sequence - Prior Copilot review concern about reverse edge case (keyboard still visible when re-subscribing): this is a separate scenario not reported as a bug, and this PR does not make it worse — it was already unhandled before this change. Multiple try-fix alternatives (drain-mode, re-hydration) that address this were more complex without being demonstrably better for the reported bug.
…yboard open (#35083) <!-- 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! ### Root Cause When `SafeAreaEdges` changes from a mode that includes `SoftInput` (e.g., `All`) to one that does not (e.g., `Container`), the view unsubscribes from iOS keyboard notifications. However, the keyboard state (`_isKeyboardShowing`, `_keyboardFrame`) was not cleared. If the keyboard was open during unsubscription and later dismissed, the `OnKeyboardWillHide` event is never received. When switching back to a `SoftInput` mode, the view re-subscribes with stale state, applying incorrect bottom padding equal to the old keyboard height instead of the home indicator inset. ### Description of Change Added state cleanup in `UnsubscribeFromKeyboardNotifications` in `MauiView.cs`. When unsubscribing, if the keyboard was previously visible, the method now clears the keyboard state, invalidates the safe area, and triggers a layout update. This prevents stale keyboard state from persisting across subscribe/unsubscribe cycles while preserving existing behavior in all other scenarios. Also added a test in `Issue28986_ContentPage` that reproduces the exact sequence: switch to All, open keyboard, change to Container, dismiss keyboard, switch back to All, and verify the layout returns to baseline without phantom padding. ### Issues Fixed Fixes #34846 Tested the behaviour in the following platforms - [x] Android - [ ] Windows - [x] iOS - [x] Mac ### Screenshots | Before Issue Fix | After Issue Fix | |------------------|-----------------| | <video width="350" alt="withoutfix" src="https://github.com/user-attachments/assets/48e5afe8-4b6c-4c5a-b223-91312bf212e3" /> | <video width="350" alt="withfix" src="https://github.com/user-attachments/assets/a7fa1af6-8927-4fed-a7f2-f8011bdbef8f" /> |
…yboard open (#35083) <!-- 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! ### Root Cause When `SafeAreaEdges` changes from a mode that includes `SoftInput` (e.g., `All`) to one that does not (e.g., `Container`), the view unsubscribes from iOS keyboard notifications. However, the keyboard state (`_isKeyboardShowing`, `_keyboardFrame`) was not cleared. If the keyboard was open during unsubscription and later dismissed, the `OnKeyboardWillHide` event is never received. When switching back to a `SoftInput` mode, the view re-subscribes with stale state, applying incorrect bottom padding equal to the old keyboard height instead of the home indicator inset. ### Description of Change Added state cleanup in `UnsubscribeFromKeyboardNotifications` in `MauiView.cs`. When unsubscribing, if the keyboard was previously visible, the method now clears the keyboard state, invalidates the safe area, and triggers a layout update. This prevents stale keyboard state from persisting across subscribe/unsubscribe cycles while preserving existing behavior in all other scenarios. Also added a test in `Issue28986_ContentPage` that reproduces the exact sequence: switch to All, open keyboard, change to Container, dismiss keyboard, switch back to All, and verify the layout returns to baseline without phantom padding. ### Issues Fixed Fixes #34846 Tested the behaviour in the following platforms - [x] Android - [ ] Windows - [x] iOS - [x] Mac ### Screenshots | Before Issue Fix | After Issue Fix | |------------------|-----------------| | <video width="350" alt="withoutfix" src="https://github.com/user-attachments/assets/48e5afe8-4b6c-4c5a-b223-91312bf212e3" /> | <video width="350" alt="withfix" src="https://github.com/user-attachments/assets/a7fa1af6-8927-4fed-a7f2-f8011bdbef8f" /> |
…yboard open (#35083) <!-- 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! ### Root Cause When `SafeAreaEdges` changes from a mode that includes `SoftInput` (e.g., `All`) to one that does not (e.g., `Container`), the view unsubscribes from iOS keyboard notifications. However, the keyboard state (`_isKeyboardShowing`, `_keyboardFrame`) was not cleared. If the keyboard was open during unsubscription and later dismissed, the `OnKeyboardWillHide` event is never received. When switching back to a `SoftInput` mode, the view re-subscribes with stale state, applying incorrect bottom padding equal to the old keyboard height instead of the home indicator inset. ### Description of Change Added state cleanup in `UnsubscribeFromKeyboardNotifications` in `MauiView.cs`. When unsubscribing, if the keyboard was previously visible, the method now clears the keyboard state, invalidates the safe area, and triggers a layout update. This prevents stale keyboard state from persisting across subscribe/unsubscribe cycles while preserving existing behavior in all other scenarios. Also added a test in `Issue28986_ContentPage` that reproduces the exact sequence: switch to All, open keyboard, change to Container, dismiss keyboard, switch back to All, and verify the layout returns to baseline without phantom padding. ### Issues Fixed Fixes #34846 Tested the behaviour in the following platforms - [x] Android - [ ] Windows - [x] iOS - [x] Mac ### Screenshots | Before Issue Fix | After Issue Fix | |------------------|-----------------| | <video width="350" alt="withoutfix" src="https://github.com/user-attachments/assets/48e5afe8-4b6c-4c5a-b223-91312bf212e3" /> | <video width="350" alt="withfix" src="https://github.com/user-attachments/assets/a7fa1af6-8927-4fed-a7f2-f8011bdbef8f" /> |
…yboardOpen test failure in May 4th Candidate (#35307) <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details PR #35083 fixed the stale safe area issue when switching from All to Container while the keyboard was open. But that change introduced a regression for the opposite runtime flow. On iOS, when switching from Container to SoftInput while the keyboard is already open, MauiView may not receive a new keyboard show event. Because the keyboard state was cleared during unsubscribe, the view can miss the current keyboard state and fail to apply the expected keyboard-related bottom inset. ### Description of Change <!-- Enter description of the fix in this section --> This change updates iOS keyboard handling in MauiView.cs:312. When switching to SoftInput while the keyboard is already open, it restores keyboard state immediately and refreshes layout, so safe area updates correctly. ### Issues Fixed * ValidateKeyboardRuntime_SwitchContainerToSoftInput_WhileKeyboardOpen
…yboard open (#35083) <!-- 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! ### Root Cause When `SafeAreaEdges` changes from a mode that includes `SoftInput` (e.g., `All`) to one that does not (e.g., `Container`), the view unsubscribes from iOS keyboard notifications. However, the keyboard state (`_isKeyboardShowing`, `_keyboardFrame`) was not cleared. If the keyboard was open during unsubscription and later dismissed, the `OnKeyboardWillHide` event is never received. When switching back to a `SoftInput` mode, the view re-subscribes with stale state, applying incorrect bottom padding equal to the old keyboard height instead of the home indicator inset. ### Description of Change Added state cleanup in `UnsubscribeFromKeyboardNotifications` in `MauiView.cs`. When unsubscribing, if the keyboard was previously visible, the method now clears the keyboard state, invalidates the safe area, and triggers a layout update. This prevents stale keyboard state from persisting across subscribe/unsubscribe cycles while preserving existing behavior in all other scenarios. Also added a test in `Issue28986_ContentPage` that reproduces the exact sequence: switch to All, open keyboard, change to Container, dismiss keyboard, switch back to All, and verify the layout returns to baseline without phantom padding. ### Issues Fixed Fixes #34846 Tested the behaviour in the following platforms - [x] Android - [ ] Windows - [x] iOS - [x] Mac ### Screenshots | Before Issue Fix | After Issue Fix | |------------------|-----------------| | <video width="350" alt="withoutfix" src="https://github.com/user-attachments/assets/48e5afe8-4b6c-4c5a-b223-91312bf212e3" /> | <video width="350" alt="withfix" src="https://github.com/user-attachments/assets/a7fa1af6-8927-4fed-a7f2-f8011bdbef8f" /> |
…yboardOpen test failure in May 4th Candidate (#35307) <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details PR #35083 fixed the stale safe area issue when switching from All to Container while the keyboard was open. But that change introduced a regression for the opposite runtime flow. On iOS, when switching from Container to SoftInput while the keyboard is already open, MauiView may not receive a new keyboard show event. Because the keyboard state was cleared during unsubscribe, the view can miss the current keyboard state and fail to apply the expected keyboard-related bottom inset. ### Description of Change <!-- Enter description of the fix in this section --> This change updates iOS keyboard handling in MauiView.cs:312. When switching to SoftInput while the keyboard is already open, it restores keyboard state immediately and refreshes layout, so safe area updates correctly. ### Issues Fixed * ValidateKeyboardRuntime_SwitchContainerToSoftInput_WhileKeyboardOpen
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
When
SafeAreaEdgeschanges from a mode that includesSoftInput(e.g.,All) to one that does not (e.g.,Container), the view unsubscribes from iOS keyboard notifications. However, the keyboard state (_isKeyboardShowing,_keyboardFrame) was not cleared. If the keyboard was open during unsubscription and later dismissed, theOnKeyboardWillHideevent is never received. When switching back to aSoftInputmode, the view re-subscribes with stale state, applying incorrect bottom padding equal to the old keyboard height instead of the home indicator inset.Description of Change
Added state cleanup in
UnsubscribeFromKeyboardNotificationsinMauiView.cs. When unsubscribing, if the keyboard was previously visible, the method now clears the keyboard state, invalidates the safe area, and triggers a layout update. This prevents stale keyboard state from persisting across subscribe/unsubscribe cycles while preserving existing behavior in all other scenarios.Also added a test in
Issue28986_ContentPagethat reproduces the exact sequence: switch to All, open keyboard, change to Container, dismiss keyboard, switch back to All, and verify the layout returns to baseline without phantom padding.Issues Fixed
Fixes #34846
Tested the behaviour in the following platforms
Screenshots
BeforeFix.33.mov
AfterFix.40.mov