Fix Android ScrollView to Measure Content Correct #30964
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes Android ScrollView content measurement by removing a redundant layout architecture and implementing proper constraint handling based on scroll orientation. The changes address issue #30951 where ScrollView content was not being measured correctly on Android.
Key changes:
- Removes the
CrossPlatformArrangedelegation pattern fromMauiScrollViewtoScrollViewHandler - Implements scroll-orientation-aware constraint calculation in the measurement logic
- Sets
FillViewport = trueon horizontal scroll views to ensure proper content sizing
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Core/src/Platform/Android/MauiScrollView.cs | Removes CrossPlatformArrange property and its usage in OnLayout, sets FillViewport = true for horizontal scroll views |
| src/Core/src/Layouts/LayoutExtensions.cs | Adds overloaded MeasureContent method with constraint parameters for content view measurement |
| src/Core/src/Handlers/ScrollView/ScrollViewHandler.Android.cs | Removes CrossPlatformArrange delegation, implements scroll-orientation-aware measurement logic |
| src/Controls/src/Core/ScrollView/ScrollView.cs | Minor code formatting change to method signature |
Comments suppressed due to low confidence (2)
src/Core/src/Layouts/LayoutExtensions.cs:170
- [nitpick] The method parameter names
constrainPresentedContentWidthToExplicitDimsOnContentViewandconstrainPresentedContentHeightToExplicitDimsOnContentVieware excessively long and unclear. Consider shorter, more descriptive names likerespectExplicitWidthandrespectExplicitHeight.
internal static Size MeasureContent(
src/Core/src/Handlers/ScrollView/ScrollViewHandler.Android.cs:265
- The null-conditional operator change in
CrossPlatformArrangeintroduces new behavior that should be tested. This change could affect layout behavior whenVirtualViewis null, and should have corresponding test coverage in TestCases.Shared.Tests to verify the Size.Zero return value.
Size ICrossPlatformLayout.CrossPlatformArrange(Rect bounds) =>
rmarinho
reviewed
Aug 1, 2025
| Size ICrossPlatformLayout.CrossPlatformMeasure(double widthConstraint, double heightConstraint) | ||
| { | ||
| var scrollView = VirtualView; | ||
| if (VirtualView is not { } scrollView) |
There was a problem hiding this comment.
This fires before we set a VirtualView?
There was a problem hiding this comment.
no, we can definitely remove this check
Let's see if it passes tests and if so I'll remove
rmarinho
approved these changes
Aug 1, 2025
github-project-automation
bot
moved this from Ready To Review
to Approved
in MAUI SDK Ongoing
|
PureWeen
added a commit
that referenced
this pull request
Aug 3, 2025
### Description of Change This PR is a re-application of #30964 to make sure that @kubaflo is added as a co-author This pull request introduces several updates to the `ScrollView` and its related components to improve layout handling, measurement logic, and platform-specific implementation for Android. The changes focus on simplifying code, enhancing functionality, and addressing edge cases in layout and measurement. ### Enhancements to measurement and layout logic: - **Refactored `ICrossPlatformLayout.CrossPlatformMeasure` and `CrossPlatformArrange`:** Improved measurement logic by introducing explicit constraints based on scroll orientation and utilizing the new `MeasureContent` method for more accurate content measurement. Added null checks and default values to prevent errors during layout operations. (`[[1]](diffhunk://#diff-d3dcb79b77b66e99d195c7e8406a19b95224604786bab4e82509c4acd3913735L246-R266)`, `[[2]](diffhunk://#diff-d3dcb79b77b66e99d195c7e8406a19b95224604786bab4e82509c4acd3913735L237-R238)`) - **Introduced `MeasureContent` method in `LayoutExtensions`:** Added a new internal method to handle content measurement with support for explicit dimensions and padding adjustments, improving modularity and reusability. (`[src/Core/src/Layouts/LayoutExtensions.csR170-R200](diffhunk://#diff-923a4005b8fd76c6fe5a26814219f9002063666b6d6a49b6b1e19aea086628f4R170-R200)`) ### Platform-specific updates for Android: - **Updated `MauiHorizontalScrollView` initialization:** Enabled the `FillViewport` property to ensure the scroll view fills its viewport, enhancing usability for horizontal scrolling. (`[src/Core/src/Platform/Android/MauiScrollView.csL100-R104](diffhunk://#diff-546ef8b25e238ce6a1820c920cb327471d6481eefc486eae39b3e5a31b45d907L100-R104)`) - **Removed unused `CrossPlatformArrange` logic:** Simplified the layout handling in `MauiScrollView` by removing the `CrossPlatformArrange` property and related code, which was no longer required. (`[[1]](diffhunk://#diff-546ef8b25e238ce6a1820c920cb327471d6481eefc486eae39b3e5a31b45d907L232-L240)`, `[[2]](diffhunk://#diff-546ef8b25e238ce6a1820c920cb327471d6481eefc486eae39b3e5a31b45d907L324-L325)`) ### Code cleanup and refactoring: - **Improved readability in `ScrollView` and `ScrollViewHandler.Android`:** Adjusted formatting and removed redundant code to enhance clarity and maintainability. (`[[1]](diffhunk://#diff-7c61a3f8442182cb2e772edaa805382c721965440aa3b0244462d1aae6c4ee4dL518-R519)`, `[[2]](diffhunk://#diff-d3dcb79b77b66e99d195c7e8406a19b95224604786bab4e82509c4acd3913735L28-L35)`) ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #30951 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> Co-authored-by: kubaflo <42434498+kubaflo@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#30997