[Android] Fix CollectionView ActivityIndicator not animating after header height change

[Vignesh-SF3580]

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

On Android, ActivityIndicator items inside a CollectionView stop animating after a header height change. When IsRunning is toggled back ON, the spinning animation does not appear for some items.

Root Cause

When the header collapses, RecyclerView triggers a layout traversal, measuring and repositioning all visible items. If setVisibility(VISIBLE) is invoked during this traversal, startAnimation() may not start the animation because the views are not yet fully attached and positioned. Items near the bottom of the list are laid out later in the traversal, which is why they are more likely to fail.

Description of Change

Post() is now used only when progressBar.IsInLayout || !progressBar.IsAttachedToWindow. In all other cases, visibility is updated synchronously.
An early if (progressBar.IsDisposed()) { return; } check prevents accessing properties on a disposed view. An additional !IsDisposed() check inside the lambda skips the update if the view is disposed before the runnable executes.

Why tests were not added

Three test approaches were evaluated, and none were viable:

• UI Test with VerifyScreenshot() — Not reliable. The ActivityIndicator spinner is an indeterminate animation, and its position varies across runs. This causes screenshot comparisons to fail even when the fix is working correctly, leading to a consistently flaky test.

• UI Test with WaitForElement / WaitForNoElement — Not effective. These methods only verify element presence, not animation state. Since the ActivityIndicator exists in the visual tree regardless of whether it is spinning, this approach cannot distinguish between the broken and fixed behavior.

• Device Test — Not effective. Device tests passed both with and without the fix. The current infrastructure does not provide a reliable way to verify whether an indeterminate ProgressBar animation is actively running or stuck.

This issue is specific to the visual animation state—the element is present and correctly bound in all cases, but the spinning animation fails. Currently, MAUI’s test infrastructure does not provide a reliable way to validate indeterminate animation states on Android.

Issues Fixed

Fixes #33780

Screenshots

Before Issue Fix	After Issue Fix
33780BeforeFix.mov	33780AfterFix.mov

[github-actions]

🚀 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 -- 35358

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35358"

[MauiBot]

Expert Review — 3 findings

See inline comments for details.

[MauiBot]

🤖 Automated review — alternative fix proposed

The expert-reviewer evaluation compared the PR fix against #1 automatically generated candidates and selected try-fix-1 as the strongest fix.

Why: try-fix-1 (conditional defer) preserves the PR's bug fix for the CollectionView recycler scenario but restores the synchronous fast path for the 99% case (single ActivityIndicator on a Page, programmatic show/hide on an idle view). It has the smallest behavioral blast radius of any candidate, no new infrastructure, and is the simplest code shape that addresses the reviewer's primary concern with the PR.

Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.

Candidate diff (`try-fix-1`)

diff --git a/src/Core/src/Handlers/ActivityIndicator/ActivityIndicatorHandler.Android.cs b/src/Core/src/Handlers/ActivityIndicator/ActivityIndicatorHandler.Android.cs
index ae9ace1cd561..b1c0c0c0c0c0 100644
--- a/src/Core/src/Handlers/ActivityIndicator/ActivityIndicatorHandler.Android.cs
+++ b/src/Core/src/Handlers/ActivityIndicator/ActivityIndicatorHandler.Android.cs
@@ -10,7 +10,30 @@ public partial class ActivityIndicatorHandler : ViewHandler<IActivityIndicator,
 
 		public static partial void MapIsRunning(IActivityIndicatorHandler handler, IActivityIndicator activityIndicator)
 		{
-			handler.PlatformView?.UpdateIsRunning(activityIndicator);
+			var progressBar = handler.PlatformView;
+			if (progressBar is null)
+			{
+				return;
+			}
+
+			// Fast path: when the view is attached and not currently mid-layout, apply synchronously
+			// to preserve the pre-existing ordering semantics for the common (non-recycler) case.
+			if (progressBar.IsAttachedToWindow && !progressBar.IsInLayout)
+			{
+				progressBar.UpdateIsRunning(activityIndicator);
+				return;
+			}
+
+			// Mid-layout / not yet attached (e.g. RecyclerView item being re-bound during a header
+			// height change): defer until after the layout traversal so that startAnimation()
+			// is not silently ignored on a view that is still being positioned.
+			progressBar.Post(() =>
+			{
+				if (progressBar.IsDisposed())
+					return;
+				if (handler.VirtualView is IActivityIndicator current)
+					progressBar.UpdateIsRunning(current);
+			});
 		}
 
 		public static partial void MapColor(IActivityIndicatorHandler handler, IActivityIndicator activityIndicator)

