-
Notifications
You must be signed in to change notification settings - Fork 1.9k
MauiScrollView resets ContentOffset on first layout pass - fix #30453
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
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 prevents MauiScrollView
on iOS from inadvertently resetting its scroll position on the very first layout pass by adding an explicit check against _previousEffectiveUserInterfaceLayoutDirection
.
- Introduces a guard so that on the initial layout pass (when
_previousEffectiveUserInterfaceLayoutDirection
isnull
),ContentOffset
isn’t reset. - Ensures that on subsequent layout passes (when the layout direction is known), the old scroll offset logic still applies.
Comments suppressed due to low confidence (1)
src/Core/src/Platform/iOS/MauiScrollView.cs:67
- Consider adding a unit test to verify that on the initial layout pass (when
_previousEffectiveUserInterfaceLayoutDirection
is null) theContentOffset
remains unchanged, and that on subsequent passes it resets correctly.
else if (_previousEffectiveUserInterfaceLayoutDirection is not null)
Hey there @@kubaflo! 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 |
Azure Pipelines successfully started running 3 pipeline(s). |
@@ -64,7 +64,7 @@ public override void LayoutSubviews() | |||
crossPlatformLayout.CrossPlatformArrange(new Rect(new Point(-horizontalOffset, 0), crossPlatformBounds)); | |||
ContentOffset = new CGPoint(horizontalOffset, 0); | |||
} | |||
else | |||
else if (_previousEffectiveUserInterfaceLayoutDirection is not null) |
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.
@mjo151 If you try this, could you share a repro sample? Would like to include a related test.
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.
I tested the build from this PR and it appears to fix the issue. I will see if I can create a simple repo sample.
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
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!
Description of Change
@mjo151 could you please verify if this PR fixes your issue, since you did not provide your repro so that I can test it
https://github.com/dotnet/maui/wiki/Testing-PR-Builds
Issues Fixed
Fixes #30147