[iOS] Fix missing partial columns in horizontal grid layout (#6241)

* Create repro

* Work around UICollectionViewFlowLayout bug to display partial columns; fixes #6077
Fix content inset behaviors when dealing with safe area on newer iOS versions

* Add check for case where there is only a single partial column

* Works better if I don't forget the return statement

* Split up tests to make it easier to determine what failed

* Fix bounds exception when changing to a smaller ItemsSource

* Add preserve attributes so the linker won't ruin my UI test

* Fix missing using after rebase

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

@@ -0,0 +1,139 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.ComponentModel;
+using System.Runtime.CompilerServices;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+using Xamarin.Forms.PlatformConfiguration;
+using Xamarin.Forms.PlatformConfiguration.iOSSpecific;
+
+#if UITEST
+using Xamarin.Forms.Core.UITests;
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+#if UITEST
+	[NUnit.Framework.Category(UITestCategories.CollectionView)]
+#endif
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 6077, "CollectionView (iOS) using horizontal grid does not display last column of uneven item count", PlatformAffected.iOS)]
+	public class Issue6077 : TestNavigationPage
+	{
+		[Preserve(AllMembers = true)]
+		public class MainViewModel : INotifyPropertyChanged
+		{
+			readonly IList<ItemModel> _items;
+			public ObservableCollection<ItemModel> Items { get; private set; }
+
+			public MainViewModel()
+			{
+				_items = new List<ItemModel>();
+				CreateItemsCollection();
+			}
+
+			void CreateItemsCollection(int items = 5)
+			{
+				for (int n = 0; n < items; n++)
+				{
+					_items.Add(new ItemModel
+					{
+						Title = $"Item {n + 1}",
+					});
+				}
+
+				Items = new ObservableCollection<ItemModel>(_items);
+			}
+
+			protected bool SetProperty<T>(ref T backingStore, T value,
+				[CallerMemberName]string propertyName = "",
+				Action onChanged = null)
+			{
+				if (EqualityComparer<T>.Default.Equals(backingStore, value))
+					return false;
+
+				backingStore = value;
+				onChanged?.Invoke();
+				OnPropertyChanged(propertyName);
+				return true;
+			}
+
+			#region INotifyPropertyChanged
+			public event PropertyChangedEventHandler PropertyChanged;
+			protected void OnPropertyChanged([CallerMemberName] string propertyName = "")
+			{
+				var changed = PropertyChanged;
+				if (changed == null)
+					return;
+
+				changed.Invoke(this, new PropertyChangedEventArgs(propertyName));
+			}
+			#endregion
+		}
+
+		[Preserve(AllMembers = true)]
+		public class ItemModel
+		{
+			public string Title { get; set; }
+		}
+
+		ContentPage CreateRoot()
+		{
+			var page = new ContentPage { Title = "Issue6077" };
+
+			var cv = new CollectionView { ItemSizingStrategy = ItemSizingStrategy.MeasureAllItems };
+
+			var itemsLayout = new GridItemsLayout(3, ItemsLayoutOrientation.Horizontal);
+			
+
+			cv.ItemsLayout = itemsLayout;
+
+			var template = new DataTemplate(() => {
+				var grid = new Grid { HeightRequest = 100, WidthRequest = 50, BackgroundColor = Color.AliceBlue };
+
+				grid.RowDefinitions = new RowDefinitionCollection { new RowDefinition { Height = new GridLength(100) } };
+				grid.ColumnDefinitions = new ColumnDefinitionCollection { new ColumnDefinition { Width = new GridLength(50) } };
+
+				var label = new Label { };
+
+				label.SetBinding(Label.TextProperty, new Binding("Title"));
+
+				var content = new ContentView { Content = label };
+
+				grid.Children.Add(content);
+
+				return grid;
+			});
+
+			cv.ItemTemplate = template;
+			cv.SetBinding(ItemsView.ItemsSourceProperty, new Binding("Items"));
+
+			page.Content = cv;
+
+			BindingContext = new MainViewModel();
+
+			return page;
+		}
+
+		protected override void Init()
+		{
+#if APP
+			Device.SetFlags(new List<string>(Device.Flags ?? new List<string>()) { "CollectionView_Experimental" });
+
+			PushAsync(CreateRoot());
+#endif
+		}
+
+#if UITEST
+		[Test]
+		public void LastColumnShouldBeVisible()
+		{
+			// If the partial column shows up, then Item 5 will be in it
+			RunningApp.WaitForElement("Item 5");
+		}
+#endif
+	}
+}

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

@@ -964,6 +964,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue5132.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue5888.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue6334.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue6077.cs" />
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">

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

