[Android] Fix ActivityIndicator remaining visible after IsRunning/IsVisible set to false - #36748
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36748Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36748" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey there @@SyedAbdulAzeemSF4852! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Fixes an Android regression where an ActivityIndicator can remain visible after IsRunning/IsVisible are set to false by ensuring deferred ProgressBar.Post(...) updates don’t apply stale “show” state.
Changes:
- Update
ActivityIndicatorExtensions.UpdateIsRunningto re-read the currentIActivityIndicatorstate inside the deferredPost()callback. - Add an Android device test intended to cover the “deferred show resurrects hidden indicator” regression scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Core/src/Platform/Android/ActivityIndicatorExtensions.cs | Recomputes target visibility inside the posted runnable to avoid applying stale visibility. |
| src/Core/tests/DeviceTests/Handlers/ActivityIndicator/ActivityIndicatorHandlerTests.Android.cs | Adds a regression test for deferred visibility updates on Android. |
| var activityIndicator = new ActivityIndicatorStub | ||
| { | ||
| IsRunning = true, | ||
| Visibility = Visibility.Visible | ||
| }; | ||
|
|
||
| var visibility = await InvokeOnMainThreadAsync(async () => | ||
| { | ||
| var handler = CreateHandler(activityIndicator); | ||
| var progressBar = handler.PlatformView; | ||
|
|
||
| activityIndicator.IsRunning = false; | ||
| activityIndicator.Visibility = Visibility.Collapsed; | ||
|
|
||
| ViewStates result = ViewStates.Visible; | ||
| await progressBar.AttachAndRun(async () => | ||
| { | ||
| await Task.Delay(100); | ||
| result = progressBar.Visibility; | ||
| }); | ||
|
|
||
| return result; | ||
| }); |
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@SyedAbdulAzeemSF4852 — new AI review results are available based on this last commit:
3cc166f. To request a fresh review after new comments or commits, comment/review rerun.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ⚠️ INCONCLUSIVE
Platform: ANDROID · Base: main · Merge base: 945fc69a
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
📱 ActivityIndicatorHandlerTests (DeferredShowDoesNotResurrectHiddenIndicator) Category=ActivityIndicator |
🔴 Without fix — 📱 ActivityIndicatorHandlerTests (DeferredShowDoesNotResurrectHiddenIndicator): ⚠️ ENV ERROR · 1595s
(no coded error found; showing last 1200 chars)
.apk: cmd: Can't find service: package
�[41m�[1m�[37mcrit�[39m�[22m�[49m: Install failure: Test command cannot continue
�[40m�[32minfo�[39m�[22m�[49m: Attempting to remove apk 'com.microsoft.maui.core.devicetests'..
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.core.devicetests'
�[41m�[30mfail�[39m�[22m�[49m: Error: Exit code: 20
Std out:
Std err:
cmd: Can't find service: package
�[40m�[32minfo�[39m�[22m�[49m: Attempting to remove apk 'com.microsoft.maui.core.devicetests'..
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.core.devicetests'
�[41m�[30mfail�[39m�[22m�[49m: Error: Exit code: 20
Std out:
Std err:
cmd: Can't find service: package
XHarness exit code: 78 (PACKAGE_INSTALLATION_FAILURE)
Tests completed with exit code: 78
🟢 With fix — 📱 ActivityIndicatorHandlerTests (DeferredShowDoesNotResurrectHiddenIndicator): ⚠️ ENV ERROR · 783s
(no coded error found; showing last 1200 chars)
report-com.microsoft.maui.core.devicetests.zip
�[40m�[32minfo�[39m�[22m�[49m: <<XHARNESS_RESULT_START>>
{
"version": 1,
"machineName": "runnervm6ochl",
"exitCode": 80,
"exitCodeName": "APP_CRASH",
"platform": "android",
"device": "emulator-5554",
"deviceOsVersion": "API 30",
"architecture": "x86_64",
"files": [
{
"name": "adb-logcat-com.microsoft.maui.core.devicetests-default.log",
"type": "logcat"
},
{
"name": "adb-bugreport-com.microsoft.maui.core.devicetests.zip",
"type": "bugreport"
}
]
}
<<XHARNESS_RESULT_END>>
�[40m�[32minfo�[39m�[22m�[49m: Attempting to remove apk 'com.microsoft.maui.core.devicetests'..
�[40m�[37mdbug�[39m�[22m�[49m: Executing command: '/home/vsts/.nuget/packages/microsoft.dotnet.xharness.cli/11.0.0-prerelease.26230.4/runtimes/any/native/adb/linux/adb -s emulator-5554 uninstall com.microsoft.maui.core.devicetests'
�[40m�[32minfo�[39m�[22m�[49m: Successfully uninstalled com.microsoft.maui.core.devicetests
XHarness exit code: 80 (APP_CRASH)
Tests completed with exit code: 80
⚠️ Failure Details
⚠️ ActivityIndicatorHandlerTests (DeferredShowDoesNotResurrectHiddenIndicator) without fix:App crashed during test run (XHarness exit 80 APP_CRASH)⚠️ ActivityIndicatorHandlerTests (DeferredShowDoesNotResurrectHiddenIndicator) with fix:App crashed during test run (XHarness exit 80 APP_CRASH)
📁 Fix files reverted (1 files)
src/Core/src/Platform/Android/ActivityIndicatorExtensions.cs
📱 UI Tests — ViewBaseTests
Detected UI test categories: ViewBaseTests
✅ Deep UI tests — 118 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
ViewBaseTests |
118/119 ✓ | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
🏁 Report — Final Recommendation
Comparative Analysis — PR #36748
Candidates evaluated
| Candidate | Summary | Regression result | Rank |
|---|---|---|---|
pr |
Raw PR fix: re-read live IActivityIndicator visibility/running state inside the deferred Android ProgressBar.Post(...) callback. |
Gate inconclusive from environment/XHarness errors; not a failing fix. | 1 |
pr-plus-reviewer |
Same as pr; expert reviewer returned no actionable inline findings to apply. |
Same as pr. |
1 |
try-fix-1 |
Alternative token/version approach that captures targetVisibility, ignores stale posted callbacks by version, and adds UpdateVisibility invalidation for ActivityIndicator. |
Failed targeted ActivityIndicator regression: Deferred Show Does Not Resurrect A Hidden Indicator reported Actual: Visible and XHarness exited TESTS_FAILED. |
3 |
Analysis
The PR fix is the best candidate because it addresses the root race at the point where stale data was introduced: the posted callback no longer applies a visibility snapshot captured before later IsRunning/IsVisible changes. Re-reading the live state inside the callback preserves the previous deferred-layout behavior needed for the CollectionView header scenario while preventing a queued "show" runnable from resurrecting a hidden indicator.
pr-plus-reviewer is equivalent to pr because the expert reviewer produced no actionable findings. It is therefore tied technically, but pr is selected as the winner because no additional sandbox changes are needed.
try-fix-1 must rank lower because its saved test run failed the targeted regression scenario. Its version-token design can still apply a captured Visible value when the later state change path does not invalidate the same version sequence in time for the queued runnable, and the empirical log shows the indicator remained Visible.
Winner
pr wins. It is the simplest candidate that matches the diagnosed stale-callback failure mode, has no expert-reviewer findings, and does not have the targeted regression failure recorded for try-fix-1.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
…isible set to false (#36748) <!-- 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! ### Issue Details - An ActivityIndicator hidden right after page load (IsRunning/IsVisible = false) can stay visibly stuck on screen even though both properties are false. ### Root Cause - The PR #35358 fixed ActivityIndicators not animating after a CollectionView header height change by deferring the ProgressBar visibility write via Post(...), but it captured the visibility as a snapshot when the runnable was queued — due to which a deferred visibility update could run after a later synchronous hide and re-apply its stale (Visible) value, leaving the indicator stuck visible even though IsRunning/IsVisible were false. ### Description of Change - Updated ActivityIndicatorExtensions.UpdateIsRunning to re-read the current visibility state inside the deferred Post() lambda, preventing a previously captured "show" value from incorrectly making a hidden indicator visible again. ### Issues Fixed Fixes #36735 ### Validated the behaviour in the following platforms - [ ] Windows - [x] Android - [ ] iOS - [ ] Mac ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/b54863bf-c589-4e79-8ae6-9ef87076e70b"> | <video src="https://github.com/user-attachments/assets/0dc0254c-8fa6-4e20-a033-53b7a7da549f"> |
…isible set to false (#36748) <!-- 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! ### Issue Details - An ActivityIndicator hidden right after page load (IsRunning/IsVisible = false) can stay visibly stuck on screen even though both properties are false. ### Root Cause - The PR #35358 fixed ActivityIndicators not animating after a CollectionView header height change by deferring the ProgressBar visibility write via Post(...), but it captured the visibility as a snapshot when the runnable was queued — due to which a deferred visibility update could run after a later synchronous hide and re-apply its stale (Visible) value, leaving the indicator stuck visible even though IsRunning/IsVisible were false. ### Description of Change - Updated ActivityIndicatorExtensions.UpdateIsRunning to re-read the current visibility state inside the deferred Post() lambda, preventing a previously captured "show" value from incorrectly making a hidden indicator visible again. ### Issues Fixed Fixes #36735 ### Validated the behaviour in the following platforms - [ ] Windows - [x] Android - [ ] iOS - [ ] Mac ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/b54863bf-c589-4e79-8ae6-9ef87076e70b"> | <video src="https://github.com/user-attachments/assets/0dc0254c-8fa6-4e20-a033-53b7a7da549f"> |
|
/backport to inflight/candidate |
|
Started backporting to |
…e after IsRunning/IsVisible set to false (#37112) Backport of #36748 to inflight/candidate /cc @kubaflo @SyedAbdulAzeemSF4852 Co-authored-by: SyedAbdulAzeemSF4852 <syedabdulazeem.a@syncfusion.com> Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…isible set to false (#36748) <!-- 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! ### Issue Details - An ActivityIndicator hidden right after page load (IsRunning/IsVisible = false) can stay visibly stuck on screen even though both properties are false. ### Root Cause - The PR #35358 fixed ActivityIndicators not animating after a CollectionView header height change by deferring the ProgressBar visibility write via Post(...), but it captured the visibility as a snapshot when the runnable was queued — due to which a deferred visibility update could run after a later synchronous hide and re-apply its stale (Visible) value, leaving the indicator stuck visible even though IsRunning/IsVisible were false. ### Description of Change - Updated ActivityIndicatorExtensions.UpdateIsRunning to re-read the current visibility state inside the deferred Post() lambda, preventing a previously captured "show" value from incorrectly making a hidden indicator visible again. ### Issues Fixed Fixes #36735 ### Validated the behaviour in the following platforms - [ ] Windows - [x] Android - [ ] iOS - [ ] Mac ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/b54863bf-c589-4e79-8ae6-9ef87076e70b"> | <video src="https://github.com/user-attachments/assets/0dc0254c-8fa6-4e20-a033-53b7a7da549f"> |
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
Description of Change
Issues Fixed
Fixes #36735
Validated the behaviour in the following platforms
Output
Before_Fix.mov
After_Fix.mov