[Android] Fix for Incorrect ItemsViewScrolledEventArgs Values in CollectionView with Grouped Items#31437
[Android] Fix for Incorrect ItemsViewScrolledEventArgs Values in CollectionView with Grouped Items#31437SyedAbdulAzeemSF4852 wants to merge 7 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes incorrect item index values in CollectionView's ItemsViewScrolledEventArgs when IsGrouped is set to true. The issue affected both Android and iOS platforms where the FirstVisibleItemIndex, CenterItemIndex, and LastVisibleItemIndex reported wrong values due to group headers and footers not being properly handled.
- Updated Android's RecyclerViewScrollListener to correctly map RecyclerView positions to actual data item indices by excluding group headers and footers
- Fixed iOS sorting logic to order visible items by Section first, then Row for proper cross-section ordering
- Added comprehensive test coverage with both HostApp UI test page and NUnit test implementation
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs | Major refactoring of GetVisibleItemsIndex method with new helper methods to handle grouped data sources correctly |
| src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs | Updated sorting logic to order by Section then Row for consistent ordering |
| src/Controls/src/Core/Handlers/Items2/iOS/ItemsViewDelegator2.cs | Applied same sorting fix as the other iOS delegator |
| src/Controls/tests/TestCases.HostApp/Issues/Issue17664.cs | Added UI test page demonstrating grouped CollectionView scrolling behavior |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue17664.cs | Added NUnit test to validate correct visible item indices |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
76a383f to
9e6da5d
Compare
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & Validation📝 Review Session — Used Math.Clamp instead of Math.Max to ensure that visible item indices stay within the valid range. ·
|
| File | Changes | Purpose |
|---|---|---|
RecyclerViewScrollListener.cs |
+120/-37 | Android: AdjustGroupIndex helpers to skip headers/footers |
ItemsViewDelegator.cs (Items/) |
+2/-1 | iOS: Sort by Section then Row |
ItemsViewDelegator2.cs (Items2/) |
+2/-1 | iOS/MacCatalyst: Same sort fix |
Issue17664.cs (HostApp) |
+120 | Test page with grouped CollectionView |
Issue17664.cs (Shared.Tests) |
+29 | NUnit test validating fix |
Key Findings
- Fix uses
AdjustGroupIndexwith 6 Copilot reviewer flagged this as complexparameters AdjustGroupIndexreturnsGetGroupedDataCount()-1for negative positions; Copilot suggests returning0instead- Test uses
#if WINDOWSinline directive withThread.Sleep( violates UI test guidelines (no inline #if in test methods)1000) - Both new test files are missing trailing newlines
kubafloCHANGES_REQUESTED (open, Feb 19, 2026): Split PR into 2 separate PRs (iOS and Android)- Windows checked in validated platforms table but no Windows code changed;
[Issue]attribute restricts test to iOS|Android - Prior agent review (Feb 18-20, 2026) confirmed gate passed on Android; 5 try-fix alternatives all passed; recommended REQUEST CHANGES
Reviewer Feedback Summary
| Reviewer | Status | Key Feedback |
|---|---|---|
| jsuarezruiz | CHANGES_REQUESTED (resolved) | Expand validation, use Math.Clamp, clarify dataIndex semantics |
| copilot-pull-request-reviewer | COMMENTED | Negative position should return 0; AdjustGroupIndex has 6 params |
| kubaflo | CHANGES_REQUESTED (OPEN) | Split PR into 2 separate PRs: iOS and Android |
Open Issues
- BLOCKING (kubaflo): Split PR into separate iOS and Android not yet addressedPRs
AdjustGroupIndexreturns last item for negative positions; should return0(first item)- Inline
#if WINDOWS+Thread.Sleepin test violates UI test guidelines - Missing trailing newlines in both test files
- Windows checked as validated when no Windows code changed
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31437 | Android: AdjustGroupIndex helpers to skip headers/footers; iOS: sort by Section+ PENDING (Gate) | 3 fix files | Original PR | Row |
🚦 Gate — Test Verification
📝 Review Session — Used Math.Clamp instead of Math.Max to ensure that visible item indices stay within the valid range. · 9e6da5d
Result PASSED:
Platform: android
Mode: Full Verification
- Tests FAIL without fix
- Tests PASS with fix (imported from prior agent emulator config blocker in current session)review
Fresh Verification (This Session)
- Without fix: Tests Test correctly detected the absence of the fixFAILED
- With fix: Android emulator config error (
Unknown AVD name [E]); infrastructure issue, not a code problemINCONCLUSIVE
Prior Review Import
Prior agent review (Feb 18, 2026) confirmed full verification passed on Android:
- Tests FAIL without fix
- Tests PASS with fix
- Commit validated:
9e6da5d(unchanged since prior review)
The test VerifyGroupedCollectionViewVisibleItemIndices scrolls to "Category C, Item #2" and verifies LastVisibleItemIndex maps to "Category C item #2". Without the fix, LastVisibleItemIndex is inflated by group headers/footers, causing wrong item text to be displayed.
🔧 Fix — Analysis & Comparison
📝 Review Session — Used Math.Clamp instead of Math.Max to ensure that visible item indices stay within the valid range. · 9e6da5d
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-sonnet-4.5) | Direct GetGroupAndIndex API to convert positions; ConvertToDataIndex + BLOCKED | 1 file | HVF emulator unavailable | CountDataItemsInGroup |
| 2 | try-fix (claude-opus-4.6) | Position-to-DataIndex mapping array (O(N) once, O(1) lookup); TranslateGroupedPositions + BLOCKED | 1 file | HVF emulator unavailable | ResolveDataIndex |
| PR | PR #31437 | AdjustGroupIndex(6 params) + GetGroupedDataCount + FindNextDataIndex + FindPrevDataIndex; iOS: sort by Section+Row PASS (Gate) | 3 files | Original PR; Gate validated |
Cross-Pollination Round 1
| Model | Response |
|---|---|
| gpt-5.2 | NEW IDEA: Prefix-sum cache per group with incremental updates via collection-change notifications |
| gpt-5.2-codex | NEW IDEA: Store GroupIndex/IndexInGroup in ViewHolder during bind, read from first visible ViewHolder |
| gemini-3-pro-preview | NEW IDEA: Flattened Source wrap IEnumerable as 1:1 IList, shift complexity to data source layer |
| claude-sonnet-4 | NEW IDEA: Differential Position maintain rolling offset of non-data items above viewport |
| claude-sonnet-4.5 | NEW IDEA: (was used in attempt 1) |
All new ideas are architecturally heavier than the PR's fix (require state tracking, collection-change wiring, or significant refactoring) and cannot be validated in the current environment.
Environment Blocker
All 2 fresh try-fix attempts were blocked by Android emulator HVF error:
HVF error: HV_UNSUPPORTED
qemu-system-aarch64-headless: failed to initialize HVF: Invalid argument
Hardware virtualization is unavailable on this machine. Android testing is not possible.
Prior Review Import
Prior agent review (Feb 18-20, 2026) ran all 5 models with a working emulator:
- All 5 alternatives tested and all passed (subtraction counting, cache-based, BinarySearch, GetGroupAndIndex, GroupInfo precompute)
- PR's fix was selected as best: simplest to review, already validated, fewest moving parts
Exhausted: Yes (environment-limited; all reasonable algorithmic approaches are covered; cross-pollination produced new ideas but all are architecturally heavier than PR's fix)
Selected Fix: PR's only validated candidate; 5 independent alternatives from prior review also passed, confirming the approach is sound. New ideas from cross-pollination require significant additional complexity that is not warranted for this targeted fix.fix
📋 Report — Final Recommendation
📝 Review Session — Used Math.Clamp instead of Math.Max to ensure that visible item indices stay within the valid range. · 9e6da5d
Final Recommendation: REQUEST CHANGES##
Summary
PR #31437 fixes incorrect FirstVisibleItemIndex, CenterItemIndex, and LastVisibleItemIndex values in ItemsViewScrolledEventArgs when CollectionView.IsGrouped = true. Gate verification (Android) confirmed tests fail without fix and pass with fix. Five independent try-fix alternatives from the prior agent review (Feb 18-20, 2026) also passed, confirming the root cause analysis and fix approach are sound.
However, there are blocking and non-blocking issues that should be addressed before merge.
Root Cause
- Android:
RecyclerViewScrollListener.GetVisibleItemsIndexreturned raw RecyclerView adapter positions without excluding group headers/footers. Every visible item's index was off by the number of headers/footers preceding it in the adapter list. - iOS:
ItemsViewDelegator.GetVisibleIndexPathssorted visibleNSIndexPathvalues only byRow, producing incorrect ordering when items from multipleSections were simultaneously visible. This meansFirstVisibleItemIndexalways appeared to be in section 0.
Fix Quality
- Gate passed ( tests fail without fix, pass with fixAndroid)
- 5 independent try-fix alternatives (prior review) also strong validationpassed
- iOS fix is minimal and clearly correct:
OrderBy(x => x.Section).ThenBy(x => x.Row) - Fix applied consistently to both Items/ and Items2/ iOS delegators
- Non-grouped Android path simplified and cleaned up with Math.Clamp
AdjustGroupIndexhas 6 parameters (flagged by Copilot reviewer as difficult to understand)-
Whenposition < 0,AdjustGroupIndexreturnsGetGroupedDataCount()-1(last Copilot suggests returning0for negative positionsitem) -
Test uses inline#if WINDOWSwithThread.Sleep( violates UI test guidelines1000)-
Issues for Author to Address
Must Fix (blocking)
- kubaflo CHANGES_REQUESTED (open Feb 19, 2026): Split into 2 separate one for iOS fix, one for Android fix. Unaddressed open review from MAUI team member.PRs
Should Fix
- Negative position handling in
AdjustGroupIndex:position < 0returns last item (GetGroupedDataCount()-1). Should return0(first item). Copilot reviewer explicitly flagged this. - Test: inline
#if WINDOWS+Thread.Sleep: UI test guidelines prohibit inline platform directives in test methods. Replace withApp.WaitForElement(...)timeout orVerifyScreenshot(retryTimeout:...). Also:[Issue]attribute restricts page toPlatformAffected.iOS | PlatformAffected.Androidso this Windows code is unreachable. - Missing trailing newlines: Both
Issue17664.cstest files end with\ No newline at end of file.
Consider (nitpick)
AdjustGroupIndex6 parameters: Consider extracting a context struct or splitting into smaller methods.- Windows checked as validated: PR description marks Windows in "Validated platforms" but no Windows-specific code changed and the test page is restricted to iOS|Android.
Title Review
Current: [Android, iOS] Fix for Incorrect ItemsViewScrolledEventArgs Values in CollectionView with Grouped Items
Assessment: Acceptable. Could be tightened to: [Android, iOS] CollectionView: Fix scroll event indices for grouped items
Description Review
Assessment: has NOTE block, per-platform root cause, description of change, and before/after video validation. Minor update recommended: uncheck Windows from validated platforms list.Good
Phase Results
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight COMPLETE | Issue #17664, Android+iOS affected, 5 files changed | |
| Gate PASSED | Android; tests FAIL without fix, PASS with fix (imported from prior review; fresh run confirmed "fails without fix", "with fix" blocked by HVF emulator) | |
| Fix COMPLETE | 2 fresh try-fix attempts BLOCKED (HVF emulator); 5 prior review alternatives all passed; PR's fix selected | |
| Report COMPLETE | REQUEST CHANGES recommended |
📋 Expand PR Finalization Review
Title: ✅ Good
Current: [Android, iOS] Fix for Incorrect ItemsViewScrolledEventArgs Values in CollectionView with Grouped Items
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!
Issue Details
When a CollectionView's IsGrouped property is set to true, the values of FirstVisibleItemIndex, CenterItemIndex, and LastVisibleItemIndex in the ItemsViewScrolledEventArgs passed to the Scrolled event handler are incorrect.
Root Cause
Android
The GetVisibleItemsIndex method did not account for group headers and footers in grouped data sources. The raw RecyclerView positions include group header and footer rows, so the calculated indices were offset and inaccurate.
iOS
Visible items were sorted only by Row, which produced incorrect ordering when items from multiple sections were visible simultaneously (e.g., the last item of section 0 followed by the first item of section 1 would be misordered).
Description of Change
Android (RecyclerViewScrollListener.cs)
When the items source is a grouped source (IGroupableItemsViewSource, excluding UngroupedItemsSource), the raw RecyclerView position is translated to a logical data-item index via a new AdjustGroupIndex helper. This helper iterates through the items, skipping group headers and footers, to map the raw position to the correct 0-based data item index. Helper methods FindNextDataIndex and FindPrevDataIndex are used to snap header/footer positions to the nearest preceding or following data item. GetGroupedDataCount counts only data items for bounds checking.
For non-grouped sources with headers/footers, the existing simpler subtraction logic is preserved (now using Math.Clamp to bound the result).
iOS (ItemsViewDelegator.cs and ItemsViewDelegator2.cs)
Changed the sort from .OrderBy(x => x.Row) to .OrderBy(x => x.Section).ThenBy(x => x.Row) so that IndexPathsForVisibleItems is ordered correctly across section boundaries. The fix is applied to both the legacy handler (Items/) and the current handler (Items2/).
Issues Fixed
Fixes #17664
Platforms Tested
- Android ✅ Fixed
- iOS ✅ Fixed
- Windows
⚠️ Not fixed in this PR (original issue also affects Windows — tracked separately) - Mac
⚠️ Not fixed in this PR
Code Review: ⚠️ Issues Found
Code Review — PR #31437
🔴 Critical Issues
1. Potential off-by-one: dataIndex initialized to hasHeader ? 1 : 0 in AdjustGroupIndex
File: src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs
Problem:
int dataIndex = hasHeader ? 1 : 0, currentItem = hasHeader ? 1 : 0;When hasHeader = true (the CollectionView has a global header), dataIndex starts at 1. The first data item encountered in the loop would then return index 1 instead of 0. This means all data indices are off by one when the collection has both a global header and grouped content.
The existing test (Issue17664) creates a CollectionView with no global header (hasHeader = false), so it doesn't cover this case and won't catch the bug.
currentItem correctly starts at 1 to skip the global header row in the RecyclerView. But dataIndex should always start at 0 because data items are 0-indexed from the consumer's perspective.
Recommendation:
int dataIndex = 0, currentItem = hasHeader ? 1 : 0;🟡 Suggestions / Nitpicks
2. Open review comment not addressed: position < 0 returns last item index (incorrect)
File: src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs (line ~130)
Open comment from copilot-pull-request-reviewer (unresolved):
The current guard for out-of-bounds position:
if (position < 0 || position >= count)
{
return Math.Max(0, GetGroupedDataCount(source) - 1);
}Returns the last valid data item index for both negative positions and positions beyond the count. For position < 0 (item not visible / no item found), returning the last item index is incorrect. It should return 0 (or -1 if the caller can handle it), as a negative position means "no item visible at this end."
Recommendation:
if (position < 0)
{
return 0;
}
if (position >= count)
{
return Math.Max(0, GetGroupedDataCount(source) - 1);
}This matches the suggestion left by copilot-pull-request-reviewer (thread PRRT_kwDOD6PVWM5brioJ), which is still open and unresolved.
3. Open review comment not addressed: AdjustGroupIndex has 6 parameters
File: src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs
Open comment from copilot-pull-request-reviewer (unresolved, thread PRRT_kwDOD6PVWM5briop):
The AdjustGroupIndex method signature is difficult to understand at the call site:
static int AdjustGroupIndex(IGroupableItemsViewSource source, int position, bool hasHeader, bool hasFooter, int count, bool isStart)The isStart parameter is particularly unclear — it's a boolean flag that controls whether to snap to the "next" or "prev" data item when the position lands on a group header/footer. For the centerItemIndex call, isStart: true is used, but "center" semantics don't clearly map to either "start" or "end."
Recommendation: Either create a parameter object, or document the isStart parameter semantics with a comment, and verify the center-item behavior is correct.
4. Open unresolved discussion: dataIndex semantics in FindNextDataIndex
File: src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs (line ~191)
Open comment from jsuarezruiz (thread PRRT_kwDOD6PVWM5f9Mjy, unresolved):
The reviewer asked what dataIndex represents in FindNextDataIndex — is it the index of the "next item to return (exclusive)" or the "last item seen (inclusive)"? The author replied in the thread but the comment was not resolved, indicating the answer may not have been sufficient or a code clarification is still needed.
Recommendation: Add a clarifying comment to FindNextDataIndex explaining the semantics:
// 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)5. Performance: O(n) traversal on every scroll event for grouped collections
File: src/Controls/src/Core/Handlers/Items/Android/RecyclerViewScrollListener.cs
AdjustGroupIndex iterates linearly through items to translate a RecyclerView position to a data index. It is called three times per scroll event (for first, center, last). GetGroupedDataCount also iterates all items when position is out of bounds. For large grouped datasets, this O(n) traversal on every OnScrolled callback could cause jank.
Recommendation: This is acceptable for typical grouped list sizes, but worth noting. A future optimization could cache a position→data-index mapping that is invalidated when items change.
6. Test uses #if WINDOWS Thread.Sleep(1000) — anti-pattern
File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue17664.cs
#if WINDOWS
Thread.Sleep(1000);
#endifPer the UI test guidelines, arbitrary Thread.Sleep calls are discouraged. Use App.WaitForElement with a timeout or VerifyScreenshot(retryTimeout: ...) instead. Inline #if directives in test methods also reduce readability.
Recommendation:
App.WaitForElement("Issue17664DescriptionLabel", timeout: TimeSpan.FromSeconds(3));
var resultItem = App.FindElement("Issue17664DescriptionLabel").GetText();7. Missing newline at end of test files
Both new test files end with \ No newline at end of file:
src/Controls/tests/TestCases.HostApp/Issues/Issue17664.cssrc/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue17664.cs
Recommendation: Add a trailing newline to both files.
✅ Looks Good
- iOS fix is clean and correct. Changing
OrderBy(x => x.Row)toOrderBy(x => x.Section).ThenBy(x => x.Row)is a minimal, well-targeted fix. Applied correctly to bothItemsViewDelegator.cs(Items/ handler) andItemsViewDelegator2.cs(Items2/ handler). - Grouped vs non-grouped detection using
itemsSource is not UngroupedItemsSource && itemsSource is IGroupableItemsViewSourceis appropriate. Math.Clampon the non-grouped path (addressing a previous review comment) correctly bounds indices.- Test structure is appropriate — uses
AutomationId, inherits_IssuesUITest, has single[Category], correct naming convention. - Test scenario is representative — uses 3 groups × 5 items to verify index correctness after programmatic scroll.
kubaflo
left a comment
There was a problem hiding this comment.
Could you split this PR on 2 separate ones: iOS and Android? Please use the same test for both
…d position check to return the last valid item when position is out of bounds.
…es stay within the valid range.
8d34cff to
d4c6b1c
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 31437Or
iex "& { $(irm https://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 31437" |
…d clarify dataIndex semantics in FindNextDataIndex.
|
Addressed concerns raised in the AI summary. |
…dices with grouped items (#34240) > [!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 - When a CollectionView's IsGrouped property is set to true, the values of FirstVisibleItemIndex, CenterItemIndex, and LastVisibleItemIndex in the ItemsViewScrolledEventArgs passed to the Scrolled event handler are incorrect. ### Root Cause - Visible items were sorted only by Row, which produced incorrect ordering when items from multiple sections were visible simultaneously. ### Description of Change **iOS (ItemsViewDelegator.cs and ItemsViewDelegator2.cs)** - Changed the sort from .OrderBy(x => x.Row) to .OrderBy(x => x.Section).ThenBy(x => x.Row) so that IndexPathsForVisibleItems is ordered correctly across section boundaries. The fix is applied to both the legacy handler (Items/) and the current handler (Items2/). ### Issues Fixed Fixes #17664 ### Validated the behaviour in the following platforms - [x] Windows - [x] Android - [x] iOS - [x] Mac Android fix PR: #31437 ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/3612815d-3a43-4dd5-8d2c-3832e3d4a077"> | <video src="https://github.com/user-attachments/assets/f6cd63b8-1ed9-465a-99d9-11e38b004dac"> |
…dices with grouped items (#34240) > [!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 - When a CollectionView's IsGrouped property is set to true, the values of FirstVisibleItemIndex, CenterItemIndex, and LastVisibleItemIndex in the ItemsViewScrolledEventArgs passed to the Scrolled event handler are incorrect. ### Root Cause - Visible items were sorted only by Row, which produced incorrect ordering when items from multiple sections were visible simultaneously. ### Description of Change **iOS (ItemsViewDelegator.cs and ItemsViewDelegator2.cs)** - Changed the sort from .OrderBy(x => x.Row) to .OrderBy(x => x.Section).ThenBy(x => x.Row) so that IndexPathsForVisibleItems is ordered correctly across section boundaries. The fix is applied to both the legacy handler (Items/) and the current handler (Items2/). ### Issues Fixed Fixes #17664 ### Validated the behaviour in the following platforms - [x] Windows - [x] Android - [x] iOS - [x] Mac Android fix PR: #31437 ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/3612815d-3a43-4dd5-8d2c-3832e3d4a077"> | <video src="https://github.com/user-attachments/assets/f6cd63b8-1ed9-465a-99d9-11e38b004dac"> |
…dices with grouped items (#34240) > [!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 - When a CollectionView's IsGrouped property is set to true, the values of FirstVisibleItemIndex, CenterItemIndex, and LastVisibleItemIndex in the ItemsViewScrolledEventArgs passed to the Scrolled event handler are incorrect. ### Root Cause - Visible items were sorted only by Row, which produced incorrect ordering when items from multiple sections were visible simultaneously. ### Description of Change **iOS (ItemsViewDelegator.cs and ItemsViewDelegator2.cs)** - Changed the sort from .OrderBy(x => x.Row) to .OrderBy(x => x.Section).ThenBy(x => x.Row) so that IndexPathsForVisibleItems is ordered correctly across section boundaries. The fix is applied to both the legacy handler (Items/) and the current handler (Items2/). ### Issues Fixed Fixes #17664 ### Validated the behaviour in the following platforms - [x] Windows - [x] Android - [x] iOS - [x] Mac Android fix PR: #31437 ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/3612815d-3a43-4dd5-8d2c-3832e3d4a077"> | <video src="https://github.com/user-attachments/assets/f6cd63b8-1ed9-465a-99d9-11e38b004dac"> |
…dices with grouped items (dotnet#34240) > [!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 - When a CollectionView's IsGrouped property is set to true, the values of FirstVisibleItemIndex, CenterItemIndex, and LastVisibleItemIndex in the ItemsViewScrolledEventArgs passed to the Scrolled event handler are incorrect. ### Root Cause - Visible items were sorted only by Row, which produced incorrect ordering when items from multiple sections were visible simultaneously. ### Description of Change **iOS (ItemsViewDelegator.cs and ItemsViewDelegator2.cs)** - Changed the sort from .OrderBy(x => x.Row) to .OrderBy(x => x.Section).ThenBy(x => x.Row) so that IndexPathsForVisibleItems is ordered correctly across section boundaries. The fix is applied to both the legacy handler (Items/) and the current handler (Items2/). ### Issues Fixed Fixes dotnet#17664 ### Validated the behaviour in the following platforms - [x] Windows - [x] Android - [x] iOS - [x] Mac Android fix PR: dotnet#31437 ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/3612815d-3a43-4dd5-8d2c-3832e3d4a077"> | <video src="https://github.com/user-attachments/assets/f6cd63b8-1ed9-465a-99d9-11e38b004dac"> |
…dices with grouped items (#34240) > [!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 - When a CollectionView's IsGrouped property is set to true, the values of FirstVisibleItemIndex, CenterItemIndex, and LastVisibleItemIndex in the ItemsViewScrolledEventArgs passed to the Scrolled event handler are incorrect. ### Root Cause - Visible items were sorted only by Row, which produced incorrect ordering when items from multiple sections were visible simultaneously. ### Description of Change **iOS (ItemsViewDelegator.cs and ItemsViewDelegator2.cs)** - Changed the sort from .OrderBy(x => x.Row) to .OrderBy(x => x.Section).ThenBy(x => x.Row) so that IndexPathsForVisibleItems is ordered correctly across section boundaries. The fix is applied to both the legacy handler (Items/) and the current handler (Items2/). ### Issues Fixed Fixes #17664 ### Validated the behaviour in the following platforms - [x] Windows - [x] Android - [x] iOS - [x] Mac Android fix PR: #31437 ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/3612815d-3a43-4dd5-8d2c-3832e3d4a077"> | <video src="https://github.com/user-attachments/assets/f6cd63b8-1ed9-465a-99d9-11e38b004dac"> |
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check updated: #31437 (comment)
|
|
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
Android (RecyclerViewScrollListener.cs)
When the items source is a grouped source (IGroupableItemsViewSource, excluding UngroupedItemsSource), the raw RecyclerView position is translated to a logical data-item index via a new AdjustGroupIndex helper. This helper iterates through the items, skipping group headers and footers, to map the raw position to the correct 0-based data item index.
Helper methods FindNextDataIndex and FindPrevDataIndex are used to snap header/footer positions to the nearest preceding or following data item.
GetGroupedDataCount counts only data items for bounds checking.
For non-grouped sources with headers/footers, the existing simpler subtraction logic is preserved (now using Math.Clamp to bound the result).
Issues Fixed
Fixes #17664
Validated the behaviour in the following platforms
iOS fix PR: #34240
Output
Android_Before.mov
Android_After.mov