@@ -49,21 +49,24 @@ namespace Xamarin.Forms.Core.UITests
 		//	"ScrollToItemCode,HorizontalList", "ScrollToItemCode,VerticalList", "ScrollToItemCode,HorizontalGrid", "ScrollToItemCode,VerticalGrid",
 		//  }, 19, 3)]
 		//[TestCase("Snap Points", new string[] { "SnapPointsCode,HorizontalList", "SnapPointsCode,VerticalList", "SnapPointsCode,HorizontalGrid", "SnapPointsCode,VerticalGrid" }, 19, 2)]
-		[TestCase("Observable Collection", new string[] { "Add/RemoveItemsList", "Add/RemoveItemsGrid" }, 19, 6)]
-		[TestCase("Default Text", new string[] { "VerticalListCode", "HorizontalListCode", "VerticalGridCode", "HorizontalGridCode" }, 101, 11)]
-		[TestCase("DataTemplate", new string[] { "VerticalListCode", "HorizontalListCode", "VerticalGridCode", "HorizontalGridCode" }, 19, 6)]
-		public void VisitAndUpdateItemsSource(string collectionTestName, string[] subGalleries, int firstItem, int lastItem)
+		[TestCase("Observable Collection", "Add/RemoveItemsList", 19, 6)]
+		[TestCase("Observable Collection", "Add/RemoveItemsGrid", 19, 6)]
+
+		[TestCase("Default Text", "VerticalListCode", 101, 11)]
+		[TestCase("Default Text", "HorizontalListCode", 101, 11)]
+		[TestCase("Default Text", "VerticalGridCode", 101, 11)]
+		[TestCase("Default Text", "HorizontalGridCode", 101, 11)]
+
+		[TestCase("DataTemplate", "VerticalListCode", 19, 6)]
+		[TestCase("DataTemplate", "HorizontalListCode", 19, 6)]
+		[TestCase("DataTemplate", "VerticalGridCode", 19, 6)]
+		[TestCase("DataTemplate", "HorizontalGridCode", 19, 6)]
+		public void VisitAndUpdateItemsSource(string collectionTestName, string subGallery, int firstItem, int lastItem)
 		{
 			VisitInitialGallery(collectionTestName);
 
-			foreach (var gallery in subGalleries)
-			{
-				if (gallery == "FilterItems")
-					continue;
-
-				VisitSubGallery(gallery, !gallery.Contains("Horizontal"), $"Item: {firstItem}", $"Item: {lastItem}", lastItem - 1, true, false);
-				App.NavigateBack();
-			}
+			VisitSubGallery(subGallery, !subGallery.Contains("Horizontal"), $"Item: {firstItem}", $"Item: {lastItem}", lastItem - 1, true, false);
+			App.NavigateBack();
 		}
 
 		//[TestCase("ScrollTo", new string[] {

Xamarin.Forms.Platform.iOS/CollectionView/GridViewLayout.cs

@@ -1,7 +1,7 @@
 using System;
 using System.ComponentModel;
-using System.Diagnostics;
 using CoreGraphics;
+using Foundation;
 using UIKit;
 
 namespace Xamarin.Forms.Platform.iOS
@@ -43,7 +43,7 @@ namespace Xamarin.Forms.Platform.iOS
 
 			ConstrainedDimension = (availableSpace - spacing) / _itemsLayout.Span;
 
-			// TODO hartez 2018/09/12 14:52:24 We need to truncate the decimal part of ConstrainedDimension
+			// We need to truncate the decimal part of ConstrainedDimension
 			// or we occasionally run into situations where the rows/columns don't fit	
 			// But this can run into situations where we have an extra gap because we're cutting off too much
 			// and we have a small gap; need to determine where the cut-off is that leads to layout dropping a whole row/column
@@ -58,5 +58,107 @@ namespace Xamarin.Forms.Platform.iOS
 			ConstrainedDimension = (int)ConstrainedDimension;
 			DetermineCellSize();
 		}
