[Android] refactoring fixes layouts

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue5766.cs

@@ -1,19 +1,36 @@
 using System;
-using System.Collections;
 using System.Linq;
-using System.Collections.ObjectModel;
-
 using Xamarin.Forms.CustomAttributes;
 using Xamarin.Forms.Internals;
-using System.Collections.Generic;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+using Xamarin.Forms.Core.UITests;
+#endif
 
 namespace Xamarin.Forms.Controls.Issues
 {
 	[Preserve (AllMembers=true)]
 	[Issue (IssueTracker.Github, 5766, "Frame size gets corrupted when ListView is scrolled", PlatformAffected.Android)]
-	public class Issue5766 : ContentPage
+#if UITEST
+	[NUnit.Framework.Category(UITestCategories.Layout)]
+#endif
+	public class Issue5766 : TestContentPage
 	{
-		public Issue5766()
+		const string StartText1 = "start1";
+		const string BigText1 = "big string > big frame1";
+		const string SmallText1 = "s1";
+		const string EndText1 = "end1";
+		const string List1 = "lst1";
+
+		const string StartText2 = "start2";
+		const string BigText2 = "big string > big frame2";
+		const string SmallText2 = "s2";
+		const string EndText2 = "end2";
+		const string List2 = "lst2";
+
+		protected override void Init()
 		{
 			var grid = new Grid
 			{
@@ -27,44 +44,118 @@ public Issue5766()
 			{
 				Text = "Scroll up and down several times and make sure Frame size is accurate when using Fast Renderers.",
 				VerticalTextAlignment = TextAlignment.Center
-			}, 0, 0);
+			}, 0, 0, 2);
+
+			var template = new DataTemplate(() =>
+			{
+				var text = new Label
+				{
+					VerticalOptions = LayoutOptions.Fill,
+					TextColor = Color.White,
+				};
+
+				text.SetBinding(Label.TextProperty, ".");
+				var view = new Grid
+				{
+					HeightRequest = 80,
+					Margin = new Thickness(0, 10, 0, 0),
+					BackgroundColor = Color.FromHex("#F1F1F1")
+				};
+				view.AddChild(new Frame
+				{
+					Padding = new Thickness(5),
+					Margin = new Thickness(0, 0, 10, 0),
+					BorderColor = Color.Blue,
+					BackgroundColor = Color.Gray,
+					VerticalOptions = LayoutOptions.Center,
+					HorizontalOptions = LayoutOptions.End,
+					CornerRadius = 3,
+					Content = text
+				}, 0, 0);
+				return new ViewCell
+				{
+					View = view
+				};
+			});
+
 			grid.AddChild(new ListView
 			{
+				AutomationId = List1,
 				HasUnevenRows = true,
-				ItemsSource = Enumerable.Range(0, 99).Select(i => i % 2 == 0 ? "small" : "big string > big frame"),
-				ItemTemplate = new DataTemplate(() =>
-				{
-					var text = new Label
-					{
-						VerticalOptions = LayoutOptions.Fill,
-						TextColor = Color.White
-					};
-					text.SetBinding(Label.TextProperty, ".");
-					var view = new Grid
-					{
-						HeightRequest = 200,
-						Margin = new Thickness(0, 10, 0, 0),
-						BackgroundColor = Color.FromHex("#F1F1F1")
-					};
-					view.AddChild(new Frame
-					{
-						Padding = new Thickness(5),
-						Margin = new Thickness(0, 0, 10, 0),
-						BorderColor = Color.Blue,
-						BackgroundColor = Color.Gray,
-						VerticalOptions = LayoutOptions.Center,
-						HorizontalOptions = LayoutOptions.End,
-						CornerRadius = 3,
-						Content = text
-					}, 0, 0);
-					return new ViewCell
-					{
-						View = view
-					};
-				})
+				ItemsSource = (new[] { StartText1 }).Concat(Enumerable.Range(0, 99).Select(i => i % 2 != 0 ? SmallText1 : BigText1)).Concat(new[] { EndText1 }),
+				ItemTemplate = template
 			}, 0, 1);
-
+			grid.AddChild(new ListView(ListViewCachingStrategy.RecycleElement)
+			{
+				AutomationId = List2,
+				HasUnevenRows = true,
+				ItemsSource = (new[] { StartText2 }).Concat(Enumerable.Range(0, 99).Select(i => i % 2 != 0 ? SmallText2 : BigText2)).Concat(new[] { EndText2 }),
+				ItemTemplate = template
+			}, 1, 1);
 			Content = grid;
 		}
