Fixed IndicatorView issue resetting the template when data updates (#8709)

* Fixed IndicatorView template resets when data updates

* Fixed failing UITests

* Undo unnecessary changes in the ControlGallery Android csproj

* Removed unnecessary changes in the ControlGallery Android csproj

* Fix wrong ControlGallery Android csproj

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

@@ -0,0 +1,179 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.Forms.Core.UITests;
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+#if UITEST
+	[Category(UITestCategories.IndicatorView)]
+#endif
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 8693, "[Bug] IndicatorView template resets when data updates", PlatformAffected.All)]
+	public class Issue8693 : TestContentPage
+	{
+		public Issue8693()
+		{
+			Title = "Issue 8693";
+			BindingContext = new Issue8693ViewModel();
+		}
+
+		protected override void Init()
+		{
+#if APP
+			Device.SetFlags(new List<string>(Device.Flags ?? new List<string>()) { "IndicatorView_Experimental" });
+#endif
+			var layout = new StackLayout();
+
+			var instructions = new Label
+			{
+				Margin = new Thickness(6),
+				BackgroundColor = Color.Black,
+				TextColor = Color.White,
+				Text = "Press the Button to update the ItemsSource of the CarouselView below. After updating, verify that the IndicatorView is still visible. If it is visible, the test has passed."
+			};
+
+			var updateButton = new Button
+			{
+				Text = "Update ItemsSource"
+			};
+
+			var itemsLayout =
+				new LinearItemsLayout(ItemsLayoutOrientation.Horizontal)
+				{
+					SnapPointsType = SnapPointsType.MandatorySingle,
+					SnapPointsAlignment = SnapPointsAlignment.Center
+				};
+
+			var carouselView = new CarouselView
+			{
+				ItemsLayout = itemsLayout,
+				ItemTemplate = GetCarouselTemplate()
+			};
+
+			carouselView.SetBinding(ItemsView.ItemsSourceProperty, "Items");
+
+			var indicatorView = new IndicatorView
+			{
+				IndicatorColor = Color.Red,
+				SelectedIndicatorColor = Color.Green,
+				IndicatorTemplate = GetIndicatorTemplate(),
+				HorizontalOptions = LayoutOptions.Center,
+				Margin = new Thickness(0, 0, 0, 24)
+			};
+
+			IndicatorView.SetItemsSourceBy(indicatorView, carouselView);
+
+			layout.Children.Add(instructions);
+			layout.Children.Add(updateButton);
+			layout.Children.Add(carouselView);
+			layout.Children.Add(indicatorView);
+
+			Content = layout;
+
+			updateButton.Clicked += (sender, args) =>
+			{
+				var vm = (Issue8693ViewModel)BindingContext;
+				vm.LoadItems();
+			};
+		}
+
+		internal DataTemplate GetCarouselTemplate()
+		{
+			return new DataTemplate(() =>
+			{
+				var grid = new Grid();
+
+				var info = new Label
+				{
+					HorizontalOptions = LayoutOptions.Center,
+					VerticalOptions = LayoutOptions.Center,
+					Margin = new Thickness(6)
+				};
+
+				info.SetBinding(Label.TextProperty, new Binding("Name"));
+
+				grid.Children.Add(info);
+
+				var frame = new Frame
+				{
+					Content = grid,
+					HasShadow = false
+				};
+
+				frame.SetBinding(BackgroundColorProperty, new Binding("Color"));
+
+				return frame;
+			});
+		}
+
+		internal DataTemplate GetIndicatorTemplate()
+		{
+			return new DataTemplate(() =>
+			{
+				var grid = new Grid
+				{
+					HeightRequest = 6,
+					WidthRequest = 50
+				};
+
+				return grid;
+			});
+		}
+	}
+
+	[Preserve(AllMembers = true)]
+	public class Issue8693Model
+	{
+		public Color Color { get; set; }
+		public string Name { get; set; }
+	}
+
+	[Preserve(AllMembers = true)]
+	public class Issue8693ViewModel : BindableObject
+	{
+		ObservableCollection<Issue8693Model> _items;
+
+		public Issue8693ViewModel()
+		{
+			LoadItems();
+		}
+
+		public ObservableCollection<Issue8693Model> Items
+		{
+			get { return _items; }
+			set
+			{
+				_items = value;
+				OnPropertyChanged();
+			}
+		}
+
+		public void LoadItems()
+		{
+			Items = new ObservableCollection<Issue8693Model>();
+
+			var random = new Random();
+			var items = new List<Issue8693Model>();
+
+			for (int n = 0; n < 5; n++)
+			{
+				items.Add(new Issue8693Model
+				{
+					Color = Color.FromRgb(random.Next(0, 255), random.Next(0, 255), random.Next(0, 255)),
+					Name = $"{n + 1}"
+				});
+			}
+
+			_items = new ObservableCollection<Issue8693Model>(items);
+			OnPropertyChanged(nameof(Items));
+		}
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -1173,6 +1173,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue8417.xaml.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue8647.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue7510.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue8693.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue7813.xaml.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue8638.xaml.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue8672.cs" />

Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/CarouselViewGalleries/CarouselViewGallery.cs

@@ -14,8 +14,8 @@ namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.CarouselVi
 
 			var button = new Button
 			{
-				Text = "Enable CarouselView",
-				AutomationId = "EnableCarouselView"
+				Text = "Enable IndicatorView",
+				AutomationId = "EnableIndicatorView"
 			};
 			button.Clicked += ButtonClicked;
 
@@ -54,7 +54,7 @@ namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.CarouselVi
 		{
 			var button = sender as Button;
 
-			button.Text = "CarouselView Enabled!";
+			button.Text = "IndicatorView Enabled!";
 			button.TextColor = Color.Black;
 			button.IsEnabled = false;
 

Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/CarouselViewGalleries/IndicatorCodeGallery.cs

@@ -23,7 +23,6 @@ namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.CarouselVi
 					new RowDefinition { Height = GridLength.Auto },
 					new RowDefinition { Height = GridLength.Auto },
 					new RowDefinition { Height = GridLength.Auto },
-					new RowDefinition { Height = GridLength.Auto },
 					new RowDefinition { Height = GridLength.Star },
 					new RowDefinition { Height = GridLength.Auto }
 				}
@@ -56,7 +55,7 @@ namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.CarouselVi
 			var indicatorView = new IndicatorView
 			{
 				HorizontalOptions = LayoutOptions.Center,
-				Margin = new Thickness(12, 6, 12, 24),
+				Margin = new Thickness(12, 6, 12, 12),
 				IndicatorColor = Color.Gray,
 				SelectedIndicatorColor = Color.Black,
 				IndicatorsShape = IndicatorShape.Square,
@@ -67,26 +66,6 @@ namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.CarouselVi
 
 			layout.Children.Add(indicatorView);
 
-			var stckMaxVisible = new StackLayout { Orientation = StackOrientation.Horizontal };
-			stckMaxVisible.Children.Add(new Label { VerticalOptions = LayoutOptions.Center, Text = "MaximumVisible" });
-			var maxVisibleSlider = new Slider
-			{
-				Maximum = nItems,
-				Minimum = 0,
-				Value = nItems,
-				WidthRequest = 150,
-				BackgroundColor = Color.Pink
-			};
-			stckMaxVisible.Children.Add(maxVisibleSlider);
-
-			maxVisibleSlider.ValueChanged += (s, e) =>
-			{
-				var maximumVisible = (int)maxVisibleSlider.Value;
-				indicatorView.MaximumVisible = maximumVisible;
-			};
-
-			layout.Children.Add(stckMaxVisible);
-
 			var stckColors = new StackLayout { Orientation = StackOrientation.Horizontal };
 			stckColors.Children.Add(new Label { VerticalOptions = LayoutOptions.Center, Text = "IndicatorColor" });
 
@@ -170,18 +149,12 @@ namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.CarouselVi
 			layout.Children.Add(stckTemplate);
 
 			Grid.SetRow(generator, 0);
-			Grid.SetRow(stckMaxVisible, 1);
-			Grid.SetRow(stckColors, 2);
-			Grid.SetRow(stckTemplate, 3);
-			Grid.SetRow(carouselView, 4);
-			Grid.SetRow(indicatorView, 5);
+			Grid.SetRow(stckColors, 1);
+			Grid.SetRow(stckTemplate, 2);
+			Grid.SetRow(carouselView, 3);
+			Grid.SetRow(indicatorView, 4);
 
 			Content = layout;
-
-			generator.CollectionChanged += (sender, e) =>
-			{
-				maxVisibleSlider.Maximum = generator.Count;
-			};
 		}
 	}
 }

