-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Conversation
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. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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. |
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! |
I grab the build and I did some tests. |
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. |
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. |
Closing this in favor of #15076 |
Thank you @heikow10 so much for your time and effort in helping diagnose and fix this! |
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. |
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! |
Description of Change
fix #15066
revert the commit e8d06f357eae3021a31272db272d4873356870b6 which introduced this bug
Issues Resolved
API Changes
None
Platforms Affected
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