Fix 15066

[heikow10]

Description of Change

fix #15066
revert the commit e8d06f357eae3021a31272db272d4873356870b6 which introduced this bug

Issues Resolved

• fixes [Bug] StackLayout's layout inside ScrollView is not updated properly when adding children #15066

API Changes

None

Platforms Affected

• Core/XAML (all platforms)
• iOS
• Android
• UWP

Behavioral/Visual Changes

The layout of a StackLayout or a Grid which is inside a ScrollView is updated properly when child elements are added to the StackLayout/Grid.

Before/After Screenshots

Testing Procedure

Android: description can be seen in Issue15066.cs
iOS: not able to test this
UWP: bug was not present before this fix

PR Checklist

• Targets the correct branch
• Tests are passing (or failures are unrelated)

[net-foundation-cla]

All CLA requirements met.

[jfversluis]

Hey @heikow10 thanks so much for your bug report and even a fix! Much appreciated! Not sure if reverting this is the right course to go, ideally we want whatever the initial fix did and a fix for this problem :) but for now I'll go ahead and create a build from it. There are a couple of more issues like these. If it turns out that this fixes a number of those I might decide otherwise.

[jfversluis]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[heikow10]

Hi @jfversluis, thanks for your answer! Of course, I would also prefer a solution which keeps the initial optimization and fixes the issue. I did not really understand what this optimization has done because it was my first look at the Xamarin.Forms repository/source code. So reverting this was the only way to fix this issue, which I would need in one of my apps to upgrade to Xamarin.Forms 5.0.0. And as I have already mentioned in the issue description there may be more reported issues which are caused by the optimization.

[jfversluis]

I'll check with the author and see what the potential implications are. The build has completed, would you be able to grab the NuGet as described here and let us know if this actually fixes this issue? That will greatly speed up the review process.

Besides verifying if this particular issue is fixed also be sure to check other scenarios in the same area to make sure that this fix doesn't accidentally has side-effects 🙂

Thanks!

[heikow10]

I grab the build and I did some tests.
On Android the issue is fixed with this build. With UWP it still works fine. And I am not able to test with iOS.
During testing I did not see any new problems with this build.

[jfversluis]

Thanks so much! I've pinged the author of the initial PR and trying to get some people of potentially related issues to test this as well to see how it goes and determine a path forward with this.

[jfversluis]

Hey @heikow10 EZ has come up with the fix in #15076 that should have both the improvements and fix the bug. Would you maybe be willing to give that one a shot as well?

[heikow10]

Hey @jfversluis, @hartez! I have tested the PR #15076 and it fixes my problem on Android.

But I am not able to test on iOS. So I don't know if this issue is present on iOS at all. But it would not be fixed with this PR. Maybe this should be checked because the iOS ScrollViewRenderer does not inherit from VisualElementRenderer, too.

[jfversluis]

Closing this in favor of #15076

[jfversluis]

Thank you @heikow10 so much for your time and effort in helping diagnose and fix this!

[heikow10]

I am glad that this bug could be fixed. @jfversluis Do you have any info if this bug exists on iOS? I am not able to build for iOS so I could not test it.

[jfversluis]

I haven't seen any reports about it for iOS and we're pretty sure that this works as intended on iOS :)

[heikow10]

I haven't seen any reports about it for iOS and we're pretty sure that this works as intended on iOS :)

Ok, that's fine!