CarouselView: Fix cascading PositionChanged/CurrentItemChanged events on collection update

[praveenkumarkarunanithi]

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

Windows
Position not updating on item add: CarouselView stayed at the old position after an item was added, leaving current/previous positions unsynced.

Cascading events: With ItemsUpdatingScrollMode.KeepItemsInView, programmatic smooth scrolls triggered multiple ViewChanged calls, causing PositionChanged to fire repeatedly with intermediate values.

Android
Programmatic smooth scrolls produced the same cascading PositionChanged events as on Windows.

Description of Change

Windows
Position Update: On item add, ItemsView.Position is explicitly set based on ItemsUpdatingScrollMode, keeping current and previous positions in sync.

Prevent Cascading Events: Added _isInternalPositionUpdate. For collection changes, animations are disabled (animate = false) so scrolling jumps directly, firing PositionChanged only once.

Android
Reused the _isInternalPositionUpdate logic. Disabled animations during collection changes, ensuring a single clean position update without duplicate events.

Issues Fixed

Fixes #29529

Tested the behaviour in the following platforms

• Android
• Windows
• iOS
• Mac

Screenshots

Before Issue Fix	After Issue Fix

[dotnet-policy-service]

Hey there @@praveenkumarkarunanithi! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[Copilot]

Pull Request Overview

This PR fixes issues with CarouselView's event handling system where CurrentItemChangedEventArgs and PositionChangedEventArgs were not working properly on Windows and Android platforms. The fix prevents cascading position change events during collection updates by introducing an internal flag to disable animations during programmatic scrolls.

Key Changes:

• Added _isInternalPositionUpdate flag to prevent cascading events during collection changes
• Fixed position synchronization issues on Windows when items are added
• Disabled animations during collection updates to ensure single, clean position updates

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
Issue29529.cs (TestCases.Shared.Tests)	Adds automated UI test to verify position and item change events fire correctly after item insertion
Issue29529.cs (TestCases.HostApp)	Creates test UI page with CarouselView demonstrating the issue and event tracking
CarouselViewHandler.Windows.cs	Implements Windows-specific fix with internal flag and position synchronization logic
MauiCarouselRecyclerView.cs	Implements Android-specific fix with internal flag and centralized scroll method

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[praveenkumarkarunanithi]

🤖 AI Summary

📊 Expand Full Review
🔍 Pre-Flight — Context & Validation
📝 Review Session — updated fix. · b650f8e
Issue: #29529 - [Windows] CurrentItemChangedEventArgs and PositionChangedEventArgs Not Working Properly in CarouselView Platforms Affected: Windows (original issue), Android (PR extends fix) Files Changed: 2 implementation files, 2 test files

Issue Summary

On Windows (and Android), CarouselView's CurrentItemChangedEventArgs and PositionChangedEventArgs do not behave correctly:

1. PreviousItem/CurrentItem don't update correctly when items are dynamically added at index 0
2. PreviousPosition/CurrentPosition don't reflect correct positions after dynamic additions
3. With ItemsUpdatingScrollMode.KeepItemsInView, programmatic smooth scrolls triggered multiple ViewChanged calls, causing PositionChanged to fire repeatedly with intermediate values (cascading events)

Files Changed

Fix files:

• src/Controls/src/Core/Handlers/Items/Android/MauiCarouselRecyclerView.cs (+20/-3)
• src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs (+47/-23)

Test files:

• src/Controls/tests/TestCases.HostApp/Issues/Issue29529.cs (+113, new file)
• src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29529.cs (+28, new file)

PR's Fix Approach

• Added _isInternalPositionUpdate bool flag on both Android and Windows handlers
• Flag is set true during CollectionItemsSourceChanged/OnCollectionItemsSourceChanged
• ScrollToItemPosition (Android) / UpdateCurrentItem (Windows) check flag to disable animation
• Windows: Also added explicit ItemsView.Position = currentItemPosition on Add to sync position
• Windows: Wrapped collection change logic in try/finally for safe flag reset
• Android: Added bounds check in new ScrollToItemPosition helper method

Reviewer Feedback