Xamarin.Forms.Core.UITests.Shared/Tests/CarouselViewUITests.cs

@@ -1,13 +1,11 @@
 using System.Linq;
 using NUnit.Framework;
-using Xamarin.UITest;
 
 namespace Xamarin.Forms.Core.UITests
 {
 	[Category(UITestCategories.CarouselView)]
 	internal class CarouselViewUITests : BaseTestFixture
 	{
-		string _enableControl = "Enable CarouselView";
 		string _carouselViewGalleries = "CarouselView Galleries";
 
 		protected override void NavigateToGallery()
@@ -16,8 +14,6 @@ namespace Xamarin.Forms.Core.UITests
 
 			App.WaitForElement(_carouselViewGalleries);
 			App.Tap(_carouselViewGalleries);
-			App.WaitForElement(_enableControl);
-			App.Tap(_enableControl);
 		}
 
 		[TestCase("CarouselView (Code, Horizontal)")]

Xamarin.Forms.Core.UITests.Shared/UITestCategories.cs

@@ -56,5 +56,6 @@
 		public const string Page = "Page";
 		public const string RefreshView = "RefreshView";
 		public const string TitleView = "TitleView";
+		public const string IndicatorView = "IndicatorView";
 	}
 }

Xamarin.Forms.Core/IndicatorView.cs