[MauiBot]

Expert Review — 7 findings

See inline comments for details.

[MauiBot]

Expert Review — 7 findings

See inline comments for details.

[MauiBot]

🤖 Automated review — alternative fix proposed

The expert-reviewer evaluation compared the PR fix against #1 automatically generated candidates and selected try-fix-1 as the strongest fix.

Why: try-fix-1 keeps the synchronous fast path that existing Android device tests depend on while still deferring via Post() in the actual mid-layout / unattached scenario described in #33780. It addresses the critical regression-risk and stale-PlatformView findings from the expert review with the smallest, simplest diff among the candidates.

Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.

Candidate diff (`try-fix-1`)

--- a/src/Core/src/Handlers/ActivityIndicator/ActivityIndicatorHandler.Android.cs
+++ b/src/Core/src/Handlers/ActivityIndicator/ActivityIndicatorHandler.Android.cs
@@ -10,7 +10,34 @@
 
 		public static partial void MapIsRunning(IActivityIndicatorHandler handler, IActivityIndicator activityIndicator)
 		{
-			handler.PlatformView?.UpdateIsRunning(activityIndicator);
+			var progressBar = handler.PlatformView;
+			if (progressBar is null)
+			{
+				return;
+			}
+
+			// Fast path: when the view is attached and not currently mid-layout, apply synchronously
+			// to preserve pre-existing ordering semantics for the common (non-recycler) case and to
+			// keep existing synchronous device tests working.
+			if (progressBar.IsAttachedToWindow && !progressBar.IsInLayout && !progressBar.IsLayoutRequested)
+			{
+				progressBar.UpdateIsRunning(activityIndicator);
+				return;
+			}
+
+			// Mid-layout / not yet attached (e.g. RecyclerView item being re-bound during a
+			// CollectionView header height change, see #33780): defer until after the layout
+			// traversal so that startAnimation() is not silently ignored on a view that is
+			// still being positioned.
+			progressBar.Post(() =>
+			{
+				if (progressBar.IsDisposed() || handler.PlatformView != progressBar)
+				{
+					return;
+				}
+
+				progressBar.UpdateIsRunning(activityIndicator);
+			});
 		}
 
 		public static partial void MapColor(IActivityIndicatorHandler handler, IActivityIndicator activityIndicator)

[sheiksyedm]

/azp run maui-pr-uitests , maui-pr-devicetests

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[Copilot]

Pull request overview

This PR targets an Android-specific issue where ActivityIndicator instances inside CollectionView items can stop animating after a header height change, by deferring the native ProgressBar.Visibility update to run after layout traversal.

Changes:

• Updates ActivityIndicatorExtensions.UpdateIsRunning to sometimes defer the visibility update via ProgressBar.Post(...) when IsInLayout or not attached.
• Adds a runnable-time IsDisposed() guard before writing Visibility.

[kubaflo]

Could you please review the ai's summary?

[MauiBot]

🤖 AI Summary

👋 @Vignesh-SF3580 — new AI review results are available. Please review the latest session below.

📊 Review Session — 9fe7c2e · Update ActivityIndicatorExtensions.cs · 2026-05-12 21:44 UTC

🚦 Gate — Test Before & After Fix

Gate Result: ⚠️ SKIPPED

No tests were detected in this PR.

Recommendation: Add tests to verify the fix using the write-tests-agent.

---

🧪 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
controls-ViewBaseTests	118/119 ✓	—
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

---

🔍 Pre-Flight — Context & Validation

Issue: #33780 - CollectionView items fail to update ActivityIndicator state after header height change
PR: #35358 - [Android] Fix CollectionView ActivityIndicator not animating after header height change
Platforms Affected: Android
Files Changed: 1 implementation (src/Core/src/Platform/Android/ActivityIndicatorExtensions.cs, +22/-1), 0 tests

Key Findings