File:Line Reviewer Comment Status
Issue29529.cs HostApp:103 copilot-pull-request-reviewer Insert(6,...) should be Insert(0,...) to match issue (adds at end, not front) ⚠️ UNRESOLVED
Issue29529.cs HostApp:57 copilot-pull-request-reviewer Missing ": " in positionLabel initial text ⚠️ UNRESOLVED
Issue29529.cs HostApp:65 copilot-pull-request-reviewer Missing ": " in itemLabel initial text ⚠️ UNRESOLVED
CarouselViewHandler.Windows.cs jsuarezruiz Use try/finally for flag reset ✅ ADDRESSED
MauiCarouselRecyclerView.cs :570 jsuarezruiz Add bounds check on scroll ✅ ADDRESSED
Note on Insert(6) vs Insert(0): The copilot reviewer suggested Insert(0,...) because the issue is about "adding at index 0". However, the button text in the test says "Insert item at index 6". The Insert(6,...) adds to the end of a 6-item list, triggering the KeepItemsInView scroll-to-0 path. This tests the cascading events scenario. The original issue's "index 0" scenario is NOT directly tested.

Edge Cases to Check

• Does PositionChanged fire exactly once after insert with KeepItemsInView on Android?
• Does the test assertion match what actually happens (position goes to 0, events fire once)?
• Are text formatting bugs in initial labels inconsequential to test assertions?

Fix Candidates

Source Approach Test Result Files Changed Notes

PR PR #31275 _isInternalPositionUpdate flag + disable animation during collection changes ⏳ PENDING (Gate) MauiCarouselRecyclerView.cs, CarouselViewHandler.Windows.cs Original PR
🚦 Gate — Test Verification
📝 Review Session — updated fix. · b650f8e
Result: ❌ GATE FAILED / ENVIRONMENT BLOCKED Platform: android Mode: Full Verification Attempted

Test Run Results

• Tests WITHOUT fix: PASSED (unexpected - should FAIL)
• Tests WITH fix: PASSED (expected)

Environment Analysis

The test output shows only build output (no device/emulator logs). Both runs completed with Build succeeded but no actual UI test execution on a device. This indicates:

• No Android emulator was running during verification
• dotnet test completed with only build phase, no deployment/execution phase
• The "Passed: True" results reflect build success, not actual test execution

Root Cause of Gate Failure

The test verification could not determine if tests catch the bug because no Android device/emulator was available to deploy and run the UI tests.

Additional Analysis (from code review)

The test in Issue29529.cs (HostApp) uses Insert(6, "Item 0") which inserts at the END of a 6-item list. The original issue reports the bug when inserting at INDEX 0 (front of list). The test may not reproduce the exact scenario from the issue report. However, the test does verify the KeepItemsInView cascading events scenario.

Unresolved text formatting bugs in test (noted by copilot-pull-request-reviewer):