@@ -1,7 +1,5 @@
 using System.Collections;
-using System.Collections.Generic;
 using System.Collections.Specialized;
-using System.Runtime.CompilerServices;
 using Xamarin.Forms.Platform;
 
 namespace Xamarin.Forms
@@ -21,8 +19,8 @@ namespace Xamarin.Forms
 
 		public static readonly BindableProperty MaximumVisibleProperty = BindableProperty.Create(nameof(MaximumVisible), typeof(int), typeof(IndicatorView), int.MaxValue);
 
-		public static readonly BindableProperty IndicatorTemplateProperty = BindableProperty.Create(nameof(IndicatorTemplate), typeof(DataTemplate), typeof(IndicatorView), default(DataTemplate), propertyChanged: (bindable, oldValue, newValue)
-			=> UpdateIndicatorLayout(((IndicatorView)bindable), newValue));
+		public static readonly BindableProperty IndicatorTemplateProperty = BindableProperty.Create(nameof(IndicatorTemplate), typeof(DataTemplate), typeof(IndicatorView), default(DataTemplate), propertyChanging: (bindable, oldValue, newValue)
+			=> UpdateIndicatorLayout((IndicatorView)bindable, newValue));
 
 		public static readonly BindableProperty HideSingleProperty = BindableProperty.Create(nameof(HideSingle), typeof(bool), typeof(IndicatorView), true);
 
@@ -192,6 +190,5 @@ namespace Xamarin.Forms
 			}
 			Count = count;
 		}
-
 	}
 }

Xamarin.Forms.Platform.Android/CollectionView/IndicatorViewRenderer.cs