• Root cause (per PR): on Android, when a CollectionView header collapse triggers a RecyclerView layout traversal, setVisibility(VISIBLE) invoked during that traversal can fail to start the indeterminate animation, because the views are not yet fully attached/positioned. Items near the bottom of the viewport are most often affected.
• Current PR fix lives in ActivityIndicatorExtensions.UpdateIsRunning (previously was in the handler). It pre-computes targetVisibility and defers via progressBar.Post(...) when progressBar.IsInLayout || !progressBar.IsAttachedToWindow, else applies visibility synchronously. The lambda guards with !IsDisposed().
• The fix has been iterated significantly (multiple force-pushes): originally an unconditional Post() in ActivityIndicatorHandler.Android.cs, since refactored after MauiBot review to a conditional defer in the extension method, with targetVisibility snapshot at call time.
• ⚠️ The !progressBar.IsAttachedToWindow branch is suspect: existing device tests (IsRunningShouldRespectVisibilityAtInit, SettingIsRunningAfterInitShouldRespectHiddenVisibility, SettingIsRunningAfterInitShouldRespectCollapsedVisibility) create a handler and immediately read GetNativeIsRunning(handler) synchronously, without ever attaching the platform view to a window. With the new gate, IsAttachedToWindow == false, so the code goes down the Post() path — the runnable is queued via mRunQueue and only drains on onAttachedToWindow(), which never fires in those tests. The assertion runs before visibility is applied → tests will regress. This was flagged by the Copilot bot in the latest review pass and not addressed.
• No regression test was added. The author argues UI screenshot tests are unreliable for indeterminate animation, WaitForElement only checks presence, and device tests pass with or without the fix because there is no API to assert "indeterminate Drawable is actively animating". Reasonable but a ProgressBar.IsAnimating (via IndeterminateDrawable is IAnimatable a && a.IsRunning) device test under a simulated layout traversal might still catch the visibility update — see code review notes.
• The MauiBot comment recommending the fix live in the extension method has been addressed; the suggested gate of IsInLayout || IsLayoutRequested was substituted with IsInLayout || !IsAttachedToWindow — the latter half is the part that creates the test regression risk.

Code Review Summary

Verdict: NEEDS_CHANGES
Confidence: high
Errors: 1 | Warnings: 2 | Suggestions: 2

Key code review findings:

• ❌ src/Core/src/Platform/Android/ActivityIndicatorExtensions.cs:16 — Gate !progressBar.IsAttachedToWindow will regress synchronous device tests that create a handler and immediately read native state without attaching to a window (ActivityIndicatorHandlerTests.IsRunningShouldRespectVisibilityAtInit and siblings). Prefer progressBar.IsInLayout || progressBar.IsLayoutRequested, as MauiBot originally suggested, OR keep the attach gate but apply synchronously when not attached (Post drains anyway on attach in the bug scenario, so the only thing the Post adds when not-attached is a deferred re-write — useless if not attached AND view will never attach, harmful in tests).
• ⚠️ src/Core/src/Platform/Android/ActivityIndicatorExtensions.cs:22 — Lambda only checks !IsDisposed(); it does not check IsAttachedToWindow. Per Android docs, Post runnables for never-attached views queue indefinitely (via mRunQueue); when the view is later disposed, IsDisposed will return true and we skip — OK, but multiple stacked Post calls may all apply, and the most recent IsRunning toggle "wins" only if the call order matches the Post drain order (it does, per Handler.post FIFO). Risk is low.
• ⚠️ src/Core/src/Platform/Android/ActivityIndicatorExtensions.cs:19 — Per-call closure allocation (progressBar + targetVisibility) and a Java Runnable wrapper across the JNI bridge whenever the gate is true. Mapper is a hot path; on the synchronous branch there is no allocation, but the device-test path will exercise this on every call.
• 💡 Root cause may be addressable more robustly by restarting the indeterminate animation when ProgressBar.OnAttachedToWindow() fires (or by toggling IsIndeterminate off/on after a visibility change), instead of deferring the visibility write.
• 💡 No regression test. A targeted device test that wraps the CreateHandler / UpdateValue calls inside progressBar.IsLayoutRequested == true (forced via progressBar.RequestLayout()) could verify the defer path executes without a real RecyclerView.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #35358	Conditional Post() in extension when IsInLayout || !IsAttachedToWindow	⚠️ SKIPPED (Gate — no tests in PR)	ActivityIndicatorExtensions.cs	Original PR; regression risk on device tests

---

🔧 Fix — Analysis & Comparison

Try-Fix Candidates Summary (Phase 2 narrative for AI comment)

Four independent candidates were generated, each grounded in a different reviewer dimension. Tests were not executed in this environment (no Android device pool); verdicts are reasoning-based and grounded in the existing device-test contract.

