[Android] Fix DatePicker dialog dismisses after the device is rotated#34980
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34980Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34980" |
|
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. |
There was a problem hiding this comment.
Pull request overview
Fixes an Android regression where the DatePicker dialog immediately dismisses after device rotation by adjusting the dialog lifecycle to avoid re-triggering the dialog’s dismiss handling during the rotation re-show flow.
Changes:
- Renamed
ResetDialog()toDestroyDialog()and updated call sites to better reflect behavior (dismiss + null-out). - Simplified
DismissEventwiring by removing per-show/per-hide subscribe/unsubscribe and keeping it scoped to dialog creation/destruction. - Updated
OnMainDisplayInfoChangedto capture the currently selected date, destroy the existing dialog, and recreate/show a new dialog with the captured date.
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/review -b feature/regression-check -p android |
1 similar comment
|
/review -b feature/regression-check -p android |
|
/review -b feature/regression-check -p android |
|
/review -b feature/regression-check -p android |
…ability (dotnet#35133) <!-- 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! > **Depends on dotnet#35136** (pipeline category detection — should merge first) ## What this does Two things: ### 1. UI test category detection in PR review During the PR review workflow, Step 0.5 detects which UI test categories the PR impacts and writes the result to the AI summary comment. This gives reviewers visibility into which UI tests are relevant. **Detection** reuses the 3-tier script from dotnet#35136 (test attributes → source paths → AI reasoning). **AI summary** shows a new 🧪 UI Tests section with detected categories before the gate section. ### 2. Gate reliability fixes Multiple fixes to make the gate (`verify-tests-fail.ps1`) more deterministic: | Fix | Problem it solves | |-----|-------------------| | **Absolute path resolution** | Gate scripts not found on Linux CI agents (`Resolve-Path`, `GetFullPath`) | | **File existence check** | Instant cryptic failure when verify script is missing — now logs clear error | | **3x retry on ENV ERROR** | Emulator timeouts, ADB failures, app crashes — transient issues that pass on retry | | **Strip bad report blocks** | Old verify script produces `Passed: False` with empty counts — stripped instead of shown | | **Gate log in fallback** | When report is missing, shows last 20 lines of gate output instead of just `❌ FAILED / Platform: IOS` | ## Files | File | Changes | |------|---------| | `.github/scripts/Review-PR.ps1` | Step 0.5 category detection + all 5 gate fixes | | `.github/scripts/post-ai-summary-comment.ps1` | Add `uitests` phase to render detected categories | | `.github/pr-review/pr-preflight.md` | Step 7: AI identifies impacted UI test categories | ## Validation — PR reviewer builds (Apr 26) 10 builds against real PRs — all succeeded ✅. Category detection shown in AI summary comment. | PR | Categories Detected | Build | AI Summary | |----|-------------------|-------|------------| | dotnet#35037 (WebView theme) | `ViewBaseTests,WebView` | [13940071](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940071) | [comment](dotnet#35037 (comment)) | | dotnet#35031 (Shell memory leak) | `Shell` | [13940072](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940072) | [comment](dotnet#35031 (comment)) | | dotnet#35020 (XAML Hot Reload) | _(none — XAML only)_ | [13940073](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940073) | ✅ Shows "No UI test categories" | | dotnet#35008 (Shell SearchHandler) | `Shell` | [13940074](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940074) | ✅ | | dotnet#34997 (RadioButton gradient) | `RadioButton,ViewBaseTests` | [13940075](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940075) | ✅ | | dotnet#34980 (DatePicker rotation) | `ViewBaseTests` | [13940076](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940076) | ✅ | | dotnet#34974 (Picker CharacterSpacing) | `ViewBaseTests` | [13940077](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940077) | ✅ | | dotnet#34923 (SwipeView threshold) | `SwipeView,ViewBaseTests` | [13940078](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940078) | ✅ | | dotnet#34907 (CollectionView ScrollTo) | `CollectionView` | [13940079](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940079) | ✅ | | dotnet#34845 (RefreshView binding) | `RefreshView,ViewBaseTests` | [13940080](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940080) | ✅ | --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#34980) <!-- 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 - The DatePicker dialog is dismissed when the device is rotated while the dialog is open. ### Root Cause of the issue - PR [29323](#29323) added DismissEvent subscription in CreateDatePickerDialog() and an OnDialogDismiss → HidePickerDialog() chain that sets IsOpen = false. - On rotation, OnMainDisplayInfoChanged unsubscribes DismissEvent and calls Dismiss(). - ShowPickerDialog() runs synchronously after Dismiss(), enters the else branch, and re-subscribes DismissEvent on the same dialog. - The DismissEvent from step 2 fires after re-subscription, triggering HidePickerDialog() which dismisses the newly-shown dialog. - Before PR [29323](#29323): There was no DismissEvent subscription, no IsOpen property, and no OnDialogDismiss handler — so the dismiss-and-re-show flow in OnMainDisplayInfoChanged worked without interference. ### Description of Change **Dialog lifecycle and event handling improvements:** * Renamed `ResetDialog()` to `DestroyDialog()` and updated all references accordingly to clarify its purpose and ensure consistent dialog destruction and recreation. * Removed unnecessary subscriptions and unsubscriptions to the dialog's `DismissEvent`, simplifying dialog event handling in both `ShowPickerDialog()` and `HidePickerDialog()`. **Display info change handling:** * Improved the logic in `OnMainDisplayInfoChanged` to properly destroy and recreate the dialog with the current date, ensuring the dialog updates correctly when display information changes (such as orientation changes). ### 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 #34973 ### Tested the behaviour in the following platforms - [ ] - Windows - [x] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/c5de443c-371d-4b48-9623-9a17965ba5c3"> | <video src="https://github.com/user-attachments/assets/014ac4eb-7958-488b-bfad-4e3c6ef69796"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
…ability (dotnet#35133) <!-- 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! > **Depends on dotnet#35136** (pipeline category detection — should merge first) ## What this does Two things: ### 1. UI test category detection in PR review During the PR review workflow, Step 0.5 detects which UI test categories the PR impacts and writes the result to the AI summary comment. This gives reviewers visibility into which UI tests are relevant. **Detection** reuses the 3-tier script from dotnet#35136 (test attributes → source paths → AI reasoning). **AI summary** shows a new 🧪 UI Tests section with detected categories before the gate section. ### 2. Gate reliability fixes Multiple fixes to make the gate (`verify-tests-fail.ps1`) more deterministic: | Fix | Problem it solves | |-----|-------------------| | **Absolute path resolution** | Gate scripts not found on Linux CI agents (`Resolve-Path`, `GetFullPath`) | | **File existence check** | Instant cryptic failure when verify script is missing — now logs clear error | | **3x retry on ENV ERROR** | Emulator timeouts, ADB failures, app crashes — transient issues that pass on retry | | **Strip bad report blocks** | Old verify script produces `Passed: False` with empty counts — stripped instead of shown | | **Gate log in fallback** | When report is missing, shows last 20 lines of gate output instead of just `❌ FAILED / Platform: IOS` | ## Files | File | Changes | |------|---------| | `.github/scripts/Review-PR.ps1` | Step 0.5 category detection + all 5 gate fixes | | `.github/scripts/post-ai-summary-comment.ps1` | Add `uitests` phase to render detected categories | | `.github/pr-review/pr-preflight.md` | Step 7: AI identifies impacted UI test categories | ## Validation — PR reviewer builds (Apr 26) 10 builds against real PRs — all succeeded ✅. Category detection shown in AI summary comment. | PR | Categories Detected | Build | AI Summary | |----|-------------------|-------|------------| | dotnet#35037 (WebView theme) | `ViewBaseTests,WebView` | [13940071](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940071) | [comment](dotnet#35037 (comment)) | | dotnet#35031 (Shell memory leak) | `Shell` | [13940072](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940072) | [comment](dotnet#35031 (comment)) | | dotnet#35020 (XAML Hot Reload) | _(none — XAML only)_ | [13940073](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940073) | ✅ Shows "No UI test categories" | | dotnet#35008 (Shell SearchHandler) | `Shell` | [13940074](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940074) | ✅ | | dotnet#34997 (RadioButton gradient) | `RadioButton,ViewBaseTests` | [13940075](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940075) | ✅ | | dotnet#34980 (DatePicker rotation) | `ViewBaseTests` | [13940076](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940076) | ✅ | | dotnet#34974 (Picker CharacterSpacing) | `ViewBaseTests` | [13940077](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940077) | ✅ | | dotnet#34923 (SwipeView threshold) | `SwipeView,ViewBaseTests` | [13940078](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940078) | ✅ | | dotnet#34907 (CollectionView ScrollTo) | `CollectionView` | [13940079](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940079) | ✅ | | dotnet#34845 (RefreshView binding) | `RefreshView,ViewBaseTests` | [13940080](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940080) | ✅ | --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#34980) <!-- 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 - The DatePicker dialog is dismissed when the device is rotated while the dialog is open. ### Root Cause of the issue - PR [29323](#29323) added DismissEvent subscription in CreateDatePickerDialog() and an OnDialogDismiss → HidePickerDialog() chain that sets IsOpen = false. - On rotation, OnMainDisplayInfoChanged unsubscribes DismissEvent and calls Dismiss(). - ShowPickerDialog() runs synchronously after Dismiss(), enters the else branch, and re-subscribes DismissEvent on the same dialog. - The DismissEvent from step 2 fires after re-subscription, triggering HidePickerDialog() which dismisses the newly-shown dialog. - Before PR [29323](#29323): There was no DismissEvent subscription, no IsOpen property, and no OnDialogDismiss handler — so the dismiss-and-re-show flow in OnMainDisplayInfoChanged worked without interference. ### Description of Change **Dialog lifecycle and event handling improvements:** * Renamed `ResetDialog()` to `DestroyDialog()` and updated all references accordingly to clarify its purpose and ensure consistent dialog destruction and recreation. * Removed unnecessary subscriptions and unsubscriptions to the dialog's `DismissEvent`, simplifying dialog event handling in both `ShowPickerDialog()` and `HidePickerDialog()`. **Display info change handling:** * Improved the logic in `OnMainDisplayInfoChanged` to properly destroy and recreate the dialog with the current date, ensuring the dialog updates correctly when display information changes (such as orientation changes). ### 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 #34973 ### Tested the behaviour in the following platforms - [ ] - Windows - [x] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/c5de443c-371d-4b48-9623-9a17965ba5c3"> | <video src="https://github.com/user-attachments/assets/014ac4eb-7958-488b-bfad-4e3c6ef69796"> | <!-- 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
Dialog lifecycle and event handling improvements:
ResetDialog()toDestroyDialog()and updated all references accordingly to clarify its purpose and ensure consistent dialog destruction and recreation.DismissEvent, simplifying dialog event handling in bothShowPickerDialog()andHidePickerDialog().Display info change handling:
OnMainDisplayInfoChangedto properly destroy and recreate the dialog with the current date, ensuring the dialog updates correctly when display information changes (such as orientation changes).Issues Fixed
Fixes #34973
Tested the behaviour in the following platforms
Before_fixdatepicker.mov
After_fixdatepicker.mov