@@ -15,7 +15,7 @@ namespace Xamarin.Forms.Platform.Android
 	public class IndicatorViewRenderer : LinearLayout, IVisualElementRenderer, IViewRenderer, ITabStop
 	{
 		VisualElementTracker _visualElementTracker;
-		VisualElementRenderer _visualElementRenderer;
+		readonly VisualElementRenderer _visualElementRenderer;
 		const int DefaultPadding = 4;
 
 		void IViewRenderer.MeasureExactly()
@@ -30,7 +30,7 @@ namespace Xamarin.Forms.Platform.Android
 		bool _disposed;
 		int _selectedIndex = 0;
 		AColor _currentPageIndicatorTintColor;
-		ShapeType _shapeType = ShapeType.Oval;
+		AShapeType _shapeType = AShapeType.Oval;
 		Drawable _currentPageShape = null;
 		Drawable _pageShape = null;
 		AColor _pageIndicatorTintColor;
@@ -46,15 +46,11 @@ namespace Xamarin.Forms.Platform.Android
 
 		public event EventHandler<VisualElementChangedEventArgs> ElementChanged;
 
-
 		public event EventHandler<PropertyChangedEventArgs> ElementPropertyChanged;
 
-
 		public IndicatorViewRenderer(Context context) : base(context)
 		{
-			//ExperimentalFlags.VerifyFlagEnabled(nameof(IndicatorViewRenderer), ExperimentalFlags.IndicatorViewExperimental);
 			_visualElementRenderer = new VisualElementRenderer(this);
-
 		}
 
 		SizeRequest IVisualElementRenderer.GetDesiredSize(int widthConstraint, int heightConstraint)
@@ -136,24 +132,16 @@ namespace Xamarin.Forms.Platform.Android
 
 		}
 
