Skip to content

[Android] Fix for Incorrect ItemsViewScrolledEventArgs Values in CollectionView with Grouped Items #31437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SyedAbdulAzeemSF4852 wants to merge 8 commits into from
Closed

[Android] Fix for Incorrect ItemsViewScrolledEventArgs Values in CollectionView with Grouped Items #31437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
de97aa1
[Android & iOS] Fix for incorrect ItemsViewScrolledEventArgs in Colle…
SyedAbdulAzeemSF4852 Aug 11, 2025
7bb728e
Modified the fix and have added comment lines to explain the logic
SyedAbdulAzeemSF4852 Aug 12, 2025
8c051e4
Updated variable naming and condition order in Android; added comment…
SyedAbdulAzeemSF4852 Aug 13, 2025
ccd2ef5
Added GetGroupedDataCount method to count only data items, and update…
SyedAbdulAzeemSF4852 Sep 24, 2025
ce3b093
Used Math.Clamp instead of Math.Max to ensure that visible item indic…
SyedAbdulAzeemSF4852 Oct 23, 2025
e0db804
Revert iOS fix
SyedAbdulAzeemSF4852 Feb 25, 2026
10ad5ac
Fix off-by-one in AdjustGroupIndex, correct position < 0 handling, an…
SyedAbdulAzeemSF4852 Mar 10, 2026
d01eeb8
Fix grouped CollectionView scroll index ordering on iOS and re-enable…
SyedAbdulAzeemSF4852 May 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 129 additions & 37 deletions src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#nullable disable
using System;
using AndroidX.RecyclerView.Widget;

namespace Microsoft.Maui.Controls.Handlers.Items
Expand Down Expand Up @@ -122,9 +123,7 @@ void HandleRemainingItemsThresholdReached()

protected virtual (int First, int Center, int Last) GetVisibleItemsIndex(RecyclerView recyclerView)
{
var firstVisibleItemIndex = -1;
var lastVisibleItemIndex = -1;
var centerItemIndex = -1;
int firstVisibleItemIndex = -1, lastVisibleItemIndex = -1, centerItemIndex = -1;

if (recyclerView.GetLayoutManager() is LinearLayoutManager linearLayoutManager)
{
Expand All @@ -133,63 +132,156 @@ protected virtual (int First, int Center, int Last) GetVisibleItemsIndex(Recycle
centerItemIndex = recyclerView.CalculateCenterItemIndex(firstVisibleItemIndex, linearLayoutManager, _getCenteredItemOnXAndY);
}

bool hasHeader = ItemsViewAdapter.ItemsSource.HasHeader;
bool hasFooter = ItemsViewAdapter.ItemsSource.HasFooter;
int itemsCount = ItemsViewAdapter.ItemCount;
var adapter = ItemsViewAdapter;
var itemsSource = adapter.ItemsSource;
int itemsCount = adapter.ItemCount;
bool hasHeader = itemsSource.HasHeader;
bool hasFooter = itemsSource.HasFooter;

if (!hasHeader && !hasFooter)
if (itemsSource is not UngroupedItemsSource && itemsSource is IGroupableItemsViewSource groupable)
{
return (firstVisibleItemIndex, centerItemIndex, lastVisibleItemIndex);
return (

@MauiBot MauiBot Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] CollectionView Android — RemainingItemsThreshold regression for grouped collections

GetVisibleItemsIndex now returns logical data-item indices (0-based, excluding group headers/footers) for grouped sources. However, OnScrolled still computes actualItemCount = ItemsViewAdapter.ItemCount - headerValue - footerValue, which includes group headers and group footers in the count.

Concrete scenario: 3 categories × 5 items with group headers = 18 adapter items, no collection header/footer.

  • actualItemCount = 18
  • Last (logical data index) is at most 14
  • Last == actualItemCount - 1 → 14 == 17 → never true
  • RemainingItemsThresholdReached never fires for grouped collections after this PR

This is a functional regression introduced by this fix. The OnScrolled threshold check at lines 78–92 must be updated to use the logical data count (e.g., GetGroupedDataCount) when the source is grouped, rather than the raw adapter item count.

AdjustGroupIndex(groupable, firstVisibleItemIndex, hasHeader, hasFooter, itemsCount, snapForward: true),
AdjustGroupIndex(groupable, centerItemIndex, hasHeader, hasFooter, itemsCount, snapForward: true),
AdjustGroupIndex(groupable, lastVisibleItemIndex, hasHeader, hasFooter, itemsCount, snapForward: false)
);
Comment thread
SyedAbdulAzeemSF4852 marked this conversation as resolved.
}

if (firstVisibleItemIndex == 0 && lastVisibleItemIndex == itemsCount - 1)
// Adjust for footer: if the last visible item is the footer, decrement to get the last data item index
if (hasFooter && lastVisibleItemIndex == itemsCount - 1)

@MauiBot MauiBot Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[moderate] Logic and Correctness — centerItemIndex not adjusted for footer in non-grouped path

The footer check only decrements lastVisibleItemIndex. If the center-visible adapter position is the footer (itemsCount - 1), centerItemIndex is not decremented here. After the subsequent header-offset subtraction it becomes itemsCount - 2. For a 5-data-item list with both header and footer (itemsCount = 7): centerItemIndex = 6 (footer) → no footer decrement → header decrement → 5 → Math.Clamp(5, 0, maxValidIndex=6) = 5. The valid data-index range is 0–4, so CenterItemIndex = 5 is reported but does not correspond to any data item.

Fix: also apply the footer decrement to centerItemIndex when hasFooter && centerItemIndex == itemsCount - 1.

{
lastVisibleItemIndex -= hasHeader && hasFooter ? 2 : 1;
lastVisibleItemIndex--;
}
else

// Non-grouped items adjustment
if (hasHeader)
{
if (hasHeader && !hasFooter)
firstVisibleItemIndex--;
lastVisibleItemIndex--;
centerItemIndex--;
}

int maxValidIndex = Math.Max(0, itemsSource.Count - 1);

@MauiBot MauiBot Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Logic and Correctness — maxValidIndex upper bound is too large

itemsSource.Count includes the collection header and footer (see ObservableItemsSource.Count = ItemsCount() + (HasHeader?1:0) + (HasFooter?1:0) and ListSource.Count — both include header/footer). So maxValidIndex = itemsSource.Count - 1 equals dataCount + header + footer - 1 rather than dataCount - 1.

In practice, the prior footer/header decrements keep values within the correct range for first and last. But the oversized ceiling means the clamp does not catch a centerItemIndex that slips one position past the last valid data index (see the footer-adjustment bug above). The correct bound should be:

int maxValidIndex = Math.Max(0, itemsSource.Count - 1 - (hasHeader ? 1 : 0) - (hasFooter ? 1 : 0));

firstVisibleItemIndex = Math.Clamp(firstVisibleItemIndex, 0, maxValidIndex);
lastVisibleItemIndex = Math.Clamp(lastVisibleItemIndex, 0, maxValidIndex);
centerItemIndex = Math.Clamp(centerItemIndex, 0, maxValidIndex);

return (firstVisibleItemIndex, centerItemIndex, lastVisibleItemIndex);
}

