-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Android] - Fix KeepScrollOffset Behavior During Dynamic Item Additions in CollectionView #29255
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
base: main
Are you sure you want to change the base?
Conversation
Hey there @@prakashKannanSf3972! 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.
The test KeepScrollOffset
is failing on Android:
at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2420
at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2437
at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 680
at Microsoft.Maui.TestCases.Tests.Issues.CollectionViewItemsUpdatingScrollModeUITests.KeepScrollOffset() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CollectionViewUITests.CollectionViewItemsUpdatingScrollMode.cs:line 51
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
@jsuarezruiz, Since the existing test logic did not align with the expected |
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
{ | ||
items = new ObservableCollection<string>(Enumerable.Range(1, 30).Select(i => $"Item {i}")); | ||
|
||
Button keepScrollOffsetButton = CreateButton("KeepScrollOffset", "KeepScrollOffsetButton", OnKeepScrollOffsetClicked); |
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.
Could include more Buttons to change the ItemsUpdatingScrollMode value https://github.com/dotnet/maui/blob/main/src/Controls/src/Core/Items/ItemsUpdatingScrollMode.cs#L8
and test the behavior with the different possibilities?
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.
Yes, adding tests for the various ItemsUpdatingScrollMode
values would provide broader coverage. However,I would like to highlight a couple of existing platform-specific issues currently impacting these modes:
On Android: The KeepItemsInView
mode currently does not work as expected, which is a known issue (#29145). This is being addressed in PR #27153, which is still under review.
On iOS : KeepLastItemInView
is also not functioning correctly. This is another known issue (28716), which is also addressed in the same PR currently under review: PR #28720
Given these inconsistencies, only the KeepScrollOffset
mode behaves consistently across all platforms at this time. This PR focuses on verifying that stable behavior.
Since the other two PRs handles the KeepItemsInView
mode and KeepLastItemInView
mode can we avoid adding tests for those modes?
Looking for your insights.
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
KeepScrollOffset
mode does not maintain scroll position when new items are inserted into the CollectionView, resulting in unintended upward scrolling.Root Cause
When new items are added to the collection, the scroll offset is recalculated based on the updated layout. This results in incorrect offset adjustments, causing the scroll position to shift upward instead of being correctly preserved.
Description of Change
AddScrollListener
andRemoveScrollListener
methods to manage scroll listeners dynamically. Adjusted theTrackOffsets
method to ensure scroll offsets are only adjusted when the first item is reached, ensuring accurate scroll behavior and preventing unintended upward scrolling during dynamic item insertions.Issues Fixed
Fixes #29131
KeepItemsInView
issue on Android.Tested the behaviour in the following platforms
Output
BeforeFix_ScrollOffset.mov
AfterFix_ScrollOffset.mov