-		void UpdateControl()
-		{
-			if (IndicatorsView.IndicatorTemplate != null)
-			{
-				var control = IndicatorsView.IndicatorLayout.GetRenderer() ?? Platform.CreateRendererWithContext(IndicatorsView.IndicatorLayout, Context);
-				AddView(control as AView);
-			}
-		}
-
 		protected virtual void OnElementPropertyChanged(object sender, PropertyChangedEventArgs changedProperty)
 		{
 			ElementPropertyChanged?.Invoke(this, changedProperty);
 
-			if (changedProperty.Is(IndicatorView.IndicatorsShapeProperty))
+			if (changedProperty.Is(IndicatorView.IndicatorTemplateProperty))
 			{
-				UpdateShapes();
+				UpdateIndicatorTemplate();
 			}
-			else if (changedProperty.Is(IndicatorView.IndicatorColorProperty) ||
+			else if (changedProperty.Is(IndicatorView.IndicatorsShapeProperty) ||
+					 changedProperty.Is(IndicatorView.IndicatorColorProperty) ||
 					 changedProperty.Is(IndicatorView.SelectedIndicatorColorProperty))
 			{
 				ResetIndicators();
@@ -197,8 +185,6 @@ namespace Xamarin.Forms.Platform.Android
 			if (Tracker == null)
 			{
 				_visualElementTracker = new VisualElementTracker(this);
-				//_visualElementPackager = new VisualElementPackager(this);
-				//_visualElementPackager.Load();
 			}
 
 			this.EnsureId();
@@ -206,15 +192,9 @@ namespace Xamarin.Forms.Platform.Android
 			UpdateBackgroundColor();
 
 			if (IndicatorsView.IndicatorTemplate != null)
-			{
-				var control = IndicatorsView.IndicatorLayout.GetRenderer() ?? Platform.CreateRendererWithContext(IndicatorsView.IndicatorLayout, Context);
-				Platform.SetRenderer(IndicatorsView.IndicatorLayout, control);
-				AddView(control as AView);
-			}
+				UpdateIndicatorTemplate();
 			else
-			{
 				UpdateItemsSource();
-			}
 
 			ElevationHelper.SetElevation(this, newElement);
 		}
@@ -292,10 +272,30 @@ namespace Xamarin.Forms.Platform.Android
 			_shapeType = IndicatorsView.IndicatorsShape == IndicatorShape.Circle ? AShapeType.Oval : AShapeType.Rectangle;
 			_pageShape = null;
 			_currentPageShape = null;
+
+			if (IndicatorsView.IndicatorTemplate == null)
 				UpdateShapes();
+			else
+				UpdateIndicatorTemplate();
+
 			UpdateIndicators();
 		}
 
+		void UpdateIndicatorTemplate()
+		{
+			if (IndicatorsView.IndicatorLayout == null)
+				return;
+
+			var renderer = IndicatorsView.IndicatorLayout.GetRenderer() ?? Platform.CreateRendererWithContext(IndicatorsView.IndicatorLayout, Context);
+			Platform.SetRenderer(IndicatorsView.IndicatorLayout, renderer);
+
+			RemoveAllViews();
+			AddView(renderer.View);
+
+			var indicatorLayoutSizeRequest = IndicatorsView.IndicatorLayout.Measure(double.PositiveInfinity, double.PositiveInfinity, MeasureFlags.IncludeMargins);
+			IndicatorsView.IndicatorLayout.Layout(new Rectangle(0, 0, indicatorLayoutSizeRequest.Request.Width, indicatorLayoutSizeRequest.Request.Height));
+		}
+
 		void UpdateIndicators()
 		{
 			if (!IsVisible)
@@ -318,16 +318,16 @@ namespace Xamarin.Forms.Platform.Android
 			if (_currentPageShape != null)
 				return;
 
-			_currentPageShape = GetCircle(_currentPageIndicatorTintColor);
-			_pageShape = GetCircle(_pageIndicatorTintColor);
+			_currentPageShape = GetShape(_currentPageIndicatorTintColor);
+			_pageShape = GetShape(_pageIndicatorTintColor);
 		}
 
-		Drawable GetCircle(AColor color)
+		Drawable GetShape(AColor color)
 		{
 			var indicatorSize = IndicatorsView.IndicatorSize;
-			ShapeDrawable shape = null;
+			ShapeDrawable shape;
 
-			if (_shapeType == ShapeType.Oval)
+			if (_shapeType == AShapeType.Oval)
 				shape = new ShapeDrawable(new AShapes.OvalShape());
 			else
 				shape = new ShapeDrawable(new AShapes.RectShape());

Xamarin.Forms.Platform.iOS/Renderers/IndicatorViewRenderer.cs

@@ -12,6 +12,8 @@ namespace Xamarin.Forms.Platform.iOS
 		bool _disposed;
 		bool _updatingPosition;
 
+		public UIView View => this;
+
 		protected override void OnElementChanged(ElementChangedEventArgs<IndicatorView> e)
 		{
 			base.OnElementChanged(e);
@@ -38,6 +40,7 @@ namespace Xamarin.Forms.Platform.iOS
 		{
 			if (_disposed)
 				return;
+
 			_disposed = true;
 
 			if (disposing)
@@ -47,14 +50,19 @@ namespace Xamarin.Forms.Platform.iOS
 					UIPager.ValueChanged -= UIPagerValueChanged;
 				}
 			}
+
 			base.Dispose(disposing);
 		}
 
 		protected override void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e)
 		{
 			base.OnElementPropertyChanged(sender, e);
-			if (UIPager == null)
-				return;
+
+			if (e.PropertyName == IndicatorsShapeProperty.PropertyName ||
+				e.PropertyName == ItemsSourceProperty.PropertyName)
+				UpdateIndicator();
+			else if (e.PropertyName == IndicatorTemplateProperty.PropertyName)
+				UpdateIndicatorTemplate();
 			if (e.PropertyName == IndicatorColorProperty.PropertyName)
 				UpdatePagesIndicatorTintColor();
 			else if (e.PropertyName == SelectedIndicatorColorProperty.PropertyName)
@@ -73,15 +81,19 @@ namespace Xamarin.Forms.Platform.iOS
 			{
 				UIPager.ValueChanged -= UIPagerValueChanged;
 			}
+
 			var uiPager = new UIPageControl();
 			_defaultPagesIndicatorTintColor = uiPager.PageIndicatorTintColor;
 			_defaultCurrentPagesIndicatorTintColor = uiPager.CurrentPageIndicatorTintColor;
 			uiPager.ValueChanged += UIPagerValueChanged;
+
 			return uiPager;
 		}
 	
 		void UpdateControl()
 		{
+			ClearIndicators();
+
 			var control = (Element.IndicatorTemplate != null)
 				? (UIView)Element.IndicatorLayout.GetRenderer()
 				: CreateNativeControl();
@@ -89,32 +101,87 @@ namespace Xamarin.Forms.Platform.iOS
 			SetNativeControl(control);
 		}
 