+
+#if UITEST && __ANDROID__
+		UITest.Queries.AppRect[] GetLabels(IApp RunningApp, string label)
+		{
+			return RunningApp
+				.Query(q => q.Class("FormsTextView"))
+				.Where(x => x.Text == label)
+				.Select(x => x.Rect)
+				.ToArray();
+		}
+
+		bool RectIsEquals(UITest.Queries.AppRect[] left, UITest.Queries.AppRect[] right)
+		{
+			if (left.Length != right.Length)
+				return false;
+
+			for (int i = 0; i < left.Length; i++)
+			{
+				if (left[i].X != right[i].X || 
+					left[i].Y != right[i].Y || 
+					left[i].Width != right[i].Width || 
+					left[i].Height != right[i].Height)
+					return false;
+			}
+
+			return true;
+		}
+
+		[Test]
+		public void FrameSizeGetsCorruptedWhenListViewIsScrolled()
+		{
+			RunningApp.WaitForElement(StartText1);
+			var start = GetLabels(RunningApp, StartText1);
+			var smalls = GetLabels(RunningApp, SmallText1);
+			var bigs = GetLabels(RunningApp, BigText1);
+
+			RunningApp.ScrollDownTo(EndText1, List1, ScrollStrategy.Gesture, 0.9, 15000, timeout: TimeSpan.FromMinutes(1));
+			RunningApp.ScrollUpTo(StartText1, List1, ScrollStrategy.Gesture, 0.9, 15000, timeout: TimeSpan.FromMinutes(1));
+
+			var startAfter = GetLabels(RunningApp, StartText1);
+			Assert.IsTrue(RectIsEquals(start, startAfter));
+			var smallAfter = GetLabels(RunningApp, SmallText1);
+			Assert.IsTrue(RectIsEquals(smalls, smallAfter));
+			var bigAfter = GetLabels(RunningApp, BigText1);
+			Assert.IsTrue(RectIsEquals(bigs, bigAfter));
+
+			// list2 with ListViewCachingStrategy.RecycleElement - issue 6297
+			RunningApp.WaitForElement(StartText2);
+			start = GetLabels(RunningApp, StartText2);
+			smalls = GetLabels(RunningApp, SmallText2);
+			bigs = GetLabels(RunningApp, BigText2);
+
+			RunningApp.ScrollDownTo(EndText2, List2, ScrollStrategy.Gesture, 0.9, 15000, timeout: TimeSpan.FromMinutes(1));
+			RunningApp.ScrollUpTo(StartText2, List2, ScrollStrategy.Gesture, 0.9, 15000, timeout: TimeSpan.FromMinutes(1));
+
+			startAfter = GetLabels(RunningApp, StartText2);
+			Assert.IsTrue(RectIsEquals(start, startAfter));
+			smallAfter = GetLabels(RunningApp, SmallText2);
+			Assert.IsTrue(RectIsEquals(smalls, smallAfter));
+			bigAfter = GetLabels(RunningApp, BigText2);
+			Assert.IsTrue(RectIsEquals(bigs, bigAfter));
+		}
+#endif
 	}
 }

Xamarin.Forms.Platform.Android/ButtonLayoutManager.cs

@@ -90,10 +90,7 @@ internal SizeRequest GetDesiredSize(int widthConstraint, int heightConstraint)
 			// if the measure of the view has changed then trigger a request for layout
 			// if the measure hasn't changed then force a layout of the button
 			if (previousHeight != View.MeasuredHeight || previousWidth != View.MeasuredWidth)
-			{
-				if (!View.IsLayoutRequested)
-					View.RequestLayout();
-			}
+				View.MaybeRequestLayout();
 			else
 				View.ForceLayout();
 

Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs

@@ -117,6 +117,15 @@ SizeRequest IVisualElementRenderer.GetDesiredSize(int widthConstraint, int heigh
 			SizeRequest result = new SizeRequest(new Size(MeasuredWidth, MeasuredHeight), new Size());
 			result.Minimum = new Size(Math.Min(Context.ToPixels(10), result.Request.Width), result.Request.Height);
 
+			// if the measure of the view has changed then trigger a request for layout
+			// if the measure hasn't changed then force a layout of the label
+			var measureIsChanged = !_lastSizeRequest.HasValue ||
+				_lastSizeRequest.HasValue && (_lastSizeRequest.Value.Request.Height != MeasuredHeight || _lastSizeRequest.Value.Request.Width != MeasuredWidth);
+			if (measureIsChanged)
+				this.MaybeRequestLayout();
+			else
+				ForceLayout();
+
 			_lastConstraintWidth = widthConstraint;
 			_lastConstraintHeight = heightConstraint;
 			_lastSizeRequest = result;
@@ -210,6 +219,7 @@ protected virtual void OnElementChanged(ElementChangedEventArgs<Label> e)
 			if (e.OldElement != null)
 			{
 				e.OldElement.PropertyChanged -= OnElementPropertyChanged;
+				this.MaybeRequestLayout();
 			}
 
 			if (e.NewElement != null)

Xamarin.Forms.Platform.Android/Renderers/ScrollViewRenderer.cs

@@ -75,6 +75,8 @@ public void SetElement(VisualElement element)
 			if (oldElement != null)
 			{
 				oldElement.PropertyChanged -= HandlePropertyChanged;
+				oldElement.LayoutChanged -= HandleLayoutChanged;
+
 				((IScrollViewController)oldElement).ScrollToRequested -= OnScrollToRequested;
 			}
 			if (element != null)
@@ -88,6 +90,8 @@ public void SetElement(VisualElement element)
 				}
 
 				_view.PropertyChanged += HandlePropertyChanged;
