[Android] Fix for TimePicker Dialog doesn't update the layout when rotating the device with dialog open#31910
Conversation
|
Hey there @@HarishwaranVijayakumar! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
jsuarezruiz
left a comment
There was a problem hiding this comment.
Could include a test?
@jsuarezruiz, The fix involves OnMainDisplayInfoChanged, which is only triggered when the device is rotated. However, SetOrientationLandscape does not actually rotate the device, so OnMainDisplayInfoChanged is not triggered, and the test still fails. |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the TimePicker dialog on Android doesn't update its layout when the device is rotated while the dialog is open. The solution adds orientation change detection to automatically dismiss and recreate the dialog with the correct dimensions.
Key Changes
- Added orientation change detection via DeviceDisplay.MainDisplayInfoChanged event subscription
- Implemented dialog recreation when orientation changes to preserve user input
- Added proper lifecycle management for event subscriptions and dialog cleanup
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| TimePickerHandler.Android.cs | Added orientation change handling, dialog lifecycle management, and proper event subscription cleanup |
| PublicAPI.Unshipped.txt | Added new public API entry for the ConnectHandler override method |
|
|
kubaflo
left a comment
There was a problem hiding this comment.
Could you please resolve conflicts?
|
|
kubaflo
left a comment
There was a problem hiding this comment.
Could you please resolve conflicts?
8048897 to
f319e13
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 31910Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 31910" |
Resolved the conflicts |
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
🚦 Gate - Test Before and After Fix📊 Expand Full Gate —
|
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31910 | Subscribe DeviceDisplay.MainDisplayInfoChanged via ViewAttachedToWindow/ViewDetachedFromWindow; unsubscribe DismissEvent before dismiss; use VirtualView?.Time; add base.DisconnectHandler |
⏳ PENDING (Gate skipped) | TimePickerHandler.Android.cs |
Mirrors DatePickerHandler; stale comment still present |
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Mirror DatePickerHandler with ResetDialog() helper; let DismissEvent fire naturally (no defensive unsubscription) |
TimePickerHandler.Android.cs |
Cleaner DRY structure; same pattern as DatePickerHandler | |
| 2 | try-fix (claude-sonnet-4.6) | Lazy orientation check at ShowPickerDialog time using _dialogOrientation field; no event subscriptions |
TimePickerHandler.Android.cs |
Pull-based; requires ShowPickerDialog call after rotation to trigger | |
| 3 | try-fix (gpt-5.3-codex) | PR's fix + stale comment update only | --no-restore; logic sound) |
TimePickerHandler.Android.cs |
Minimal change; comment fix only |
| 4 | try-fix (gpt-5.4) | PR's fix + dedup guard in OnViewAttachedToWindow + stale comment update |
TimePickerHandler.Android.cs |
Hardens against duplicate subscription if view re-attaches | |
| PR | PR #31910 | Subscribe DeviceDisplay.MainDisplayInfoChanged via ViewAttachedToWindow/ViewDetachedFromWindow; defensive DismissEvent unsubscription; use VirtualView?.Time; add base.DisconnectHandler |
⏳ Pending (Gate skipped — no tests) | TimePickerHandler.Android.cs |
Functionally correct; stale comment remains |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| explore agent (all models) | 2 | Yes | DialogFragment approach: wrap TimePickerDialog in Android DialogFragment subclass to leverage native lifecycle management and automatic configuration-change handling. Out of scope for this targeted bugfix. |
| explore agent (all models) | 2 | No new actionable ideas | DialogFragment idea noted but is a major architectural change, not appropriate for this bug fix |
Exhausted: Yes (4 models, cross-pollination complete)
Selected Fix: PR's current fix is functionally correct. Improvements from Attempt 4:
- Add dedup guard in
OnViewAttachedToWindow(unsubscribe before subscribe) - Fix stale comment on
ShowPickerDialog(TimeSpan? time)— remove "Not useful until we have orientation changed events"
All try-fix attempts were Blocked due to lack of physical device for rotation testing. Build succeeds for all approaches.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | 1 file changed, Android-only, no tests; prior AI review found |
| Gate | No tests detected in PR | |
| Try-Fix | ✅ COMPLETE | 4 attempts: all Blocked (build OK, no device for rotation test); 2 improvements identified |
| Report | ✅ COMPLETE |
Summary
PR #31910 fixes a genuine Android bug: TimePickerHandler had no orientation change detection. When the device rotated with the TimePicker dialog open, the dialog stayed at the prior orientation's dimensions. The PR's fix mirrors the identical pattern already in DatePickerHandler.Android.cs — subscribing to DeviceDisplay.MainDisplayInfoChanged via ViewAttachedToWindow/ViewDetachedFromWindow, then dismissing and recreating the dialog on orientation change.
The author has already addressed the major feedback from the prior AI review (removed redundant _currentHour/_currentMinute fields, now uses VirtualView?.Time directly). Two minor issues remain unaddressed. Try-Fix explored 4 independent alternatives; all blocked due to no physical device for rotation testing. Two improvements from Try-Fix were identified (dedup guard + stale comment).
Root Cause
TimePickerHandler.Android.cs did not listen for display/orientation changes. Unlike DatePickerHandler, which already handles this, TimePickerHandler had no mechanism to dismiss and recreate its dialog on rotation. The pre-existing comment in the code even acknowledged this gap: "Not useful until we have orientation changed events."
Fix Quality
Strengths:
- ✅ Correct approach — directly mirrors
DatePickerHandler.Android.cswhich has the identical pattern - ✅ Adds
base.DisconnectHandler(platformView)which was missing — fixes a secondary cleanup bug - ✅ Properly manages event subscription lifecycle (attach/detach + disconnect)
- ✅ Defensive: unsubscribes
DismissEventbefore programmatic dismiss to prevent spuriousVirtualView.IsOpen = falseside-effects during recreation - ✅ Author already addressed prior review feedback: removed redundant
_currentHour/_currentMinutefields, usesVirtualView?.Timedirectly
Issues to Address:
-
Stale comment not updated — Lines 146–148 of
TimePickerHandler.Android.csstill say:// This overload is here so we can pass in the current values from the dialog // on an orientation change (so that orientation changes don't cause the user's date selection progress // to be lost). Not useful until we have orientation changed events.
The phrase "Not useful until we have orientation changed events" is now false — this PR adds exactly those events. Update to remove the stale sentence, for example:
// This overload is here so we can pass in the current values from the dialog // on an orientation change, so the user's time selection progress is preserved.
-
Potential duplicate event subscription —
OnViewAttachedToWindowdoesDeviceDisplay.MainDisplayInfoChanged += OnMainDisplayInfoChangedwithout first unsubscribing. If the Android view is attached more than once without a balanced detach (e.g., view re-parenting), this creates duplicate subscriptions andOnMainDisplayInfoChangedfires multiple times per orientation change. DatePickerHandler has the same issue, but as an improvement, guard against it:void OnViewAttachedToWindow(object? sender = null, View.ViewAttachedToWindowEventArgs? e = null) { DeviceDisplay.MainDisplayInfoChanged -= OnMainDisplayInfoChanged; DeviceDisplay.MainDisplayInfoChanged += OnMainDisplayInfoChanged; }
-
No tests — Gate was skipped because no tests were added. The author explained that
SetOrientationLandscapedoesn't physically triggerMainDisplayInfoChanged, making a direct UI test difficult. Consider adding at least a unit/device test that manually invokesOnMainDisplayInfoChangedvia reflection (or makes the methodinternaland calls it directly) to verify the dialog is dismissed and recreated when showing. This would give confidence the handler wiring works correctly.
Selected Fix: PR's fix with the 2 minor improvements above
- Fix stale comment (1 line change)
- Add dedup guard in
OnViewAttachedToWindow(1 line addition) - All other PR changes are correct as-is
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the AI's suggestions?
@kubaflo, Addressed the AI summary |
Code Review — PR #31910Independent AssessmentWhat this changes: Adds device orientation change handling to Inferred motivation: The TimePicker dialog on Android doesn't resize when the device rotates while the dialog is showing, leaving a portrait-sized dialog in landscape mode (or vice versa). DatePicker already has this exact pattern. Reconciliation with PR NarrativeAuthor claims: TimePicker lacks orientation change detection; fix mirrors DatePickerHandler's approach. Agreement: The code change exactly mirrors the Findings
|
…tating the device with dialog open (#31910) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Android, when opening a TimePicker and then rotating the device, the picker dialog doesn’t seem to redraw itself to match the new screen dimensions. ### Root Cause of the issue - TimePicker lacks orientation change detection entirely. When the device rotates while the dialog is open, TimePicker has no mechanism to dismiss and re-show the dialog with updated layout, unlike DatePicker which detects orientation changes and refreshes the dialog display. ### Description of Change - Added ConnectHandler and related event subscriptions (ViewAttachedToWindow/ViewDetachedFromWindow) to manage display info changes and cleanup when the view is attached or detached. This helps ensure the time picker dialog responds to device orientation changes and releases resources properly. - Implemented OnMainDisplayInfoChanged to dismiss and recreate the time picker dialog with the current time when the device orientation changes, preserving user selection progress. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #31658 ### Reference - [DatePickerHandler](https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/DatePicker/DatePickerHandler.Android.cs) ### Tested the behaviour in the following platforms - [ ] - Windows - [x] - Android - [ ] - Mac - [x] - iOS ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/55091d04-0aa3-4794-aab8-fb5dfc71624e"> | <video src="https://github.com/user-attachments/assets/56c01750-00a1-4c63-8788-ca6c35d7dbd5"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
…tating the device with dialog open (dotnet#31910) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Android, when opening a TimePicker and then rotating the device, the picker dialog doesn’t seem to redraw itself to match the new screen dimensions. ### Root Cause of the issue - TimePicker lacks orientation change detection entirely. When the device rotates while the dialog is open, TimePicker has no mechanism to dismiss and re-show the dialog with updated layout, unlike DatePicker which detects orientation changes and refreshes the dialog display. ### Description of Change - Added ConnectHandler and related event subscriptions (ViewAttachedToWindow/ViewDetachedFromWindow) to manage display info changes and cleanup when the view is attached or detached. This helps ensure the time picker dialog responds to device orientation changes and releases resources properly. - Implemented OnMainDisplayInfoChanged to dismiss and recreate the time picker dialog with the current time when the device orientation changes, preserving user selection progress. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes dotnet#31658 ### Reference - [DatePickerHandler](https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/DatePicker/DatePickerHandler.Android.cs) ### Tested the behaviour in the following platforms - [ ] - Windows - [x] - Android - [ ] - Mac - [x] - iOS ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/55091d04-0aa3-4794-aab8-fb5dfc71624e"> | <video src="https://github.com/user-attachments/assets/56c01750-00a1-4c63-8788-ca6c35d7dbd5"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
…tating the device with dialog open (#31910) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Android, when opening a TimePicker and then rotating the device, the picker dialog doesn’t seem to redraw itself to match the new screen dimensions. ### Root Cause of the issue - TimePicker lacks orientation change detection entirely. When the device rotates while the dialog is open, TimePicker has no mechanism to dismiss and re-show the dialog with updated layout, unlike DatePicker which detects orientation changes and refreshes the dialog display. ### Description of Change - Added ConnectHandler and related event subscriptions (ViewAttachedToWindow/ViewDetachedFromWindow) to manage display info changes and cleanup when the view is attached or detached. This helps ensure the time picker dialog responds to device orientation changes and releases resources properly. - Implemented OnMainDisplayInfoChanged to dismiss and recreate the time picker dialog with the current time when the device orientation changes, preserving user selection progress. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #31658 ### Reference - [DatePickerHandler](https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/DatePicker/DatePickerHandler.Android.cs) ### Tested the behaviour in the following platforms - [ ] - Windows - [x] - Android - [ ] - Mac - [x] - iOS ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/55091d04-0aa3-4794-aab8-fb5dfc71624e"> | <video src="https://github.com/user-attachments/assets/56c01750-00a1-4c63-8788-ca6c35d7dbd5"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
…tating the device with dialog open (#31910) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Android, when opening a TimePicker and then rotating the device, the picker dialog doesn’t seem to redraw itself to match the new screen dimensions. ### Root Cause of the issue - TimePicker lacks orientation change detection entirely. When the device rotates while the dialog is open, TimePicker has no mechanism to dismiss and re-show the dialog with updated layout, unlike DatePicker which detects orientation changes and refreshes the dialog display. ### Description of Change - Added ConnectHandler and related event subscriptions (ViewAttachedToWindow/ViewDetachedFromWindow) to manage display info changes and cleanup when the view is attached or detached. This helps ensure the time picker dialog responds to device orientation changes and releases resources properly. - Implemented OnMainDisplayInfoChanged to dismiss and recreate the time picker dialog with the current time when the device orientation changes, preserving user selection progress. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #31658 ### Reference - [DatePickerHandler](https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/DatePicker/DatePickerHandler.Android.cs) ### Tested the behaviour in the following platforms - [ ] - Windows - [x] - Android - [ ] - Mac - [x] - iOS ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/55091d04-0aa3-4794-aab8-fb5dfc71624e"> | <video src="https://github.com/user-attachments/assets/56c01750-00a1-4c63-8788-ca6c35d7dbd5"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
…tating the device with dialog open (#31910) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Android, when opening a TimePicker and then rotating the device, the picker dialog doesn’t seem to redraw itself to match the new screen dimensions. ### Root Cause of the issue - TimePicker lacks orientation change detection entirely. When the device rotates while the dialog is open, TimePicker has no mechanism to dismiss and re-show the dialog with updated layout, unlike DatePicker which detects orientation changes and refreshes the dialog display. ### Description of Change - Added ConnectHandler and related event subscriptions (ViewAttachedToWindow/ViewDetachedFromWindow) to manage display info changes and cleanup when the view is attached or detached. This helps ensure the time picker dialog responds to device orientation changes and releases resources properly. - Implemented OnMainDisplayInfoChanged to dismiss and recreate the time picker dialog with the current time when the device orientation changes, preserving user selection progress. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #31658 ### Reference - [DatePickerHandler](https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/DatePicker/DatePickerHandler.Android.cs) ### Tested the behaviour in the following platforms - [ ] - Windows - [x] - Android - [ ] - Mac - [x] - iOS ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/55091d04-0aa3-4794-aab8-fb5dfc71624e"> | <video src="https://github.com/user-attachments/assets/56c01750-00a1-4c63-8788-ca6c35d7dbd5"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
…tating the device with dialog open (#31910) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Android, when opening a TimePicker and then rotating the device, the picker dialog doesn’t seem to redraw itself to match the new screen dimensions. ### Root Cause of the issue - TimePicker lacks orientation change detection entirely. When the device rotates while the dialog is open, TimePicker has no mechanism to dismiss and re-show the dialog with updated layout, unlike DatePicker which detects orientation changes and refreshes the dialog display. ### Description of Change - Added ConnectHandler and related event subscriptions (ViewAttachedToWindow/ViewDetachedFromWindow) to manage display info changes and cleanup when the view is attached or detached. This helps ensure the time picker dialog responds to device orientation changes and releases resources properly. - Implemented OnMainDisplayInfoChanged to dismiss and recreate the time picker dialog with the current time when the device orientation changes, preserving user selection progress. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #31658 ### Reference - [DatePickerHandler](https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/DatePicker/DatePickerHandler.Android.cs) ### Tested the behaviour in the following platforms - [ ] - Windows - [x] - Android - [ ] - Mac - [x] - iOS ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/55091d04-0aa3-4794-aab8-fb5dfc71624e"> | <video src="https://github.com/user-attachments/assets/56c01750-00a1-4c63-8788-ca6c35d7dbd5"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
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!
Issue Details
Root Cause of the issue
Description of Change
Added ConnectHandler and related event subscriptions (ViewAttachedToWindow/ViewDetachedFromWindow) to manage display info changes and cleanup when the view is attached or detached. This helps ensure the time picker dialog responds to device orientation changes and releases resources properly.
Implemented OnMainDisplayInfoChanged to dismiss and recreate the time picker dialog with the current time when the device orientation changes, preserving user selection progress.
Issues Fixed
Fixes #31658
Reference
Tested the behaviour in the following platforms
Output
Screen.Recording.2025-10-08.at.4.50.08.pm.mov
Screen.Recording.2025-10-08.at.4.47.47.pm.mov