+		void ClearIndicators()
+		{
+			foreach (var child in View.Subviews)
+				child.RemoveFromSuperview();
+		}
+
+		void UpdateIndicator()
+		{
+			if (Element.IndicatorsShape == IndicatorShape.Circle && Element.IndicatorTemplate == null)
+				UpdateIndicatorShape();
+			else
+				UpdateIndicatorTemplate();
+		}
+
+		void UpdateIndicatorShape()
+		{
+			ClearIndicators();
+			AddSubview(UIPager);
+		}
+
+		void UpdateIndicatorTemplate()
+		{
+			if (Element.IndicatorLayout == null)
+				return;
+
+			ClearIndicators();
+			var control = (UIView)Element.IndicatorLayout.GetRenderer();
+			AddSubview(control);
+
+			var indicatorLayoutSizeRequest = Element.IndicatorLayout.Measure(double.PositiveInfinity, double.PositiveInfinity, MeasureFlags.IncludeMargins);
+			Element.IndicatorLayout.Layout(new Rectangle(0, 0, indicatorLayoutSizeRequest.Request.Width, indicatorLayoutSizeRequest.Request.Height));
+		}
+
 		void UIPagerValueChanged(object sender, System.EventArgs e)
 		{
-			if (_updatingPosition)
+			if (_updatingPosition || UIPager == null)
 				return;
+
 			Element.Position = (int)UIPager.CurrentPage;
 		}
 
 		void UpdateCurrentPage()
 		{
+			if (UIPager == null)
+				return;
+
 			_updatingPosition = true;
 			UIPager.CurrentPage = Element.Position;
 			_updatingPosition = false;
 		}
 
-		void UpdatePages() => UIPager.Pages = Element.Count;
+		void UpdatePages()
+		{
+			if (UIPager == null)
+				return;
 
-		void UpdateHidesForSinglePage() => UIPager.HidesForSinglePage = Element.HideSingle;
+			UIPager.Pages = Element.Count;
+		}
+
+		void UpdateHidesForSinglePage()
+		{
+			if (UIPager == null)
+				return;
+
+			UIPager.HidesForSinglePage = Element.HideSingle;
+		}
 
 		void UpdatePagesIndicatorTintColor()
 		{
+			if (UIPager == null)
+				return;
+
 			var color = Element.IndicatorColor;
 			UIPager.PageIndicatorTintColor = color.IsDefault ? _defaultPagesIndicatorTintColor : color.ToUIColor();
 		}
 
 		void UpdateCurrentPagesIndicatorTintColor()
 		{
+			if (UIPager == null)
+				return;
+
 			var color = Element.SelectedIndicatorColor;
 			UIPager.CurrentPageIndicatorTintColor = color.IsDefault ? _defaultCurrentPagesIndicatorTintColor : color.ToUIColor();
 		}