/// <param name="snapForward">
/// When the adapter position falls on a group header or group footer,
/// true = snap to the first data item in the following group (use for FirstVisible/Center),
/// false = snap to the last data item in the preceding group (use for LastVisible).
/// </param>
static int AdjustGroupIndex(IGroupableItemsViewSource source, int position, bool hasHeader, bool hasFooter, int count, bool snapForward)

@MauiBot MauiBot Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[major] Performance-Critical Path — O(N²) per scroll event for grouped collections

AdjustGroupIndex iterates from position 0 to position, and on each step calls source.IsGroupHeader(currentItem) and source.IsGroupFooter(currentItem). Each call to ObservableGroupedSource.IsGroupHeader invokes GetGroupAndIndex(currentItem), which itself runs an O(currentItem) while-loop. Total cost = Σ(0..position) of O(currentItem) = O(position²) per call.

GetVisibleItemsIndex invokes AdjustGroupIndex three times per scroll event (first, center, last). At 60 fps with a 1000-item grouped list (e.g., 100 groups × 10 items + 100 group headers = 1100 adapter positions), this is roughly 3 × O(1100²) ≈ 3.6 M operations per scroll event.

Recommendation: avoid calling IsGroupHeader/IsGroupFooter in a position-scanning loop. Instead, implement a single O(N) forward pass that maintains group context (current group index + offset within group) and advances it in lockstep with currentItem, eliminating repeated calls to GetGroupAndIndex from scratch.

{
if (position < 0)
{
return 0;
}

if (position >= count)
{
return Math.Max(0, GetGroupedDataCount(source) - 1);
}

int dataIndex = 0, currentItem = hasHeader ? 1 : 0;

// Iterate through items until we reach the target position
while (currentItem <= position && currentItem < count)
{
if (hasFooter && currentItem == count - 1)
{
lastVisibleItemIndex -= 1;
firstVisibleItemIndex -= 1;
break;
}
else if (!hasHeader && hasFooter)

bool isHeader = source.IsGroupHeader(currentItem), isFooter = source.IsGroupFooter(currentItem);

// If current item is a normal data item (not header/footer)
if (!isHeader && !isFooter)
{
if (lastVisibleItemIndex == itemsCount - 1)
if (currentItem == position)
{
lastVisibleItemIndex -= 1;
return dataIndex;
}

dataIndex++;
}
else if (hasHeader && hasFooter)
// If position is a group header/footer, find the nearest data item
else if (currentItem == position)
{
if (firstVisibleItemIndex == 0)
{
lastVisibleItemIndex -= 1;
}
else if (lastVisibleItemIndex != itemsCount - 1)
{
firstVisibleItemIndex -= 1;
lastVisibleItemIndex -= 1;
}
else
{
firstVisibleItemIndex -= 1;
lastVisibleItemIndex -= 2;
}
return snapForward
? FindNextDataIndex(source, currentItem, hasFooter, count, dataIndex)
: FindPrevDataIndex(source, currentItem, hasHeader);
}

currentItem++;
}

// If we reach here, pos was beyond the last item
// Return the last valid data index (or 0 if empty)
return Math.Max(0, dataIndex - 1);
}