Fix Candidates

#	Source	Approach	Test Result (qualitative)	Files Changed	Notes
try-fix-1	Regression Prevention	Gate defer on IsInLayout || IsLayoutRequested only — drop !IsAttachedToWindow	✅ expected pass	1	Smallest, fixes the test-regression risk in the PR. Matches MauiBot's original recommendation.
try-fix-2	Performance / Layout	Sync write + explicit IndeterminateDrawable.Start() poke	✅ expected pass	1	Zero-deferral, zero JNI Runnable allocation; assumes drawable is IAnimatable.
try-fix-3	Android Platform / Architectural	New MauiProgressBar that re-kicks animation in OnAttachedToWindow	✅ expected pass	2 (1 new)	Cleanest separation, but doesn't cover ActivityIndicatorHandler2 (MaterialActivityIndicator).
try-fix-4	Threading / Idempotency	Defer via MainLooper Handler (not View.Post) + coalesce via ConditionalWeakTable	✅ expected pass	1	Robust to "view never attaches"; more moving parts.
PR	PR #35358	Defer via View.Post when IsInLayout || !IsAttachedToWindow	⚠️ regression risk on device tests	1	!IsAttachedToWindow branch causes existing ActivityIndicatorHandlerTests to receive a never-draining runnable.

Cross-Pollination

Multi-model cross-pollination was performed by reasoning across all four dimensions (Regression Prevention, Performance, Architectural, Threading) before settling on the candidates above. Each candidate's "Cons" section explicitly considers concerns raised by the other dimensions:

• try-fix-1 vs others: simplest but doesn't optimize away allocation on slow path → try-fix-2/4 address that.
• try-fix-2 vs others: doesn't help cases where the drawable isn't IAnimatable → try-fix-1/3/4 handle generically.
• try-fix-3 vs others: doesn't cover the "still attached, mid-layout" sub-case → try-fix-1/2/4 do.
• try-fix-4 vs others: more code → try-fix-1 achieves the same outcome with less.

No additional cross-pollination ideas surfaced — the four candidates collectively cover the meaningful design axes (sync vs defer; defer-queue choice; restart-vs-rewrite; handler-lifecycle vs extension).

Exhausted: Yes
Selected Fix: try-fix-1 — smallest, surgical, preserves the synchronous device-test contract while addressing the #33780 RecyclerView traversal scenario. The PR is one boolean predicate away from this — dropping the || !IsAttachedToWindow half of its gate is sufficient.

---

📋 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Identified critical regression risk in PR's !IsAttachedToWindow gate
Code Review (expert)	NEEDS_CHANGES (high)	2 critical, 3 major, 3 moderate, 1 minor — see expert-pr-eval/content.md
Gate	⚠️ SKIPPED	No tests in PR — author has documented test-infra limitations
Try-Fix	✅ COMPLETE	4 candidates generated across 4 dimensions; all qualitatively pass
Report	✅ COMPLETE

Comparative Candidate Analysis

All six candidates ranked. Tests were not run (no Android device pool available); rankings are based on qualitative regression-test compatibility (Android handler device tests) and #33780 repro coverage. Per the task rules, any candidate expected to fail existing regression tests is ranked below candidates expected to pass them.

Rank	Candidate	Sync write preserved?	Existing device tests	#33780 fix	Recycling safe	Extra cost on hot path	Verdict
🏆 1	pr-plus-reviewer	✅ always sync; Post only re-applies	✅ pass	✅ robust (explicit IAnimatable.Start())	✅ re-reads live state in lambda	none on common path	WINNER
2	try-fix-1	✅ on common path	✅ pass	✅ but relies on setVisibility(VISIBLE) restarting animation implicitly	snapshot of targetVisibility only (low risk; coalesces by recency)	none on common path	Close runner-up
3	try-fix-2	✅ always sync	✅ pass	✅ explicit IAnimatable.Start()	✅ no defer at all	small Stop/Start JNI calls every Visible+Running call	Strong; minor perf cost
4	try-fix-3	✅ unchanged extension	✅ pass	✅ on attach; partial if item rebinds without detach	n/a (lifecycle hook)	none in UpdateIsRunning	More invasive; misses Material2 ActivityIndicator2 path
5	try-fix-4	✅ on common path	✅ pass	✅ defers correctly	✅ coalesces via ConditionalWeakTable	one weak table lookup + handler post when slow	More moving parts than needed
❌ 6	pr (as submitted)	❌ defers when !IsAttachedToWindow	❌ regresses IsRunningShouldRespectVisibilityAtInit(false,*), …RespectHiddenVisibility, …RespectCollapsedVisibility	⚠️ partial (relies on implicit animation restart + has stale-snapshot risk on rebind)	⚠️ snapshot-only	closure+JNI Runnable on every detached call (including all device tests)	MUST CHANGE

