[iOS] Better fix for EstimatedRowHeight (#4365) fixes #4356

* [Controls] Add repo for issue #4356

* [iOS] Call UpdateEstimatedRowHeight on MainThread so it delays and reloads after the row is inserted

* [IOS] Try remove UpdateEstimatedRowHeight

* [iOS] Fix EstimatedRowHeight

* [iOS] Consolidate EstimatedRowHeight calculation on the ListviewDataSource

* [iOS] Fix InvalidateCache and clear estimatedRowHeight

* [iOS] Use bool for checking if is empty

Co-Authored-By: rmarinho <me@ruimarinho.net>

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

Co-Authored-By: rmarinho <me@ruimarinho.net>

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

Co-Authored-By: rmarinho <me@ruimarinho.net>

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

Co-Authored-By: rmarinho <me@ruimarinho.net>

* [iOS] Fix rebase

* [Build] Update submodule

* [Build] Remove xcode select

* [iOS] Don't disable EstimatedRowHeight on iOS10
This commit is contained in:
Rui Marinho 2019-03-24 19:48:16 +00:00 коммит произвёл GitHub
Родитель 7a2a6a241e
Коммит b0724320b4
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
5 изменённых файлов: 318 добавлений и 61 удалений

@ -1 +1 @@
Subproject commit c5be7a66d068c26e8334202deda3a5fafc03a4b1
Subproject commit bde6c61641ace9b90c39ffc12e97e9e590ce0f31

Просмотреть файл

@ -0,0 +1,190 @@
using System;
using System.Collections.Generic;
using Xamarin.Forms;
using System.Threading.Tasks;
using System.Collections.ObjectModel;
using System.ComponentModel;
using Xamarin.Forms.CustomAttributes;
using Xamarin.Forms.Internals;
#if UITEST
using Xamarin.UITest;
using NUnit.Framework;
#endif
namespace Xamarin.Forms.Controls.Issues
{
[Preserve(AllMembers = true)]
[Issue(IssueTracker.Github, 4356, "[iOS] NSInternalInconsistencyException thrown when adding item to ListView after clearing bound ObservableCollection")]
public partial class Issue4356 : TestContentPage
{
#if UITEST
[Test]
public void Issue4356Test()
{
RunningApp.WaitForElement(q => q.Marked("Will this repo work?"));
RunningApp.WaitForElement(q => q.Marked("Remove item"));
RunningApp.Tap(q => q.Marked("Remove item"));
RunningApp.Tap(q => q.Marked("Remove item"));
RunningApp.Tap(q => q.Marked("Add item"));
RunningApp.WaitForElement(q => q.Marked("Added from Button Command"));
}
#endif
FavoritesViewModel ViewModel
{
get { return BindingContext as FavoritesViewModel; }
}
public Issue4356()
{
#if APP
InitializeComponent();
BindingContext = new FavoritesViewModel();
listView.SeparatorVisibility = SeparatorVisibility.Default;
listView.SeparatorColor = Color.FromHex("#ababab");
listView.ItemTapped += (sender, args) =>
{
if (listView.SelectedItem == null)
return;
//do nothing anyway
return;
};
#endif
}
protected override void Init()
{
}
#pragma warning disable 1998 // considered for removal
public async void OnDelete(object sender, EventArgs e)
#pragma warning restore 1998
{
var mi = ((MenuItem)sender);
if (mi.CommandParameter == null)
return;
var articlelistingitem = mi.CommandParameter;
if (articlelistingitem != null)
DisplayAlert("Alert", "I'm not deleting just refreshing...", "Ok");
ViewModel.LoadFavoritesCommand.Execute(null);
}
protected override void OnAppearing()
{
base.OnAppearing();
if (ViewModel == null || !ViewModel.CanLoadMore || ViewModel.IsBusy)
return;
Device.BeginInvokeOnMainThread(() =>
{
ViewModel.LoadFavoritesCommand.Execute(null);
});
}
[Preserve(AllMembers = true)]
public class FavoritesViewModel : BaseViewModelF
{
public ObservableCollection<ArticleListing> FavoriteArticles { get; set; }
public int Count { get; private set; } = 0;
readonly object _listLock = new object();
public FavoritesViewModel()
{
Title = "Favorites";
FavoriteArticles = new ObservableCollection<ArticleListing>();
AddCommand = new Command(() =>
{
lock (_listLock)
{
FavoriteArticles.Add(new ArticleListing
{
ArticleTitle = "Added from Button Command",
AuthorString = "Rui Marinho",
FormattedPostedDate = "08-11-2018"
});
}
});
RemoveCommand = new Command(() =>
{
lock (_listLock)
{
if (FavoriteArticles.Count > 0)
{
FavoriteArticles.RemoveAt(FavoriteArticles.Count - 1);
--Count;
}
}
});
}
public Command AddCommand { get; }
public Command RemoveCommand { get; }
Command _loadFavoritesCommand;
public Command LoadFavoritesCommand
{
get
{
return _loadFavoritesCommand ??
(_loadFavoritesCommand = new Command(async () =>
{
await ExecuteFavoritesCommand();
}, () =>
{
return !IsBusy;
}));
}
}
#pragma warning disable 1998 // considered for removal
public async Task ExecuteFavoritesCommand()
#pragma warning restore 1998
{
if (IsBusy)
return;
IsBusy = true;
LoadFavoritesCommand.ChangeCanExecute();
FavoriteArticles.Clear();
var articles = new ObservableCollection<ArticleListing> {
new ArticleListing {
ArticleTitle = "Will this repo work?",
AuthorString = "Ben Crispin",
FormattedPostedDate = "7-28-2015"
},
new ArticleListing {
ArticleTitle = "Xamarin Forms BugZilla",
AuthorString = "Some Guy",
FormattedPostedDate = "7-28-2015"
}
};
var templist = new ObservableCollection<ArticleListing>();
foreach (var article in articles)
{
//templist.Add(article);
FavoriteArticles.Add(article);
}
//FavoriteArticles = templist;
OnPropertyChanged("FavoriteArticles");
IsBusy = false;
LoadFavoritesCommand.ChangeCanExecute();
}
}
}
}

Просмотреть файл

@ -0,0 +1,42 @@
<?xml version="1.0" encoding="UTF-8"?>
<local:TestContentPage xmlns="http://xamarin.com/schemas/2014/forms" xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" xmlns:local="clr-namespace:Xamarin.Forms.Controls" x:Class="Xamarin.Forms.Controls.Issues.Issue4356">
<StackLayout Orientation="Vertical">
<StackLayout Orientation="Horizontal">
<Button Text="Add item" HorizontalOptions="CenterAndExpand" Command="{Binding AddCommand}" />
<Button Text="Remove item" HorizontalOptions="CenterAndExpand" Command="{Binding RemoveCommand}" />
</StackLayout>
<ListView x:Name="listView"
ItemsSource="{Binding FavoriteArticles}"
HasUnevenRows="True"
IsPullToRefreshEnabled="True"
HorizontalOptions="FillAndExpand"
VerticalOptions="FillAndExpand"
SeparatorVisibility="None"
SeparatorColor="Transparent"
BackgroundColor="#F0F0F0"
RefreshCommand="{Binding LoadFavoritesCommand}"
IsRefreshing="{Binding IsBusy, Mode=OneWay}">
<ListView.ItemTemplate>
<DataTemplate>
<ViewCell Height="85">
<!-- <ViewCell.ContextActions>
<MenuItem Clicked="OnDelete" CommandParameter="{Binding .}" Text="Delete" IsDestructive="True" />
</ViewCell.ContextActions>-->
<Grid Padding="10,5">
<Grid.ColumnDefinitions>
<ColumnDefinition Width="Auto" />
<ColumnDefinition Width="*" />
</Grid.ColumnDefinitions>
<StackLayout Grid.Column="1" Padding="5" Spacing="1" VerticalOptions="Center">
<Label Text="{Binding ArticleTitle}" FontSize="Medium" FontAttributes="Bold" TextColor="#333333" LineBreakMode="WordWrap" />
<Label Text="{Binding AuthorString}" FontSize="Small" TextColor="#919191" LineBreakMode="TailTruncation" />
<Label Text="{Binding FormattedPostedDate}" FontSize="Small" TextColor="#919191" LineBreakMode="NoWrap" />
<Label Text="{Binding ItemId}" FontSize="Small" TextColor="#919191" LineBreakMode="NoWrap" IsVisible="false" />
</StackLayout>
</Grid>
</ViewCell>
</DataTemplate>
</ListView.ItemTemplate>
</ListView>
</StackLayout>
</local:TestContentPage>

Просмотреть файл

@ -887,6 +887,9 @@
<Compile Include="$(MSBuildThisFileDirectory)Issue4314.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue5172.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue2204.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Issue4356.cs">
<DependentUpon>Issue4356.xaml</DependentUpon>
</Compile>
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">
@ -1004,6 +1007,9 @@
</EmbeddedResource>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)VisualControlsPage.xaml">
<SubType>Designer</SubType>
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue4356.xaml">
<Generator>MSBuild:UpdateDesignTimeXaml</Generator>
</EmbeddedResource>
</ItemGroup>