+
+		/* `CollectionViewContentSize` and `LayoutAttributesForElementsInRect` are overridden here to work around what 
+		 * appears to be a bug in the UICollectionViewFlowLayout implementation: for horizontally scrolling grid
+		 * layouts with auto-sized cells, trailing items which don't fill out a column are never displayed. 
+		 * For example, with a span of 3 and either 4 or 5 items, the resulting layout looks like
+		 *
+		 * 		Item1 
+		 * 		Item2
+		 * 		Item3
+		 * 
+		 * But with 6 items, it looks like
+		 * 
+		 * 		Item1 Item4
+		 * 		Item2 Item5
+		 * 		Item3 Item6
+		 * 
+		 * IOW, if there are not enough items to fill out the last column, the last column is ignored.
+		 * 
+		 * These overrides detect and correct that situation.	 
+		 */
+
+		public override CGSize CollectionViewContentSize
+		{
+			get
+			{
+				if (!NeedsPartialColumnAdjustment())
+				{
+					return base.CollectionViewContentSize;
+				}
+
+				var contentSize = base.CollectionViewContentSize;
+
+				// Add space for the missing column at the end
+				var correctedSize = new CGSize(contentSize.Width + EstimatedItemSize.Width, contentSize.Height);
+
+				return correctedSize;
+			}
+		}
+
+		public override UICollectionViewLayoutAttributes[] LayoutAttributesForElementsInRect(CGRect rect)
+		{
+			if (!NeedsPartialColumnAdjustment())
+			{ 
+				return base.LayoutAttributesForElementsInRect(rect);
+			}
+
+			// When we implement Groups, we'll have to iterate over all of them to adjust and this will
+			// be a lot more complicated. But until then, we only have to worry about section 0
+			int section = 0;
+
+			var fullColumns = base.LayoutAttributesForElementsInRect(rect);
+
+			var itemCount = CollectionView.NumberOfItemsInSection(section);
+
+			if (fullColumns.Length == itemCount)
+			{
+				return fullColumns;
+			}
+
+			var missingCellCount = itemCount % _itemsLayout.Span;
+
+			UICollectionViewLayoutAttributes[] allCells = new UICollectionViewLayoutAttributes[fullColumns.Length + missingCellCount];
+			fullColumns.CopyTo(allCells, 0);
+
+			for (int n = fullColumns.Length; n < allCells.Length; n++)
+			{
+				allCells[n] = LayoutAttributesForItem(NSIndexPath.FromItemSection(n, section));
+			}
+
+			return allCells;
+		}
+
+		bool NeedsPartialColumnAdjustment(int section = 0)
+		{
+			if (ScrollDirection == UICollectionViewScrollDirection.Vertical)
+			{
+				// The bug only occurs with Horizontal scrolling
+				return false; 
+			}
+
+			if (EstimatedItemSize.IsEmpty)
+			{
+				// The bug only occurs when using Autolayout; with a set ItemSize, we don't have to worry about it
+				return false;
+			}
+
+			var itemCount = CollectionView.NumberOfItemsInSection(section);
+
+			if (itemCount < _itemsLayout.Span)
+			{
+				// If there is just one partial column, no problem; UICollectionViewFlowLayout gets it right
+				return false;
+			}
+
+			if (itemCount % _itemsLayout.Span == 0)
+			{
+				// All of the columns are full; the bug only occurs when we have a partial column
+				return false;
+			}
+
+			return true;
+		}
 	}
 }

Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs

@@ -27,6 +27,15 @@ namespace Xamarin.Forms.Platform.iOS
 				: UICollectionViewScrollDirection.Vertical;
 
 			Initialize(scrollDirection);
+
+			if (Forms.IsiOS11OrNewer)
+			{
+				// `ContentInset` is actually the default value, but I'm leaving this here as a note to
+				// future maintainers; it's likely that someone will want a Platform Specific to change this behavior
+				// (Setting it to `SafeArea` lets you do the thing where the header/footer of your UICollectionView
+				// fills the screen width in landscape while your items are automatically shifted to avoid the notch)
+				SectionInsetReference = UICollectionViewFlowLayoutSectionInsetReference.ContentInset;
+			}
 		}
 
 		protected override void Dispose(bool disposing)

Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewRenderer.cs

@@ -85,6 +85,16 @@ namespace Xamarin.Forms.Platform.iOS
 
 			UpdateLayout();
 			ItemsViewController = CreateController(newElement, _layout);
+			 
+			if (Forms.IsiOS11OrNewer)
+			{
+				// We set this property to keep iOS from trying to be helpful about insetting all the 
+				// CollectionView content when we're in landscape mode (to avoid the notch)
+				// The SetUseSafeArea Platform Specific is already taking care of this for us 
+				// That said, at some point it's possible folks will want a PS for controlling this behavior
+				ItemsViewController.CollectionView.ContentInsetAdjustmentBehavior = UIScrollViewContentInsetAdjustmentBehavior.Never;
+			}
+
 			SetNativeControl(ItemsViewController.View);
 			ItemsViewController.CollectionView.BackgroundColor = UIColor.Clear;
 			ItemsViewController.UpdateEmptyView();