From 8f85f2d8a580a7d8ce28f7f7c5e590f3893b3365 Mon Sep 17 00:00:00 2001 From: Pavel Yakovlev Date: Thu, 18 Jul 2019 04:29:08 +0300 Subject: [PATCH] [Android] refactoring fixes layouts (#6390) * [Android] refactoring fixes layouts (issues 1332, 1760, 5766, 5184, 6297) * issue3000 and ToolbarItem tests should be passes * MaybeRequestLayout extension method * added automated UITest 5766 * fix 5766 * fix 6297 * merge 6297 into 5766 uitest * rename test --- .../Issue5766.cs | 169 ++++++++++++++++----- .../ButtonLayoutManager.cs | 5 +- .../FastRenderers/LabelRenderer.cs | 10 ++ .../Renderers/ScrollViewRenderer.cs | 12 +- Xamarin.Forms.Platform.Android/ViewExtensions.cs | 10 ++ .../VisualElementRenderer.cs | 3 - .../VisualElementTracker.cs | 16 +- 7 files changed, 163 insertions(+), 62 deletions(-) diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue5766.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue5766.cs index 9e8972d..ee82f4e 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue5766.cs +++ b/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 @@ namespace Xamarin.Forms.Controls.Issues { 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 } } diff --git a/Xamarin.Forms.Platform.Android/ButtonLayoutManager.cs b/Xamarin.Forms.Platform.Android/ButtonLayoutManager.cs index 279d579..caa25f5 100644 --- a/Xamarin.Forms.Platform.Android/ButtonLayoutManager.cs +++ b/Xamarin.Forms.Platform.Android/ButtonLayoutManager.cs @@ -90,10 +90,7 @@ namespace Xamarin.Forms.Platform.Android // 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(); diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs b/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs index 89acfcd..c40aab2 100644 --- a/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs +++ b/Xamarin.Forms.Platform.Android/FastRenderers/LabelRenderer.cs @@ -117,6 +117,15 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers 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 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers if (e.OldElement != null) { e.OldElement.PropertyChanged -= OnElementPropertyChanged; + this.MaybeRequestLayout(); } if (e.NewElement != null) diff --git a/Xamarin.Forms.Platform.Android/Renderers/ScrollViewRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/ScrollViewRenderer.cs index 0d9cdbd..56d7360 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/ScrollViewRenderer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/ScrollViewRenderer.cs @@ -75,6 +75,8 @@ namespace Xamarin.Forms.Platform.Android if (oldElement != null) { oldElement.PropertyChanged -= HandlePropertyChanged; + oldElement.LayoutChanged -= HandleLayoutChanged; + ((IScrollViewController)oldElement).ScrollToRequested -= OnScrollToRequested; } if (element != null) @@ -88,6 +90,8 @@ namespace Xamarin.Forms.Platform.Android } _view.PropertyChanged += HandlePropertyChanged; + _view.LayoutChanged += HandleLayoutChanged; + Controller.ScrollToRequested += OnScrollToRequested; LoadContent(); @@ -107,6 +111,11 @@ namespace Xamarin.Forms.Platform.Android EffectUtilities.RegisterEffectControlProvider(this, oldElement, element); } + void HandleLayoutChanged(object sender, EventArgs e) + { + UpdateLayout(); + } + void UpdateFlowDirection() { if (Element is IVisualElementController controller) @@ -127,8 +136,7 @@ namespace Xamarin.Forms.Platform.Android public void UpdateLayout() { - if (Tracker != null) - Tracker.UpdateLayout(); + Tracker?.UpdateLayout(); } public ViewGroup ViewGroup => this; diff --git a/Xamarin.Forms.Platform.Android/ViewExtensions.cs b/Xamarin.Forms.Platform.Android/ViewExtensions.cs index ed540e5..1134377 100644 --- a/Xamarin.Forms.Platform.Android/ViewExtensions.cs +++ b/Xamarin.Forms.Platform.Android/ViewExtensions.cs @@ -93,5 +93,15 @@ namespace Xamarin.Forms.Platform.Android 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(); + } } } \ No newline at end of file diff --git a/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs b/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs index ce127b3..bddbd6d 100644 --- a/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs +++ b/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs @@ -241,9 +241,6 @@ namespace Xamarin.Forms.Platform.Android if (AutoTrack && Tracker == null) SetTracker(new VisualElementTracker(this)); - if (oldElement != null && element != null) - Tracker?.UpdateLayout(); - if (element != null) SendVisualElementInitialized(element, this); diff --git a/Xamarin.Forms.Platform.Android/VisualElementTracker.cs b/Xamarin.Forms.Platform.Android/VisualElementTracker.cs index a909cec..c406b93 100644 --- a/Xamarin.Forms.Platform.Android/VisualElementTracker.cs +++ b/Xamarin.Forms.Platform.Android/VisualElementTracker.cs @@ -115,8 +115,6 @@ namespace Xamarin.Forms.Platform.Android //On Width or Height changes, the anchors needs to be updated UpdateAnchorX(); UpdateAnchorY(); - - MaybeRequestLayout(); } void HandlePropertyChanged(object sender, PropertyChangedEventArgs e) @@ -150,7 +148,7 @@ namespace Xamarin.Forms.Platform.Android 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 @@ namespace Xamarin.Forms.Platform.Android _batchedProperties.Clear(); if (_layoutNeeded) - MaybeRequestLayout(); + _renderer.View.MaybeRequestLayout(); _layoutNeeded = false; } @@ -199,16 +197,6 @@ namespace Xamarin.Forms.Platform.Android 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); -- 2.7.4