Просмотреть файл

@ -19,7 +19,6 @@ namespace Xamarin.Forms.Platform.iOS
{
const int DefaultRowHeight = 44;
ListViewDataSource _dataSource;
bool _estimatedRowHeight;
IVisualElementRenderer _headerRenderer;
IVisualElementRenderer _footerRenderer;
@ -66,7 +65,7 @@ namespace Xamarin.Forms.Platform.iOS
// You will remove it and test and find everything is fiiiiiine, but it is not fine, no it is far from fine. See iOS, or at least iOS 8
// has an issue where-by if the TableHeaderView happens to NOT be an integer height, it will add padding to the space between the content
// of the UITableView and the TableHeaderView to the tune of the difference between Math.Ceiling (height) - height. Now this seems fine
// and when you test it will be, EXCEPT that it does this every time you toggle the visibility of the UITableView causing the spacing to
// and when you test it will be, EXCEPT that it does this every time you toggle the visibility of the UITableView causing the spacing to
// grow a little each time, which you weren't testing at all were you? So there you have it, the stupid reason we integer align here.
//
// The same technically applies to the footer, though that could hardly matter less. We just do it for fun.
@ -240,7 +239,6 @@ namespace Xamarin.Forms.Platform.iOS
Control.Source = _dataSource = e.NewElement.HasUnevenRows ? new UnevenListViewDataSource(e.NewElement, _tableViewController) : new ListViewDataSource(e.NewElement, _tableViewController);
UpdateEstimatedRowHeight();
UpdateHeader();
UpdateFooter();
UpdatePullToRefreshEnabled();
@ -269,7 +267,6 @@ namespace Xamarin.Forms.Platform.iOS
_dataSource.UpdateGrouping();
else if (e.PropertyName == Xamarin.Forms.ListView.HasUnevenRowsProperty.PropertyName)
{
_estimatedRowHeight = false;
Control.Source = _dataSource = Element.HasUnevenRows ? new UnevenListViewDataSource(_dataSource) : new ListViewDataSource(_dataSource);
ReloadData();
}
@ -396,39 +393,6 @@ namespace Xamarin.Forms.Platform.iOS
}
}
void UpdateEstimatedRowHeight()
{
if (_estimatedRowHeight)
return;
// if even rows OR uneven rows but user specified a row height anyway...
if (!Element.HasUnevenRows || Element.RowHeight != -1)
{
Control.EstimatedRowHeight = 0;
_estimatedRowHeight = true;
return;
}
var source = _dataSource as UnevenListViewDataSource;
// We want to make sure we reset the cached defined row heights whenever this is called.
// Failing to do this will regress Bugzilla 43313
// (strange animation when adding rows with uneven heights)
//source?.CacheDefinedRowHeights();
if (source == null)
{
// We need to set a default estimated row height,
// because re-setting it later(when we have items on the TIL)
// will cause the UITableView to reload, and throw an Exception
Control.EstimatedRowHeight = DefaultRowHeight;
return;
}
Control.EstimatedRowHeight = source.GetEstimatedRowHeight(Control);
_estimatedRowHeight = true;
return;
}
void UpdateFooter()
{
@ -448,7 +412,7 @@ namespace Xamarin.Forms.Platform.iOS
return;
}
Control.TableFooterView = null;
_footerRenderer.Element?.DisposeModalAndChildRenderers();
_footerRenderer.Dispose();
_footerRenderer = null;
@ -554,8 +518,6 @@ namespace Xamarin.Forms.Platform.iOS
switch (e.Action)
{
case NotifyCollectionChangedAction.Add:
UpdateEstimatedRowHeight();
if (e.NewStartingIndex == -1 || groupReset)
goto case NotifyCollectionChangedAction.Reset;
@ -569,7 +531,7 @@ namespace Xamarin.Forms.Platform.iOS
DeleteRows(e.OldStartingIndex, e.OldItems.Count, section);
if (_estimatedRowHeight && TemplatedItemsView.TemplatedItems.Count == 0)
if (TemplatedItemsView.TemplatedItems.Count == 0)
InvalidateCellCache();
break;
@ -580,7 +542,7 @@ namespace Xamarin.Forms.Platform.iOS
MoveRows(e.NewStartingIndex, e.OldStartingIndex, e.OldItems.Count, section);
if (_estimatedRowHeight && e.OldStartingIndex == 0)
if (e.OldStartingIndex == 0)
InvalidateCellCache();
break;
@ -591,7 +553,7 @@ namespace Xamarin.Forms.Platform.iOS
ReloadRows(e.OldStartingIndex, e.OldItems.Count, section);
if (_estimatedRowHeight && e.OldStartingIndex == 0)
if (e.OldStartingIndex == 0)
InvalidateCellCache();
break;
@ -685,7 +647,6 @@ namespace Xamarin.Forms.Platform.iOS
void InvalidateCellCache()
{
_estimatedRowHeight = false;
_dataSource.InvalidatePrototypicalCellCache();
}
@ -711,7 +672,7 @@ namespace Xamarin.Forms.Platform.iOS
void UpdateSeparatorColor()
{
var color = Element.SeparatorColor;
// ...and Steve said to the unbelievers the separator shall be gray, and gray it was. The unbelievers looked on, and saw that it was good, and
// ...and Steve said to the unbelievers the separator shall be gray, and gray it was. The unbelievers looked on, and saw that it was good, and
// they went forth and documented the default color. The holy scripture still reflects this default.
// Defined here: https://developer.apple.com/library/ios/documentation/UIKit/Reference/UITableView_Class/#//apple_ref/occ/instp/UITableView/separatorColor
Control.SeparatorColor = color.ToUIColor(UIColor.Gray);
@ -751,7 +712,7 @@ namespace Xamarin.Forms.Platform.iOS
if (_tableViewController != null)
_tableViewController.UpdateRefreshControlColor(color == Color.Default ? null : color.ToUIColor());
}
void UpdateVerticalScrollBarVisibility()
{
if (_defaultVerticalScrollVisibility == null)
@ -804,7 +765,7 @@ namespace Xamarin.Forms.Platform.iOS
{
}
internal nfloat GetEstimatedRowHeight(UITableView table)
nfloat GetEstimatedRowHeight(UITableView table)
{
if (List.RowHeight != -1)
{
@ -840,7 +801,7 @@ namespace Xamarin.Forms.Platform.iOS
// we don't need to use estimatedRowHeight at all; zero will disable it and use the known heights.
// However, not setting the EstimatedRowHeight will drastically degrade performance with large lists.
// In this case, we will cache the specified cell heights asynchronously, which will be returned one time on
// table load by EstimatedHeight.
// table load by EstimatedHeight.
return 0;
}
@ -848,12 +809,21 @@ namespace Xamarin.Forms.Platform.iOS
return CalculateHeightForCell(table, firstCell);
}
internal override void InvalidatePrototypicalCellCache()
internal override void InvalidatingPrototypicalCellCache()
{
ClearPrototype();
_prototypicalCellByTypeOrDataTemplate.Clear();
}
protected override void UpdateEstimatedRowHeight(UITableView tableView)
{
var estimatedRowheight = GetEstimatedRowHeight(tableView);
//if we are providing 0 we are disabling EstimatedRowHeight,
//this works fine on newer versions, but iOS10 it will cause a crash so we leave the default value
if (estimatedRowheight == 0 && Forms.IsiOS11OrNewer)
tableView.EstimatedRowHeight = estimatedRowheight;
}
internal Cell GetPrototypicalCell(NSIndexPath indexPath)
{
var itemTypeOrDataTemplate = default(object);
@ -976,6 +946,9 @@ namespace Xamarin.Forms.Platform.iOS
bool _isDragging;
bool _selectionFromNative;
bool _disposed;
bool _wasEmpty;
bool _estimatedRowHeight;
public UITableViewRowAnimation ReloadSectionsAnimation { get; set; } = UITableViewRowAnimation.Automatic;
public ListViewDataSource(ListViewDataSource source)
@ -1003,12 +976,15 @@ namespace Xamarin.Forms.Platform.iOS
public Dictionary<int, int> Counts { get; set; }
UIColor DefaultBackgroundColor
UIColor DefaultBackgroundColor => UIColor.Clear;
internal void InvalidatePrototypicalCellCache()
{
get { return UIColor.Clear; }
_estimatedRowHeight = false;
InvalidatingPrototypicalCellCache();
}
internal virtual void InvalidatePrototypicalCellCache()
internal virtual void InvalidatingPrototypicalCellCache()
{
}
@ -1197,12 +1173,28 @@ namespace Xamarin.Forms.Platform.iOS
List.NotifyRowTapped(indexPath.Section, indexPath.Row, formsCell);
}
public override void WillDisplay(UITableView tableView, UITableViewCell cell, NSIndexPath indexPath)
{
if (!_estimatedRowHeight)
{
// Our cell size/estimate is out of date, probably because we moved from zero to one item; update it
DetermineEstimatedRowHeight();
}
}
public override nint RowsInSection(UITableView tableview, nint section)
{
int countOverride;
if (Counts.TryGetValue((int)section, out countOverride))
{
Counts.Remove((int)section);
if (_wasEmpty && countOverride > 0)
{
// We've moved from no items to having at least one item; it's likely that the layout needs to update
// its cell size/estimate
_estimatedRowHeight = false;
}
_wasEmpty = countOverride == 0;
return countOverride;
}
@ -1261,6 +1253,16 @@ namespace Xamarin.Forms.Platform.iOS
PerformWithoutAnimation(() => { _uiTableView.ReloadData(); });
}
public void DetermineEstimatedRowHeight()
{
if (_estimatedRowHeight)
return;
UpdateEstimatedRowHeight(_uiTableView);
_estimatedRowHeight = true;
}
protected bool IsValidIndexPath(NSIndexPath indexPath)
{
var templatedItems = TemplatedItemsView.TemplatedItems;
@ -1369,6 +1371,19 @@ namespace Xamarin.Forms.Platform.iOS
}
}
protected virtual void UpdateEstimatedRowHeight(UITableView tableView)
{
// We need to set a default estimated row height,
// because re-setting it later(when we have items on the TIL)
// will cause the UITableView to reload, and throw an Exception
tableView.EstimatedRowHeight = DefaultRowHeight;
// if even rows OR uneven rows but user specified a row height anyway...
if (!List.HasUnevenRows || List.RowHeight != -1)
tableView.EstimatedRowHeight = 0;
}
protected override void Dispose(bool disposing)
{
if (_disposed)
@ -1434,9 +1449,9 @@ namespace Xamarin.Forms.Platform.iOS
internal bool _usingLargeTitles;
bool _isRefreshing;
public FormsUITableViewController(ListView element, bool usingLargeTitles)
: base(element.OnThisPlatform().GetGroupHeaderStyle() == GroupHeaderStyle.Plain
? UITableViewStyle.Plain
public FormsUITableViewController(ListView element, bool usingLargeTitles)
: base(element.OnThisPlatform().GetGroupHeaderStyle() == GroupHeaderStyle.Plain
? UITableViewStyle.Plain
: UITableViewStyle.Grouped)
{
if (Forms.IsiOS9OrNewer)
@ -1463,7 +1478,7 @@ namespace Xamarin.Forms.Platform.iOS
{
_refresh.BeginRefreshing();
//hack: On iOS11 with large titles we need to adjust the scroll offset manually
//hack: On iOS11 with large titles we need to adjust the scroll offset manually
//since our UITableView is not the first child of the UINavigationController
UpdateContentOffset(TableView.ContentOffset.Y - _refresh.Frame.Height);
@ -1499,7 +1514,7 @@ namespace Xamarin.Forms.Platform.iOS
}
}
// https://bugzilla.xamarin.com/show_bug.cgi?id=52962
// just because pullToRefresh is being disabled does not mean we should kill an in progress refresh.
// just because pullToRefresh is being disabled does not mean we should kill an in progress refresh.
// Consider the case where:
// 1. User pulls to refresh
// 2. App RefreshCommand fires (at this point _refresh.Refreshing is true)
@ -1509,7 +1524,7 @@ namespace Xamarin.Forms.Platform.iOS
// 5. We end up here; A refresh is in progress while being asked to disable pullToRefresh
}
//hack: Form some reason UIKit isn't allowing to scroll negative values with largetitles
//hack: Form some reason UIKit isn't allowing to scroll negative values with largetitles
public void ForceRefreshing()
{
if (!_list.IsPullToRefreshEnabled)
@ -1543,6 +1558,11 @@ namespace Xamarin.Forms.Platform.iOS
UpdateIsRefreshing(true);
}
public override void ViewWillLayoutSubviews()
{
(TableView?.Source as ListViewRenderer.ListViewDataSource)?.DetermineEstimatedRowHeight();
}
public void UpdateRefreshControlColor(UIColor color)
{
if (RefreshControl != null)
@ -1610,7 +1630,6 @@ namespace Xamarin.Forms.Platform.iOS
}
}
public class FormsRefreshControl : UIRefreshControl
{
bool _usingLargeTitles;
@ -1628,7 +1647,7 @@ namespace Xamarin.Forms.Platform.iOS
}
set
{
//hack: ahahah take that UIKit!
//hack: ahahah take that UIKit!
//when using pull to refresh with Large tiles sometimes iOS tries to hide the UIRefreshControl
if (_usingLargeTitles && value && Refreshing)
return;