Disable using EstimatedRowHeight for uneven rows with known heights (#454)

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

@@ -0,0 +1,204 @@
+using System;
+using System.Collections.ObjectModel;
+using System.ComponentModel;
+using System.Linq;
+using System.Runtime.CompilerServices;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+namespace Xamarin.Forms.Controls
+{
+	[Preserve(AllMembers = true)]
+	public abstract class Bugzilla43313_Template : ContentPage
+	{
+		public static int ItemCount = 20;
+		readonly ListView _listView;
+		protected abstract DataTemplate CellTemplate();
+
+		protected Bugzilla43313_Template()
+		{
+			BindingContext = new _43313ViewModel();
+
+			var btnAdd = new Button
+			{
+				Text = "Add item",
+				WidthRequest = 100
+			};
+			btnAdd.Clicked += BtnAddOnClicked;
+
+			var btnBottom = new Button
+			{
+				Text = "Scroll to end",
+				WidthRequest = 100
+			};
+			btnBottom.Clicked += BtnBottomOnClicked;
+
+			var btnPanel = new StackLayout
+			{
+				Orientation = StackOrientation.Horizontal,
+				HorizontalOptions = LayoutOptions.Center,
+				Children = { btnAdd, btnBottom }
+			};
+
+			_listView = new ListView
+			{
+				HasUnevenRows = true,
+				BackgroundColor = Color.Transparent,
+				VerticalOptions = LayoutOptions.FillAndExpand,
+				ItemTemplate = CellTemplate()
+			};
+
+			_listView.SetBinding(ListView.ItemsSourceProperty, new Binding("ListViewContent"));
+			_listView.ItemTapped += (sender, e) => ((ListView)sender).SelectedItem = null;
+
+			var instructions = new Label() { FontSize = 12, Text = "Tap the 'Add Item' button; a new item should be added to the bottom of the list and the list should scroll smoothly to display it. If the list scrolls back to the top before scrolling down to the new item, the test has failed." };
+
+			Content = new StackLayout
+			{
+				Padding = new Thickness(0, 40, 0, 0),
+				Children =
+				{
+					instructions,
+					btnPanel,
+					_listView
+				}
+			};
+		}
+
+		void BtnAddOnClicked(object sender, EventArgs eventArgs)
+		{
+			string str = $"Item {ItemCount++}";
+			var item = new _43313Model { Name = str };
+			(BindingContext as _43313ViewModel).ListViewContent.Add(item);
+
+			_listView.ScrollTo(item, ScrollToPosition.End, true);
+		}
+
+		void BtnBottomOnClicked(object sender, EventArgs e)
+		{
+			_43313Model item = (BindingContext as _43313ViewModel).ListViewContent.Last();
+			_listView.ScrollTo(item, ScrollToPosition.End, true);
+		}
+
+		[Preserve(AllMembers = true)]
+		public class _43313Model
+		{
+			public string Name { get; set; }
+		}
+
+		[Preserve(AllMembers = true)]
+		public class _43313ViewModel : INotifyPropertyChanged
+		{
+			ObservableCollection<_43313Model> _listViewContent;
+
+			public _43313ViewModel()
+			{
+				ListViewContent = new ObservableCollection<_43313Model>();
+
+				for (int n = 0; n < ItemCount; n++)
+				{
+					_listViewContent.Add(new _43313Model { Name = $"Item {n}" });
+				}
+			}
+
+			public ObservableCollection<_43313Model> ListViewContent
+			{
+				get { return _listViewContent; }
+				set
+				{
+					_listViewContent = value;
+					OnPropertyChanged();
+				}
+			}
+
+			public event PropertyChangedEventHandler PropertyChanged;
+
+			protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
+			{
+				PropertyChangedEventHandler handler = PropertyChanged;
+				handler?.Invoke(this, new PropertyChangedEventArgs(propertyName));
+			}
+		}
+	}
+
+	[Preserve(AllMembers = true)]
+	public class Bugzilla43313_KnownHeight : Bugzilla43313_Template
+	{
+		protected override DataTemplate CellTemplate()
+		{
+			return new DataTemplate(() =>
+			{
+				var label = new Label { FontSize = 16, VerticalOptions = LayoutOptions.Center };
+				label.SetBinding(Label.TextProperty, "Name");
+				int height = 60 + new Random().Next(10, 100);
+
+				return new ViewCell
+				{
+					Height = height,
+					View = new StackLayout
+					{
+						Padding = new Thickness(0, 5, 0, 5),
+						BackgroundColor = Color.Transparent,
+						Children =
+							{
+								label
+							}
+					}
+				};
+			});
+		}
+	}
+
+	[Preserve(AllMembers = true)]
+	public class Bugzilla43313_EstimatedHeight : Bugzilla43313_Template
+	{
+		protected override DataTemplate CellTemplate()
+		{
+			return new DataTemplate(() =>
+			{
+				var label = new Label { FontSize = 16, VerticalOptions = LayoutOptions.Center };
+				label.SetBinding(Label.TextProperty, "Name");
+
+				label.FontSize = 12 + new Random().Next(1, 6);
+
+				return new ViewCell
+				{
+					View = new StackLayout
+					{
+						Padding = new Thickness(0, 5, 0, 5),
+						BackgroundColor = Color.Transparent,
+						Children =
+							{
+								label
+							}
+					}
+				};
+			});
+		}
+	}
+
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Bugzilla, 43313, "Adding an item to ListView ItemSource has unexpected animation with different height rows and HasUnevenRows is true", PlatformAffected.iOS)]
+	public class Bugzilla43313 : TestNavigationPage
+	{
+		protected override void Init()
+		{
+			var root = new ContentPage();
+			
+			var layout = new StackLayout();
+
+			var knownHeightButton = new Button() { Text = "Known Height (original bug report test case)" };
+			knownHeightButton.Clicked += (sender, args) => PushAsync(new Bugzilla43313_KnownHeight());
+
+			var estimatedHeightButton = new Button() { Text = "Estimated Height" };
+			estimatedHeightButton.Clicked += (sender, args) => PushAsync(new Bugzilla43313_EstimatedHeight());
+
+			layout.Children.Add(knownHeightButton);
+			layout.Children.Add(estimatedHeightButton);
+
+			root.Content = layout;
+
+			PushAsync(root);
+		}
+	}
+}

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

