[Android] Fix ScrollView filling layouts

[jknaudt21]

Problem

ScrollView controls would try to fill their parent container on Android even thought that wasn't the expected behavior. In cases where the MaximumHeight property was set, the ScrollView would grow to that size.

The problem was being caused by additional logic placed to make ScrollView fill an entire page (and thereby size child elements correctly) when it is the topmost element. More specifically, we would set the height's MeasureSpec to MeasureSpecMode.Exactly. This caused the layout to grow to exactly its largest constraint, i.e the MaximumHeight. For more context on why this was originally done, see #7514

Solution

We check if the ScrollView has a explicitly set width or height. If it does, we don't change the MeasureSpec. Otherwise, we use the old logic.

Before & After

    <VerticalStackLayout>
        <ScrollView MaximumHeightRequest="300" BackgroundColor="LightBlue">
            <Label Text="Hello world" />
        </ScrollView>
    </VerticalStackLayout>

Before	After

Issues Fixed

Fixes #16464

[rmarinho]

Does is look the same on iOS?

[hartez]

Does is look the same on iOS?

Yes.

[jknaudt21]

Adding a couple screenshots based on offline discussion:

ScrollView not contained in a layout (click me)

 <ScrollView BackgroundColor="LightBlue">
            <Label Text="Hello world" />
  </ScrollView>

ScrollView inside a grid (click me)

<Grid>
     <ScrollView BackgroundColor="LightBlue">
            <Label Text="Hello world" />
     </ScrollView>
</Grid>

Windows looks the same in these cases too

[mattleibow]

Is this something the visual tests need to confirm? OR is the added test enough?

[hartez]

With this layout:

    <Grid>
        <ScrollView Background="LightBlue"
                    MaximumHeightRequest="500"
                    VerticalOptions="Fill">
            <VerticalStackLayout Background="LightGreen"
                                 Margin="5">
                <Label Text="Hello" />
            </VerticalStackLayout>

        </ScrollView>
    </Grid>

The inner VSL should be filling up the ScrollView's viewport, because the ScrollView is set to Fill in the vertical direction. But it's not, when this PR is in effect, because the fill feature is being disabled by the check for a Layout as the ScrollView's parent.

[hartez]

Is this something the visual tests need to confirm? OR is the added test enough?

This should be verifiable with Device Tests, but the current implementation is wrong.

[hartez]

The flaw in the original logic (before this PR) is that it doesn't take into account the situation where the constraint is infinite (meaning the measurement should ignore Fill and just size to the content) but the MeasureSpec is AtMost because of the MaximumHeight (or MaximumWidth).

We just need to skip the FillViewPort logic entirely if the measurement constraint in the ScrollView's unconstrained direction is infinite.

Something like this:

if (platformView.FillViewport)
{
	/*	With FillViewport active, the Android ScrollView will measure the content at least once; if it is 
		smaller than the ScrollView's viewport, it measure a second time at the size of the viewport
		so that the content can properly fill the whole viewport. But it will only do this if the measurespec
		is set to Exactly. So if we want our ScrollView to Fill the space in the scroll direction, we need to
		adjust the MeasureSpec accordingly. If the ScrollView is not set to Fill, we can just leave the spec
		alone and the ScrollView will size to its content as usual. */

	/*	We have to make sure that the constraint in the ScrollView direction isn't infinity; in that case,
		even with Fill in effect, the content's size should be only what's required to display it. */

	var orientation = virtualView.Orientation;

	if (!double.IsInfinity(heightConstraint))
	{
		if (orientation == ScrollOrientation.Both || orientation == ScrollOrientation.Vertical)
		{
			heightSpec = AdjustSpecForAlignment(heightSpec, virtualView.VerticalLayoutAlignment);
		}
	}

	if (!double.IsInfinity(widthConstraint))
	{
		if (orientation == ScrollOrientation.Both || orientation == ScrollOrientation.Horizontal)
		{
			widthSpec = AdjustSpecForAlignment(widthSpec, virtualView.HorizontalLayoutAlignment);
		}
	}
}

[hartez]

Needs to account for the situation outlined in #17243 (comment)

[jknaudt21]

@hartez I addressed your feedback. Thanks for catching the error!

@mattleibow I added this additional test that catches the case that E.Z outlined in his comments.

Device tests are failing, I'll look into it.

[jknaudt21]

So the tests were failing because of off-by-one (or two) pixels in Android.

For some reason iOS and Mac render a layout whose height I specify to be 500 to be ~460, which is surprising. I made the test less strict since this PR doesn't touch any Apple code. This only happens on CI though; locally the tests pass...

[jknaudt21]

Windows test is failing. I'll investigate why