-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
ScrollView CrossPlatformMeasure - refactor #27085
base: main
Are you sure you want to change the base?
Conversation
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). |
|
||
if (double.IsInfinity(widthConstraint)) | ||
{ | ||
widthConstraint = result.Width; | ||
widthConstraint = contentSize.Width; |
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.
Do we want to delete both of these if
checks on Android and Windows?
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.
Without them AdjustForFill
doesn't work correctly
@kubaflo Does this PR potentially improve performance please? |
I hope so, but I didn't measure anything. The main idea was to uniform the code across the platforms |
Description of Change
Refactored the
CrossPlatformMeasure
method for ScrollViewHandler on Android and Windows, so that it is similar to the iOS one introduced by this PR #26763@PureWeen