@@ -129,6 +129,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla42329.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla42364.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla42519.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Bugzilla43313.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla43516.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla43663.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla44944.cs" />

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

@@ -24,6 +24,7 @@ namespace Xamarin.Forms.Platform.iOS
 		RectangleF _previousFrame;
 		ScrollToRequestedEventArgs _requestedScroll;
 		bool _shouldEstimateRowHeight = true;
+			
 		FormsUITableViewController _tableViewController;
 		IListViewController Controller => Element;
 		ITemplatedItemsView<Cell> TemplatedItemsView => Element;
@@ -327,6 +328,7 @@ namespace Xamarin.Forms.Platform.iOS
 
 			var position = GetScrollPosition(e.Position);
 			var scrollArgs = (ITemplatedItemsListScrollToRequestedEventArgs)e;
+
 			var templatedItems = TemplatedItemsView.TemplatedItems;
 			if (Element.IsGroupingEnabled)
 			{
@@ -338,7 +340,10 @@ namespace Xamarin.Forms.Platform.iOS
 			{
 				var index = templatedItems.GetGlobalIndexOfItem(scrollArgs.Item);
 				if (index != -1)
+				{
+					Control.Layer.RemoveAllAnimations();
 					Control.ScrollToRow(NSIndexPath.FromRowSection(index, 0), position, e.ShouldAnimate);
+				}
 			}
 		}
 
@@ -353,11 +358,9 @@ namespace Xamarin.Forms.Platform.iOS
 				var source = _dataSource as UnevenListViewDataSource;
 				if (_shouldEstimateRowHeight)
 				{
-					var templatedItems = TemplatedItemsView.TemplatedItems;
-					if (templatedItems.Count > 0 && source != null)
+					if (source != null)
 					{
-						var estimatedHeightFromFirstCell = source.CalculateHeightForCell(Control, templatedItems.First());
-						Control.EstimatedRowHeight = estimatedHeightFromFirstCell;
+						Control.EstimatedRowHeight = source.GetEstimatedRowHeight(Control);
 						_estimatedRowHeight = true;
 					}
 					else
@@ -482,12 +485,13 @@ namespace Xamarin.Forms.Platform.iOS
 			switch (e.Action)
 			{
 				case NotifyCollectionChangedAction.Add:
+
 					UpdateEstimatedRowHeight();
 					if (e.NewStartingIndex == -1 || groupReset)
 						goto case NotifyCollectionChangedAction.Reset;
+
 					Control.BeginUpdates();
 					Control.InsertRows(GetPaths(section, e.NewStartingIndex, e.NewItems.Count), UITableViewRowAnimation.Automatic);
-
 					Control.EndUpdates();
 
 					break;
@@ -560,11 +564,9 @@ namespace Xamarin.Forms.Platform.iOS
 		void UpdateRowHeight()
 		{
 			var rowHeight = Element.RowHeight;
-			if (Element.HasUnevenRows && rowHeight == -1 && Forms.IsiOS7OrNewer)
-			{
-				if (Forms.IsiOS8OrNewer)
-					Control.RowHeight = UITableView.AutomaticDimension;
-			}
+			
+			if (Element.HasUnevenRows && rowHeight == -1 && Forms.IsiOS8OrNewer)
+				Control.RowHeight = UITableView.AutomaticDimension;
 			else
 				Control.RowHeight = rowHeight <= 0 ? DefaultRowHeight : rowHeight;
 		}
@@ -606,6 +608,36 @@ namespace Xamarin.Forms.Platform.iOS
 			{
 			}
 
+			internal nfloat GetEstimatedRowHeight(UITableView table) 
+			{
+				if (List.RowHeight != -1)
+				{
+					// Not even sure we need this case; A list with HasUnevenRows and a RowHeight doesn't make a ton of sense
+					// Anyway, no need for an estimate, because the heights we'll use are known
+					return 0;
+				}
+
+				var templatedItems = TemplatedItemsView.TemplatedItems;
+
+				if (templatedItems.Count == 0)
+				{
+					// No cells to provide an estimate, use the default row height constant
+					return DefaultRowHeight;
+				}
+
+				// We're going to base our estimate off of the first cell 
+				var firstCell = templatedItems.First();
+
+				if (firstCell.Height > 0) 
+				{
+					// Seems like we've got cells which already specify their height; since the heights are known,
+					// we don't need to use estimatedRowHeight at all; zero will disable it and use the known heights
+					return 0; 
+				}
+
+				return CalculateHeightForCell(table, firstCell);
+			}
+
 			public override nfloat GetHeightForRow(UITableView tableView, NSIndexPath indexPath)
 			{
 				var cell = GetCellForPath(indexPath);
@@ -662,7 +694,7 @@ namespace Xamarin.Forms.Platform.iOS
 			readonly FormsUITableViewController _uiTableViewController;
 			protected readonly ListView List;
 			IListViewController Controller => List;
-			ITemplatedItemsView<Cell> TemplatedItemsView => List;
+			protected ITemplatedItemsView<Cell> TemplatedItemsView => List;
 			bool _isDragging;
 			bool _selectionFromNative;