• "Current Position{carouselView.Position}" → missing ": " (doesn't affect assertions)
• "Current Item{carouselView.CurrentItem}" → missing ": " (doesn't affect assertions)

🔧 Fix — Analysis & Comparison
📝 Review Session — updated fix. · b650f8e

Fix Candidates

Source Approach Test Result Files Changed Notes

PR PR #31275 _isInternalPositionUpdate flag + disable animation during collection changes + explicit position sync (Windows) ✅ PASS (Gate/Build) MauiCarouselRecyclerView.cs, CarouselViewHandler.Windows.cs Original PR

Try-Fix Status

SKIPPED - No Android device/emulator available for test execution. All try-fix attempts require a running device to validate test results. The Gate phase confirmed no device is available (builds succeed but no test execution occurs).

Exhausted: No (skipped due to environment blocker) Selected Fix: PR's fix - Only candidate; code analysis confirms the approach is generally correct

Code Analysis (In Lieu of try-fix)

The PR's approach of using _isInternalPositionUpdate flag to disable animation during collection changes is a valid pattern. Key observations:

1. Windows: Uses try/finally for flag reset (✅ Good - addressed jsuarezruiz feedback)

2. Android: Does NOT use try/finally - flag reset paths are:

   • Early return when removingAnyPrevious = true (resets inline)
   • Inside async dispatched lambda (may not always execute)
   • Risk: If an exception occurs in the non-early-return path, flag stays true

3. Test insert index: Uses Insert(6,...) (adds at end) vs original issue's "insert at index 0" - tests different scenario

📋 Report — Final Recommendation
📝 Review Session — updated fix. · b650f8e

⚠️ Final Recommendation: REQUEST CHANGES

Summary

PR #31275 fixes CarouselView's PositionChanged and CurrentItemChanged cascading events on Windows and Android when items are added during KeepItemsInView mode. The fix approach is sound but has a few issues that should be addressed before merging.

Gate Status

Gate phase was environment-blocked (no Android device available for UI test execution). Tests built successfully but could not be deployed/executed. The fix was assessed via code analysis.

Root Cause

On both Windows and Android, when items are added to a CarouselView with ItemsUpdatingScrollMode.KeepItemsInView, the scroll-to-position call uses animation. The animated scroll generates multiple ViewChanged/OnScrollViewChanged events, each firing PositionChanged with intermediate values — causing cascading events.

On Windows specifically, the position (ItemsView.Position) was not explicitly updated when items were added, leaving CurrentItem and position out of sync.

Fix Quality Assessment

What's Good:

• The _isInternalPositionUpdate flag pattern is clean and targeted
• Windows: uses try/finally for safe flag reset (good defensive programming)
• Windows: adds explicit ItemsView.Position = currentItemPosition when adding items — correctly fixes the position sync bug
• Android: new ScrollToItemPosition helper adds bounds check (good defensive programming)
• Resolves both reviewer requests from jsuarezruiz

Issues Found:

1. Android: Missing try/finally (Medium Risk)

The Windows handler was updated to use try/finally for _isInternalPositionUpdate after reviewer feedback. The Android handler was NOT updated similarly. If an exception occurs in the Dispatcher.Dispatch lambda (lines 295–316 of MauiCarouselRecyclerView.cs), _isInternalPositionUpdate will be stuck as true, permanently disabling animation for subsequent scrolls.

Suggested fix for Android:

_isInternalPositionUpdate = true;
try
{
    // ... existing collection change logic ...
    Carousel.Handler.MauiContext.GetDispatcher().Dispatch(() =>
    {
        try
        {
            // ... existing dispatch logic ...
        }
        finally
        {
            _isInternalPositionUpdate = false;
        }
    });
}
catch
{
    _isInternalPositionUpdate = false;
    throw;
}

Or more simply, handle it within the existing removingAnyPrevious early-return path (which correctly resets, but the rest of the code relies on the async reset).

2. Test Uses Wrong Insert Index (Medium)

The test (Issue29529.cs HostApp) inserts at index 6 (Insert(6, "Item 0")) of a 6-item collection — this adds to the end of the list. But issue #29529 explicitly states the bug occurs when "items are dynamically added at index 0" (front of list). This means the test does NOT reproduce the primary scenario from the issue.

The copilot-pull-request-reviewer raised this: Insert(6, "Item 0") should be Insert(0, "Item 0").

The button text says "Insert item at index 6" — both the button text and the insert index should be changed to 0 to match the original issue.

3. Minor: Text Formatting in Initial Label Text

• Text = $"Current Position{carouselView.Position}" → missing ": "
• Text = $"Current Item{carouselView.CurrentItem}" → missing ": "

These don't affect test assertions (the assertions read after events fire, which use correct formatting), but they cause confusing initial display.

Detailed Code Review

File Change Assessment
MauiCarouselRecyclerView.cs _isInternalPositionUpdate flag ✅ Correct approach
MauiCarouselRecyclerView.cs ScrollToItemPosition helper with bounds check ✅ Good improvement
MauiCarouselRecyclerView.cs Flag reset in dispatched lambda (no try/finally) ⚠️ Needs try/finally
CarouselViewHandler.Windows.cs _isInternalPositionUpdate with try/finally ✅ Correct
CarouselViewHandler.Windows.cs Explicit ItemsView.Position = currentItemPosition ✅ Fixes position sync
Issue29529.cs (HostApp) Insert(6,...) should be Insert(0,...) ❌ Wrong insert index
Issue29529.cs (HostApp) Missing : in initial label texts ⚠️ Minor cosmetic
Issue29529.cs (Tests) Asserts position=0, count=1 each ✅ Correct for KeepItemsInView

Requested Changes

1. Android MauiCarouselRecyclerView.cs: Add try/finally or equivalent exception safety for _isInternalPositionUpdate reset, consistent with the Windows handler
2. Issue29529.cs (HostApp): Change carouselItems.Insert(6, "Item 0") to carouselItems.Insert(0, "Item 0") and update button text to "Insert item at index 0" — to test the original reported bug scenario
3. Issue29529.cs (HostApp): Fix initial label text formatting ("Current Position: {carouselView.Position}" and "Current Item: {carouselView.CurrentItem}")
4. Issue29529.cs (Tests): Update test assertions to match new Insert(0) behavior — with KeepItemsInView and insert at index 0, verify position stays at 0 and PreviousPosition is 3

📋 Expand PR Finalization Review
Title: ✅ Good

Current: [Windows, Android] Fix for CurrentItemChangedEventArgs and PositionChangedEventArgs Not Working Properly in CarouselView

Description: ✅ Good

Description needs updates. See details below.

✨ Suggested PR Description

[!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

Windows: When a new item was added to the CarouselView's items source, ItemsView.Position was not explicitly updated to reflect the current item's new position. This left CurrentItem and Position out of sync. Additionally, when ItemsUpdatingScrollMode was KeepItemsInView, the resulting programmatic smooth scroll triggered multiple ScrollViewerViewChanged calls, causing PositionChanged to fire repeatedly with intermediate values — instead of once with the final position.

Android: Programmatic smooth scrolls during collection changes similarly triggered cascading PositionChanged events, producing multiple intermediate events instead of a single clean update.

Description of Change

Windows (CarouselViewHandler.Windows.cs):

• Added _isInternalPositionUpdate flag, set to true at the start of OnCollectionItemsSourceChanged and reset in a finally block (ensuring reset even if an exception occurs).
• On item add, explicitly sets ItemsView.Position = currentItemPosition to keep position in sync.
• Added handling for KeepLastItemInView (position = last item) and KeepItemsInView (position = 0) modes in the collection-changed handler.
• In UpdateCurrentItem, animation is disabled when _isInternalPositionUpdate is true, causing a direct jump scroll that fires PositionChanged only once.

Android (MauiCarouselRecyclerView.cs):

• Added _isInternalPositionUpdate flag, set to true at the start of CollectionItemsSourceChanged and reset at the exit points.

• Extracted a new ScrollToItemPosition(int position, bool shouldAnimate) helper method that:

  • Includes bounds checking (position < 0 || position >= Count) to prevent out-of-range scrolls.
  • Disables animation when _isInternalPositionUpdate is true, preventing cascading scroll events.

• Both UpdateFromCurrentItem and UpdateFromPosition now call ScrollToItemPosition instead of ItemsView.ScrollTo directly.

Key Technical Details

• _isInternalPositionUpdate flag: Guards scroll calls that occur as a side effect of collection mutations. When true, the animate parameter is forced to false, causing a jump scroll instead of a smooth scroll — which avoids triggering intermediate ViewChanged/OnScrolled callbacks.
• Windows uses try/finally to ensure the flag is always reset, even on exceptions.
• Android resets the flag manually at each exit point of CollectionItemsSourceChanged — not in a try/finally (see code review).

Issues Fixed

Fixes #29529

Platforms Tested

• Android
• Windows
• iOS
• Mac

Code Review: ✅ Passed

Code Review Findings — PR #31275

🟠 High Priority Issues

1. Unresolved Review Comment: Test inserts at wrong index

File: src/Controls/tests/TestCases.HostApp/Issues/Issue29529.cs, line 103 Reviewer comment (copilot-pull-request-reviewer, unresolved):

// Current (BUG):
carouselItems.Insert(6, "Item 0");

// Should be:
carouselItems.Insert(0, "Item 0");

Problem: The button text says "Insert item at index 6", but inserting at index 6 in a 6-item collection (indices 0–5) adds the item to the end, not the beginning. The original issue reports a bug with inserting at index 0. The UI test verifies behavior after inserting at index 6, which tests KeepItemsInView scroll-to-zero behavior rather than the issue's described scenario of insertion at the front.

This is a functional mismatch: the test is not actually reproducing the original bug scenario. The button text and insert index should both be corrected.

2. Unresolved Review Comment: Label text formatting — missing : separator

File: src/Controls/tests/TestCases.HostApp/Issues/Issue29529.cs, lines 57 and 65 Reviewer comments (copilot-pull-request-reviewer, unresolved):

// Line 57 - Current (BUG):
Text = $"Current Position{carouselView.Position}",
// Should be:
Text = $"Current Position: {carouselView.Position}",

// Line 65 - Current (BUG):
Text = $"Current Item{carouselView.CurrentItem}",
// Should be:
Text = $"Current Item: {carouselView.CurrentItem}",

Problem: The initial label text is missing : between the label name and the value. The event handlers correctly format as "Current Position: {e.CurrentPosition}", but the initialization strings don't match this format. If the test page is ever inspected before the first event fires, the displayed values won't match expectations and could confuse debugging.

More importantly, the positionLabel and itemLabel initial values are used as the initial display — if the test reads these before tapping InsertButton, the format mismatch could cause test failures in edge cases.

🟡 Medium Priority Issues

3. Android: _isInternalPositionUpdate flag not guarded with try/finally

File: src/Controls/src/Core/Handlers/Items/Android/MauiCarouselRecyclerView.cs

The Windows implementation (correctly) wraps the entire OnCollectionItemsSourceChanged body in a try/finally block to ensure _isInternalPositionUpdate is always reset. The Android CollectionItemsSourceChanged method resets the flag manually at two exit points:

1. Early return when removingAnyPrevious is true
2. At the end of the async MainThread.BeginInvokeOnMainThread (or equivalent) callback

If any exception occurs between setting _isInternalPositionUpdate = true and the final reset, the flag remains true permanently. Subsequent scroll operations would silently have animations disabled even in non-collection-change contexts.

Recommendation: Apply the same try/finally pattern as Windows:

_isInternalPositionUpdate = true;
try
{
    // ... existing logic ...
}
finally
{
    _isInternalPositionUpdate = false;
}

Note: the async callback adds complexity — the flag should ideally be reset after the async work completes, not in a synchronous finally. Consider whether the flag needs to be scoped to the async continuation as well.

4. Windows: KeepItemsInView unconditionally sets carouselPosition = 0

File: src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs

else if (ItemsView.ItemsUpdatingScrollMode == ItemsUpdatingScrollMode.KeepItemsInView)
{
    carouselPosition = 0;
}

Problem: KeepItemsInView means "keep the currently visible items in view" — when adding items, the view should not scroll away. Unconditionally setting carouselPosition = 0 overrides any current position and always jumps to the first item, even if the user was viewing item 3 of 10. This seems overly aggressive and may change behavior in existing apps using KeepItemsInView.

The original code before this PR did not include this branch for KeepItemsInView. It was only added in this PR and is not explained in the description.

Recommendation: Verify whether this is the intended behavior for KeepItemsInView. If adding an item at any position should always reset the carousel to position 0, that should be clearly documented. If not, this branch may be incorrect.

✅ Positive Observations

• Windows try/finally: Correctly uses try/finally to guarantee _isInternalPositionUpdate is always reset, even on exceptions. This was added in response to a reviewer comment and is the right approach.
• Android bounds check: ScrollToItemPosition validates position >= 0 && position < Count before scrolling, preventing out-of-range scroll attempts. This was added in response to a reviewer comment.
• Android helper method extraction: The new ScrollToItemPosition helper cleanly centralizes the animation-disable logic, avoiding duplication in UpdateFromCurrentItem and UpdateFromPosition.
• UI tests added: Both a host app page and a shared NUnit test are included for the new functionality.

Addressed all AI Agent concerns — corrected Insert(0, ...) with button text "Insert item at index 0", fixed label initial text formatting (": "), wrapped the Android dispatch lambda in try/finally to ensure _isInternalPositionUpdate resets on exception and scoped UpdateItemDecoration/UpdateVisualStates inside the _scrollToCounter guard, and confirmed KeepItemsInView → carouselPosition = 0 is intentional — Android and iOS had identical behavior pre-PR, the Windows branch now aligns with them. No pending concerns remaining.

[praveenkumarkarunanithi]

Could you please resolve conflicts?

Conflicts have been resolved in the branch.

[sheiksyedm]

/azp run maui-pr-uitests

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).