[iOS, Mac] Fix for CollectionView not triggering Scrolled event when grouped and groups are empty#30375
[iOS, Mac] Fix for CollectionView not triggering Scrolled event when grouped and groups are empty#30375SyedAbdulAzeemSF4852 wants to merge 2 commits intodotnet:mainfrom
Conversation
|
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. |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that grouped CollectionView instances with empty groups on iOS and Mac still trigger the Scrolled event by checking for headers/footers before early-returning, and adds end-to-end UI tests to validate the fix.
- Updated
Scrolledimplementations to include a header/footer presence check. - Introduced a
HasHeaderOrFooterhelper method in bothItemsViewDelegatorandItemsViewDelegator2. - Added a host app scenario and a shared UI test to verify scrolling on grouped empty collections.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs | Modified Scrolled early-return condition and added HasHeaderOrFooter helper |
| src/Controls/src/Core/Handlers/Items2/iOS/ItemsViewDelegator2.cs | Applied the same Scrolled update and helper in the Items2 delegator |
| src/Controls/tests/TestCases.HostApp/Issues/Issue30249.cs | Introduced a test page with a grouped CollectionView and AutomationIds for UI interaction |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue30249.cs | Added a UI test that waits for the collection, scrolls, and asserts the Scrolled event flag |
| return (VisibleItems, firstVisibleItemIndex, centerItemIndex, lastVisibleItemIndex); | ||
| } | ||
|
|
||
| bool HasHeaderOrFooter() |
There was a problem hiding this comment.
This helper method is duplicated in ItemsViewDelegator2; consider extracting it into a shared base class or utility to avoid code duplication.
| return (VisibleItems, firstVisibleItemIndex, centerItemIndex, lastVisibleItemIndex); | ||
| } | ||
|
|
||
| bool HasHeaderOrFooter() |
There was a problem hiding this comment.
Duplicate logic from the other delegator; extracting this into a common helper would improve maintainability.
| { | ||
| App.WaitForElement("CollectionViewWithEmptyGroups"); | ||
| App.ScrollDown("CollectionViewWithEmptyGroups", ScrollStrategy.Gesture, 0.2, 500); | ||
|
|
There was a problem hiding this comment.
[nitpick] Consider using App.WaitForElement("ScrolledEventStatusLabel") before accessing its text to reduce test flakiness on slower devices or CI environments.
| App.WaitForElement("ScrolledEventStatusLabel"); |
|
|
||
| scrolledEventStatusLabel = new Label | ||
| { | ||
| AutomationId = "ScrolledEventStatusLabel", |
There was a problem hiding this comment.
Ensure these AutomationIds are unique across the host app to prevent conflicts when calling WaitForElement in shared tests.
2a7e5c0 to
0658431
Compare
|
|
kubaflo
left a comment
There was a problem hiding this comment.
Could you please resolve conflicts?
|
Closing this PR since the issue has already been resolved on the main branch. PR #34153 removed the early if (!visibleItems) return; exit from the Scrolled method, ensuring the event is always fired and effectively fixing the bug. |
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 #30249
Validated the behaviour in the following platforms
Output
Before.mov
After.mov