static int GetGroupedDataCount(IGroupableItemsViewSource source)
{
// Count data items only (excluding all headers and footers)
int dataCount = 0;
for (int index = 0; index < source.Count; index++)
{
if (!source.IsGroupHeader(index) && !source.IsGroupFooter(index) &&
!source.IsHeader(index) && !source.IsFooter(index))
{
dataCount++;
}
}

if (firstVisibleItemIndex < 0)
return dataCount;
}

// dataIndex: the 0-based data item index to assign to the next valid item found.
// Returned without incrementing because the item following a header/footer inherits this index.
static int FindNextDataIndex(IGroupableItemsViewSource source, int start, bool hasFooter, int count, int dataIndex)
Comment thread
SyedAbdulAzeemSF4852 marked this conversation as resolved.
{
for (int i = start + 1; i < count; i++)
{
firstVisibleItemIndex = 0;
// Skip footer item if present
if (hasFooter && i == count - 1)
{
break;
}

// If we find a regular item (not a group header or footer),
// return the current data index without incrementing
if (!source.IsGroupHeader(i) && !source.IsGroupFooter(i))
{
return dataIndex;
}
}

if (lastVisibleItemIndex < 0)
// If no valid data item found ahead, return the previous data index
// (or 0 if no valid items exist)
return Math.Max(0, dataIndex - 1);
}

static int FindPrevDataIndex(IGroupableItemsViewSource source, int start, bool hasHeader)
{
int lastValid = -1;
int currentItem = hasHeader ? 1 : 0;

for (; currentItem < start; currentItem++)
{
lastVisibleItemIndex = 0;
// Increment counter only for data items (not headers/footers)
// to get accurate position for last visible item index
if (!source.IsGroupHeader(currentItem) && !source.IsGroupFooter(currentItem))
{
lastValid++;
}
}

return (firstVisibleItemIndex, centerItemIndex, lastVisibleItemIndex);
// Return the last valid data item found (or 0 if none)
return Math.Max(0, lastValid);
}

protected override void Dispose(bool disposing)
Expand Down
2 changes: 1 addition & 1 deletion src/Controls/tests/TestCases.HostApp/Issues/Issue17664.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Maui.Controls.Sample.Issues;

[Issue(IssueTracker.Github, 17664, "Incorrect ItemsViewScrolledEventArgs in CollectionView when IsGrouped is set to true", PlatformAffected.iOS)]
[Issue(IssueTracker.Github, 17664, "Incorrect ItemsViewScrolledEventArgs in CollectionView when IsGrouped is set to true", PlatformAffected.iOS | PlatformAffected.Android)]
public class Issue17664 : ContentPage
{
CollectionView _collectionView;
Expand Down
5 changes: 1 addition & 4 deletions src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue17664.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
#if TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_WINDOWS // Android fix: https://github.com/dotnet/maui/pull/31437
// Windows: The Scrolled event is not consistently triggered in the CI environment during automated
// scrolling, so the label text is never updated. This is a test infrastructure limitation on Windows;
// the fix itself is iOS/MacCatalyst-only and works correctly on iOS and MacCatalyst.
#if TEST_FAILS_ON_WINDOWS // Windows: The Scrolled event is not consistently triggered in the CI environment during automated scrolling, so the label text is not updated. This is a known limitation of the test infrastructure on Windows.

@MauiBot MauiBot Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] Regression Prevention — test validates only LastVisibleItemIndex; FirstVisibleItemIndex and CenterItemIndex are untested

The fix translates all three grouped adapter positions to logical data indices (AdjustGroupIndex is called for first, center, and last). The test only exercises the LastVisibleItemIndex path via the Scrolled event handler. A regression in firstVisibleItemIndex or centerItemIndex (e.g., the missing footer adjustment or a future refactor) would go undetected.

Consider adding a second scroll scenario that exposes FirstVisibleItemIndex and CenterItemIndex through the label or separate labels so all three indices are exercised.

using NUnit.Framework;
using UITest.Appium;
using UITest.Core;
Expand Down
Loading