-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix Customised CollectionView inherited from does not ScrollTo and display selection correctly #31549
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.
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
Conversation
Hey there @@SuthiYuvaraj! 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a Windows-specific issue where custom CollectionView classes that inherit from CollectionView fail to scroll to items correctly. The problem occurs because the Windows implementation uses reference equality checks instead of value equality when finding target items for scrolling.
- Updates the item comparison logic in Windows ItemsViewHandler to use
Equals()
instead of reference equality - Adds comprehensive UI test cases to verify scroll functionality works correctly in custom CollectionView implementations
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
src/Controls/src/Core/Handlers/Items/ItemsViewHandler.Windows.cs |
Changes item comparison from reference equality (== ) to value equality (Equals() ) in FindBoundItem method |
src/Controls/tests/TestCases.HostApp/Issues/Issue31351.cs |
Adds test page with custom CollectionView implementation and scroll test functionality |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31351.cs |
Adds automated UI tests to verify scroll behavior works correctly |
label.SetBinding(Label.TextProperty, nameof(Issue31351CollectionViewItem.Title)); | ||
label.Padding = new Thickness(10); | ||
label.BackgroundColor = Colors.White; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Label element lacks an AutomationId which is required for UI test automation. According to the guidelines, AutomationIds should be UNIQUE and are necessary for WaitForElement to find the correct element in tests.
Copilot uses AI. Check for mistakes.
App.WaitForElement("Issue31351CollectionView"); | ||
App.WaitForElement("Issue31351ScrollButton"); | ||
App.Tap("Issue31351ScrollButton"); | ||
var MidItemRect = App.WaitForElement("Item 50").GetRect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test relies on finding elements by text content ('Item 50') rather than AutomationId. This approach is brittle because it depends on the specific text generation logic. The corresponding UI elements in the HostApp should have unique AutomationIds for more reliable test automation.
Copilot uses AI. Check for mistakes.
Assert.That(MidItemRect.X, Is.GreaterThanOrEqualTo(0)); | ||
App.WaitForElement("Issue31351TopScrollButton"); | ||
App.Tap("Issue31351TopScrollButton"); | ||
var TopItemRect = App.WaitForElement("Item 1").GetRect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the previous issue, this test relies on text content ('Item 1') to locate elements instead of using AutomationIds. This makes the test fragile and dependent on the exact text formatting.
Copilot uses AI. Check for mistakes.
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 Description
When creating a custom class that inherits from CollectionView, calling ScrollTo works as expected on Android and iOS, but it does nothing on WinUI.
Description of Change
On Windows, the method FindBoundItem is used to locate the target item before scrolling. However, this method checks the item by reference, which causes the issue: even though the items are logically the same, the reference check fails, preventing scrolling.
To fix this, I updated the condition to match the behavior on Android and iOS, ensuring that the item can be correctly identified and scrolled to.
Issues Fixed
Fixes #31351
Tested the existing UI and DeviceTest in the following platforms
Output Screenshot:
BeforeScroll.mp4
AfterScroll.mp4