Fix FlyoutItem recycling (#13681)

* Correctly Disable View Recycling Android Flyout * - fix android to not reset element when items are removed * - fix iOS to not always recreate cells
xamarin · Feb 9, 2021 · 053ede6 · 053ede6
1 parent e08748c
commit 053ede6
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 26 deletions.
diff --git a/Xamarin.Forms.Platform.Android/Renderers/ShellFlyoutRecyclerAdapter.cs b/Xamarin.Forms.Platform.Android/Renderers/ShellFlyoutRecyclerAdapter.cs
@@ -16,13 +16,12 @@ public class ShellFlyoutRecyclerAdapter : RecyclerView.Adapter
 		readonly IShellContext _shellContext;
 		List<AdapterListItem> _listItems;
 		List<List<Element>> _flyoutGroupings;
-		Dictionary<int, DataTemplate> _templateMap = new Dictionary<int, DataTemplate>();
 		Action<Element> _selectedCallback;
 		bool _disposed;
-		ElementViewHolder _elementViewHolder;
 
 		public ShellFlyoutRecyclerAdapter(IShellContext shellContext, Action<Element> selectedCallback)
 		{
+			HasStableIds = true;
 			_shellContext = shellContext;
 
 			ShellController.FlyoutItemsChanged += OnFlyoutItemsChanged;
@@ -43,7 +42,22 @@ public ShellFlyoutRecyclerAdapter(IShellContext shellContext, Action<Element> se
 
 		public override int GetItemViewType(int position)
 		{
-			var item = _listItems[position];
+			return _listItems[position].Index;
+		}
+
+		DataTemplate GetDataTemplate(int viewTypeId)
+		{
+			AdapterListItem item = null;
+
+			foreach(var ali in _listItems)
+			{
+				if(viewTypeId == ali.Index)
+				{
+					item = ali;
+					break;
+				}
+			}
+
 			DataTemplate dataTemplate = ShellController.GetFlyoutItemDataTemplate(item.Element);
 			if (item.Element is IMenuItemController)
 			{
@@ -57,18 +71,26 @@ public override int GetItemViewType(int position)
 			}
 
 			var template = dataTemplate.SelectDataTemplate(item.Element, Shell);
-			var id = ((IDataTemplateController)template).Id;
-
-			_templateMap[id] = template;
-
-			return id;
+			return template;
 		}
 
 		public override void OnViewRecycled(Java.Lang.Object holder)
 		{
 			if (holder is ElementViewHolder evh)
 			{
-				evh.Element = null;
+				// only clear out the Element if the item has been removed
+				bool found = false;
+				foreach(var item in _listItems)
+				{
+					if(item.Element == evh.Element)
+					{
+						found = true;
+						break;
+					}
+				}
+
+				if(!found)
+					evh.Element = null;
 			}
 
 			base.OnViewRecycled(holder);
@@ -154,7 +176,7 @@ public override AView FocusSearch([GeneratedEnum] FocusSearchDirection direction
 
 		public override RecyclerView.ViewHolder OnCreateViewHolder(ViewGroup parent, int viewType)
 		{
-			var template = _templateMap[viewType];
+			var template = GetDataTemplate(viewType);
 
 			var content = (View)template.CreateContent();
 
@@ -175,14 +197,13 @@ public override RecyclerView.ViewHolder OnCreateViewHolder(ViewGroup parent, int
 			container.LayoutParameters = new LP(LP.MatchParent, LP.WrapContent);
 			linearLayout.AddView(container);
 
-			_elementViewHolder = new ElementViewHolder(content, linearLayout, bar, _selectedCallback, _shellContext.Shell);
-
-			return _elementViewHolder;
+			return new ElementViewHolder(content, linearLayout, bar, _selectedCallback, _shellContext.Shell);
 		}
 
 		protected virtual List<AdapterListItem> GenerateItemList()
 		{
 			var result = new List<AdapterListItem>();
+			_listItems = _listItems ?? result;
 
 			List<List<Element>> grouping = ((IShellController)_shellContext.Shell).GenerateFlyoutGrouping();
 
@@ -198,7 +219,18 @@ protected virtual List<AdapterListItem> GenerateItemList()
 				bool first = !skip;
 				foreach (var element in sublist)
 				{
-					result.Add(new AdapterListItem(element, first));
+					AdapterListItem toAdd = null;
+					foreach (var existingItem in _listItems)
+					{
+						if(existingItem.Element == element)
+						{
+							existingItem.DrawTopLine = first;
+							toAdd = existingItem;
+						}
+					}
+
+					toAdd = toAdd ?? new AdapterListItem(element, first);
+					result.Add(toAdd);
 					first = false;
 				}
 				skip = false;
@@ -229,24 +261,27 @@ protected override void Dispose(bool disposing)
 			{
 				((IShellController)Shell).FlyoutItemsChanged -= OnFlyoutItemsChanged;
 
-				_elementViewHolder?.Dispose();
-
 				_listItems = null;
 				_selectedCallback = null;
-				_elementViewHolder = null;
 			}
 
 			base.Dispose(disposing);
 		}
 
 		public class AdapterListItem
 		{
+			// This ensures that we have a stable id for each element
+			// if the elements change position
+			static int IndexCounter = 0;
+
 			public AdapterListItem(Element element, bool drawTopLine = false)
 			{
 				DrawTopLine = drawTopLine;
 				Element = element;
+				Index = IndexCounter++;
 			}
 
+			public int Index { get; }
 			public bool DrawTopLine { get; set; }
 			public Element Element { get; set; }
 		}

diff --git a/Xamarin.Forms.Platform.iOS/Renderers/ShellTableViewController.cs b/Xamarin.Forms.Platform.iOS/Renderers/ShellTableViewController.cs
@@ -62,7 +62,7 @@ protected ShellTableViewSource CreateShellTableViewSource()
 
 		void OnFlyoutItemsChanged(object sender, EventArgs e)
 		{
-			_source.ClearCache();
+			_source.ReSyncCache();
 			TableView.ReloadData();
 			ShellFlyoutContentManager.UpdateVerticalScrollMode();
 		}

diff --git a/Xamarin.Forms.Platform.iOS/Renderers/ShellTableViewSource.cs b/Xamarin.Forms.Platform.iOS/Renderers/ShellTableViewSource.cs
@@ -47,6 +47,41 @@ public List<List<Element>> Groups
 
 		protected virtual DataTemplate DefaultMenuItemTemplate => null;
 
+		internal void ReSyncCache()
+		{
+			var newGroups = ((IShellController)_context.Shell).GenerateFlyoutGrouping();
+
+			if (newGroups == _groups)
+				return;
+
+			_groups = newGroups;
+			if (_cells == null)
+			{
+				_cells = new Dictionary<Element, UIContainerCell>();
+				return;
+			}
+
+			var oldList = _cells;
+			_cells = new Dictionary<Element, UIContainerCell>();
+
+			foreach (var group in newGroups)
+			{
+				foreach(var element in group)
+				{
+					UIContainerCell result;
+					if(oldList.TryGetValue(element, out result))
+					{
+						_cells.Add(element, result);
+						oldList.Remove(element);
+					}
+				}
+			}
+
+			foreach (var cell in oldList.Values)
+				cell.Disconnect(_context.Shell);
+		}
+
+
 		public void ClearCache()
 		{
 			var newGroups = ((IShellController)_context.Shell).GenerateFlyoutGrouping();
@@ -125,7 +160,7 @@ public override UITableViewCell GetCell(UITableView tableView, NSIndexPath index
 			else
 			{
 				var view = _cells[context].View;
-				cell.Disconnect();
+				cell.Disconnect(keepRenderer: true);
 				cell = new UIContainerCell(cellId, view, _context.Shell, context);
 			}
 
@@ -148,8 +183,10 @@ public override nfloat GetHeightForFooter(UITableView tableView, nint section)
 		{
 			if (section < Groups.Count - 1)
 				return 1;
+
 			return 0;
 		}
+
 		public override UIView GetViewForFooter(UITableView tableView, nint section)
 		{
 			return new SeparatorView();

diff --git a/Xamarin.Forms.Platform.iOS/Renderers/UIContainerCell.cs b/Xamarin.Forms.Platform.iOS/Renderers/UIContainerCell.cs
@@ -18,9 +18,14 @@ internal UIContainerCell(string cellId, View view, Shell shell, object context)
 			View = view;
 			View.MeasureInvalidated += MeasureInvalidated;
 			SelectionStyle = UITableViewCellSelectionStyle.None;
-
-			_renderer = Platform.CreateRenderer(view);
-			Platform.SetRenderer(view, _renderer);
+
+			_renderer = Platform.GetRenderer(view);
+
+			if (_renderer == null)
+			{
+				_renderer = Platform.CreateRenderer(view);
+				Platform.SetRenderer(view, _renderer);
+			}
 
 			ContentView.AddSubview(_renderer.NativeView);
 			_renderer.NativeView.ClipsToBounds = true;
@@ -49,15 +54,18 @@ internal void ReloadRow()
 			TableView.ReloadRows(new[] { IndexPath }, UITableViewRowAnimation.Automatic);
 		}
 
-		internal void Disconnect(Shell shell = null)
+		internal void Disconnect(Shell shell = null, bool keepRenderer = false)
 		{
 			ViewMeasureInvalidated = null;
 			View.MeasureInvalidated -= MeasureInvalidated;
 			if (_bindingContext != null && _bindingContext is BaseShellItem baseShell)
 				baseShell.PropertyChanged -= OnElementPropertyChanged;
 
 			_bindingContext = null;
-			Platform.SetRenderer(View, null);
+
+			if (!keepRenderer)
+				Platform.SetRenderer(View, null);
+
 			if (shell != null)
 				shell.RemoveLogicalChild(shell);
 
@@ -70,7 +78,8 @@ internal void Disconnect(Shell shell = null)
 		public object BindingContext
 		{
 			get => _bindingContext;
-			set {
+			set
+			{
 				if (value == _bindingContext)
 					return;
 
@@ -91,7 +100,7 @@ public object BindingContext
 		public override void LayoutSubviews()
 		{
 			base.LayoutSubviews();
-			if(View != null)
+			if (View != null)
 				View.Layout(Bounds.ToRectangle());
 		}