+				_view.LayoutChanged += HandleLayoutChanged;
+
 				Controller.ScrollToRequested += OnScrollToRequested;
 
 				LoadContent();
@@ -107,6 +111,11 @@ public void SetElement(VisualElement element)
 			EffectUtilities.RegisterEffectControlProvider(this, oldElement, element);
 		}
 
+		void HandleLayoutChanged(object sender, EventArgs e)
+		{
+			UpdateLayout();
+		}
+
 		void UpdateFlowDirection()
 		{
 			if (Element is IVisualElementController controller)
@@ -127,8 +136,7 @@ void UpdateFlowDirection()
 
 		public void UpdateLayout()
 		{
-			if (Tracker != null)
-				Tracker.UpdateLayout();
+			Tracker?.UpdateLayout();
 		}
 
 		public ViewGroup ViewGroup => this;

Xamarin.Forms.Platform.Android/ViewExtensions.cs

@@ -93,5 +93,15 @@ public static void SetClipToOutline(this AView view, bool value)
 
 			view.ClipToOutline = value;
 		}
+
+		internal static void MaybeRequestLayout(this AView view)
+		{
+			var isInLayout = false;
+			if ((int)Build.VERSION.SdkInt >= 18)
+				isInLayout = view.IsInLayout;
+
+			if (!isInLayout && !view.IsLayoutRequested)
+				view.RequestLayout();
+		}
 	}
 }

Xamarin.Forms.Platform.Android/VisualElementRenderer.cs

@@ -241,9 +241,6 @@ public void SetElement(TElement element)
 			if (AutoTrack && Tracker == null)
 				SetTracker(new VisualElementTracker(this));
 
-			if (oldElement != null && element != null)
-				Tracker?.UpdateLayout();
-
 			if (element != null)
 				SendVisualElementInitialized(element, this);
 

Xamarin.Forms.Platform.Android/VisualElementTracker.cs

@@ -115,8 +115,6 @@ public void UpdateLayout()
 			//On Width or Height changes, the anchors needs to be updated
 			UpdateAnchorX();
 			UpdateAnchorY();
-
-			MaybeRequestLayout();
 		}
 
 		void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
@@ -150,7 +148,7 @@ void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
 
 			if (e.PropertyName == VisualElement.XProperty.PropertyName || e.PropertyName == VisualElement.YProperty.PropertyName || e.PropertyName == VisualElement.WidthProperty.PropertyName ||
 				e.PropertyName == VisualElement.HeightProperty.PropertyName)
-				MaybeRequestLayout();
+				_renderer.View.MaybeRequestLayout();
 			else if (e.PropertyName == VisualElement.AnchorXProperty.PropertyName)
 				UpdateAnchorX();
 			else if (e.PropertyName == VisualElement.AnchorYProperty.PropertyName)
@@ -184,7 +182,7 @@ void HandleRedrawNeeded(object sender, EventArg<VisualElement> e)
 			_batchedProperties.Clear();
 
 			if (_layoutNeeded)
-				MaybeRequestLayout();
+				_renderer.View.MaybeRequestLayout();
 			_layoutNeeded = false;
 		}
 
@@ -199,16 +197,6 @@ void HandleViewAttachedToWindow()
 			UpdateClipToBounds();
 		}
 
-		void MaybeRequestLayout()
-		{
-			var isInLayout = false;
-			if ((int)Build.VERSION.SdkInt >= 18)
-				isInLayout = _renderer.View.IsInLayout;
-
-			if (!isInLayout && !_renderer.View.IsLayoutRequested)
-				_renderer.View.RequestLayout();
-		}
-
 		void RendererOnElementChanged(object sender, VisualElementChangedEventArgs args)
 		{
 			SetElement(args.OldElement, args.NewElement);