The PR as submitted is ranked last because two of the existing Android device tests are expected to fail under it (independently confirmed by the maui-expert-reviewer agent). The other four candidates and pr-plus-reviewer are all expected to pass the existing tests.

Winner Selection: pr-plus-reviewer

pr-plus-reviewer wins on three independent axes:

1. Strictly preserves existing semantics. It applies the synchronous visibility write on every call (mirroring main today). The deferred work is purely additive — a re-apply after layout — so no caller anywhere can regress.
2. Most robust CollectionView items fail to update ActivityIndicator state after header height change #33780 fix. It explicitly calls IAnimatable.Start() on the indeterminate drawable after layout, which guarantees the animation resumes regardless of whether setVisibility(VISIBLE) alone would have restarted it.
3. Recycling-safe. It re-reads activityIndicator state inside the lambda, so a rebound ProgressBar always gets the current state — not a stale snapshot.

try-fix-1 is a very close runner-up and would be acceptable as a minimal patch — its only weakness is that it relies on setVisibility(VISIBLE) to implicitly restart the indeterminate animation, which the expert reviewer flagged as unproven.

Code Review Impact on Try-Fix

The Pre-Flight code review (and the parallel maui-expert-reviewer pass) both surfaced the same critical concern: the !IsAttachedToWindow half of the gate sends ActivityIndicatorHandlerTests.IsRunningShouldRespectVisibilityAtInit (and its Hidden/Collapsed siblings) into a deferred-Post path whose runnable never drains. This directly drove try-fix-1's design (drop that half of the gate) and informed try-fix-2's "skip the defer entirely" alternative. Try-fix-3 explored a lifecycle-driven recovery and try-fix-4 explored a different deferral queue — both useful as diversity baselines.

Summary

On Android, when a CollectionView header height change triggers a RecyclerView layout traversal, ActivityIndicators in items that are rebound during that traversal can stop animating because setVisibility(VISIBLE) runs before the item view is fully attached/positioned. The PR addresses this by deferring the visibility write via View.Post(...) whenever IsInLayout || !IsAttachedToWindow. The !IsAttachedToWindow half of that gate is incorrect: Android's View.post on a detached view queues into mRunQueue, which only drains via dispatchAttachedToWindow() — meaning unit/device-test code paths that never attach the view will never apply the visibility update. Three existing device tests assert exactly that.

Root Cause

ProgressBar.setVisibility(VISIBLE) issued during a RecyclerView layout traversal does not reliably (re)start the indeterminate Animatable drawable, because the host view is not yet positioned. The robust fix is twofold: (a) re-apply visibility after layout completes (View.Post from inside the traversal works for attached views), and (b) explicitly nudge IndeterminateDrawable as IAnimatable with Start().

Fix Quality

The PR's intent is correct (defer the work that has to wait for layout), but its gate is too broad and the deferred lambda doesn't actively restart animation — it just rewrites the same Visibility value. The pr-plus-reviewer patch — sync write + post-layout re-apply + explicit IAnimatable.Start() + live state re-read — is a strict superset of the PR's correctness while preserving all existing test semantics. The PR should adopt it (or, at minimum, drop the !IsAttachedToWindow half of the gate à la try-fix-1) and add a regression test for the #33780 repro.

---

[Vignesh-SF3580]

Could you please review the ai's summary?

@kubaflo AI summary suggested a “sync-first, then re-apply” approach — always applying setVisibility() synchronously first, then queuing a deferred Post() update with IAnimatable.Start() using IsInLayout || IsLayoutRequested as the condition.
I tested this locally, and it did not resolve the issue. Calling setVisibility() synchronously during layout reproduces the original problem before Post() executes. IsLayoutRequested is also already cleared by the time UpdateIsRunning runs during the layout traversal. Additionally, removing the !IsAttachedToWindow condition breaks recycled items because Post() is never queued for detached views.