centralize bottom nav behaviors so all the bottom navs work the same (#5904)

* Reuse Bottom Nav more behavior for non shell

* - apply issues comments

Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs

@@ -19,6 +19,7 @@ using AView = Android.Views.View;
 using AMenu = Android.Views.Menu;
 using AColor = Android.Graphics.Color;
 using System.Threading.Tasks;
+using System.Collections.Generic;
 
 namespace Xamarin.Forms.Platform.Android.AppCompat
 {
@@ -463,7 +464,6 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 				else
 				{
 					SetupBottomNavigationView(e);
-					UpdateBottomNavigationViewIcons();
 					bottomNavigationView.SetOnNavigationItemSelectedListener(this);
 				}
 
@@ -631,56 +631,39 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			}
 		}
 
+		List<(string title, ImageSource icon, bool tabEnabled)> CreateTabList()
+		{
+			var items = new List<(string title, ImageSource icon, bool tabEnabled)>();
+
+			for (int i = 0; i < Element.Children.Count; i++)
+			{
+				var item = Element.Children[i];
+				items.Add((item.Title, item.IconImageSource, item.IsEnabled));
+			}
+
+			return items;
+		}
+
 		void SetupBottomNavigationView(NotifyCollectionChangedEventArgs e)
 		{
 			if (IsDisposed)
 				return;
 
-			BottomNavigationView bottomNavigationView = _bottomNavigationView;
+			var currentIndex = Element.Children.IndexOf(Element.CurrentPage);
+			var items = CreateTabList();
 
-			int startingIndex = 0;
-
-			if (e.Action == NotifyCollectionChangedAction.Add && e.NewStartingIndex == bottomNavigationView.Menu.Size())
-				startingIndex = e.NewStartingIndex;
-			else if (e.Action == NotifyCollectionChangedAction.Remove && (e.OldStartingIndex + 1) == bottomNavigationView.Menu.Size())
-			{
-				startingIndex = Element.Children.Count;
-				bottomNavigationView.Menu.RemoveItem(e.OldStartingIndex);
-			}
-			else
-				bottomNavigationView.Menu.Clear();
-
-
-			for (var i = startingIndex; i < Element.Children.Count; i++)
-			{
-				Page child = Element.Children[i];
-				var menuItem = bottomNavigationView.Menu.Add(AMenu.None, i, i, child.Title);
-				if (Element.CurrentPage == child)
-					bottomNavigationView.SelectedItemId = menuItem.ItemId;
-			}
+			BottomNavigationViewUtils.SetupMenu(
+				_bottomNavigationView.Menu,
+				_bottomNavigationView.MaxItemCount,
+				items,
+				currentIndex,
+				_bottomNavigationView,
+				Context);
 
 			if (Element.CurrentPage == null && Element.Children.Count > 0)
 				Element.CurrentPage = Element.Children[0];
 		}
 
-		void UpdateBottomNavigationViewIcons()
-		{
-			if (IsDisposed)
-				return;
-
-			BottomNavigationView bottomNavigationView = _bottomNavigationView;
-
-			for (var i = 0; i < Element.Children.Count; i++)
-			{
-				Page child = Element.Children[i];
-				var menuItem = bottomNavigationView.Menu.GetItem(i);
-				_ = this.ApplyDrawableAsync(child, Page.IconImageSourceProperty, Context, icon =>
-				{
-					menuItem.SetIcon(icon);
-				});
-			}
-		}
-
 		void UpdateTabIcons()
 		{
 			if (IsDisposed)
@@ -868,11 +851,46 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			if (Element == null || IsDisposed)
 				return false;
 
-			int selectedIndex = item.Order;
-			if (_bottomNavigationView.SelectedItemId != item.ItemId && Element.Children.Count > selectedIndex && selectedIndex >= 0)
+			var id = item.ItemId;
+			if (id == BottomNavigationViewUtils.MoreTabId)
+			{
+				var items = CreateTabList();
+				var bottomSheetDialog = BottomNavigationViewUtils.CreateMoreBottomSheet(OnMoreItemSelected, Context, items, _bottomNavigationView.MaxItemCount);
+				bottomSheetDialog.DismissEvent += OnMoreSheetDismissed;
+				bottomSheetDialog.Show();
+			}
+			else
+			{
+				if (_bottomNavigationView.SelectedItemId != item.ItemId && Element.Children.Count > item.ItemId)
+					Element.CurrentPage = Element.Children[item.ItemId];
+			}
+			return true;
+		}
+
+		void OnMoreSheetDismissed(object sender, EventArgs e)
+		{
+			var index = Element.Children.IndexOf(Element.CurrentPage);
+			using (var menu = _bottomNavigationView.Menu)
+			{
+				index = Math.Min(index, menu.Size() - 1);
+				if (index < 0)
+					return;
+				using (var menuItem = menu.GetItem(index))
+					menuItem.SetChecked(true);
+			}
+
+			if(sender is BottomSheetDialog bsd)
+				bsd.DismissEvent -= OnMoreSheetDismissed;
+		}
+
+		void OnMoreItemSelected(int selectedIndex, BottomSheetDialog dialog)
+		{
+			if (selectedIndex >= 0 && _bottomNavigationView.SelectedItemId != selectedIndex && Element.Children.Count > selectedIndex)
 				Element.CurrentPage = Element.Children[selectedIndex];
 
-			return true;
+			dialog.Dismiss();
+			dialog.DismissEvent -= OnMoreSheetDismissed;
+			dialog.Dispose();
 		}
 
 		bool IsDisposed
@@ -992,7 +1010,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat
 			return _emptyStateSet;
 		}
 
-		int[] GetStateSet(System.Collections.Generic.IList<int> stateSet)
+		int[] GetStateSet(IList<int> stateSet)
 		{
 			var results = new int[stateSet.Count];
 			for (int i = 0; i < results.Length; i++)

Xamarin.Forms.Platform.Android/Extensions/ImageViewExtensions.cs

@@ -1,4 +1,6 @@
 using System.Threading.Tasks;
+using Android.Content;
+using Android.Graphics;
 using AImageView = Android.Widget.ImageView;
 
 namespace Xamarin.Forms.Platform.Android
@@ -74,5 +76,10 @@ namespace Xamarin.Forms.Platform.Android
 				return (imageElement != null) ? imageElement.Source == imageSource : true;
 			}
 		}
+
+		internal static async void SetImage(this AImageView image, ImageSource source, Context context)
+		{
+			image.SetImageDrawable(await context.GetFormsDrawableAsync(source));
+		}
 	}
 }

Xamarin.Forms.Platform.Android/Renderers/BottomNavigationViewTracker.cs

@@ -0,0 +1,29 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+
+using Android.App;
+using Android.Content;
+using Android.OS;
+using Android.Runtime;
+using Android.Views;
+using Android.Widget;
+
+namespace Xamarin.Forms.Platform.Android
+{
+	internal class BottomNavigationViewTracker : IDisposable
+	{
+
+		#region IDisposable
+		bool _isDisposed = false;
+		public void Dispose()
+		{
+			if (_isDisposed)
+				return;
+
+			_isDisposed = true;
+		}
+		#endregion
+	}
+}

Xamarin.Forms.Platform.Android/Renderers/BottomNavigationViewUtils.cs

@@ -11,6 +11,16 @@ using Android.Views;
 using Android.Widget;
 using Android.Support.Design.Widget;
 using Android.Support.Design.Internal;
+using AColor = Android.Graphics.Color;
+using AView = Android.Views.View;
+using ColorStateList = Android.Content.Res.ColorStateList;
+using IMenu = Android.Views.IMenu;
+using LP = Android.Views.ViewGroup.LayoutParams;
+using Orientation = Android.Widget.Orientation;
+using Typeface = Android.Graphics.Typeface;
+using TypefaceStyle = Android.Graphics.TypefaceStyle;
+using Android.Graphics.Drawables;
+using System.Threading.Tasks;
 
 #if __ANDROID_28__
 using ALabelVisibilityMode = Android.Support.Design.BottomNavigation.LabelVisibilityMode;
@@ -20,6 +30,173 @@ namespace Xamarin.Forms.Platform.Android
 {
 	public static class BottomNavigationViewUtils
 	{
+		internal const int MoreTabId = 99;
+
+		public static Drawable CreateItemBackgroundDrawable()
+		{
+			var stateList = ColorStateList.ValueOf(Color.Black.MultiplyAlpha(0.2).ToAndroid());
+			return new RippleDrawable(stateList, new ColorDrawable(AColor.White), null);
+		}
+
+		internal static void UpdateEnabled(bool tabEnabled, IMenuItem menuItem)
+		{
+			if (menuItem.IsEnabled != tabEnabled)
+				menuItem.SetEnabled(tabEnabled);
+		}
+
+		internal static async void SetupMenu(
+			IMenu menu, 
+			int maxBottomItems, 
+			List<(string title, ImageSource icon, bool tabEnabled)> items, 
+			int currentIndex, 
+			BottomNavigationView bottomView, 
+			Context context)
+		{
+			menu.Clear();
+			int numberOfMenuItems = items.Count;
+			bool showMore = numberOfMenuItems > maxBottomItems;
+			int end = showMore ? maxBottomItems - 1 : numberOfMenuItems;
+
+
+			List<IMenuItem> menuItems = new List<IMenuItem>();
+			List<Task> loadTasks = new List<Task>();
+			for (int i = 0; i < end; i++)
+			{
+				var item = items[i];
+				using (var title = new Java.Lang.String(item.title))
+				{
+					var menuItem = menu.Add(0, i, 0, title);
+					menuItems.Add(menuItem);
+					loadTasks.Add(SetMenuItemIcon(menuItem, item.icon, context));
+					UpdateEnabled(item.tabEnabled, menuItem);
+					if (i == currentIndex)
+					{
+						menuItem.SetChecked(true);
+						bottomView.SelectedItemId = i;
+					}
+				}
+			}
+
+			if (showMore)
+			{
+				var moreString = new Java.Lang.String("More");
+				var menuItem = menu.Add(0, MoreTabId, 0, moreString);
+				menuItems.Add(menuItem);
+				moreString.Dispose();
+
+				menuItem.SetIcon(Resource.Drawable.abc_ic_menu_overflow_material);
+				if (currentIndex >= maxBottomItems - 1)
+					menuItem.SetChecked(true);
+			}
+
+			bottomView.SetShiftMode(false, false);
+
+			if (loadTasks.Count > 0)
+				await Task.WhenAll(loadTasks);
+
+			foreach (var menuItem in menuItems)
+				menuItem.Dispose();
+		}
+
+		static async Task SetMenuItemIcon(IMenuItem menuItem, ImageSource source, Context context)
+		{
+			if (source == null)
+				return;
+			var drawable = await context.GetFormsDrawableAsync(source);
+			menuItem.SetIcon(drawable);			
+			drawable?.Dispose();
+		}
+
+
+		public static BottomSheetDialog CreateMoreBottomSheet(
+			Action<int, BottomSheetDialog> selectCallback,
+			Context context,
+			List<(string title, ImageSource icon, bool tabEnabled)> items)
+		{
+			return CreateMoreBottomSheet(selectCallback, context, items, 5);
+		}
+
+		internal static BottomSheetDialog CreateMoreBottomSheet(
+			Action<int, BottomSheetDialog> selectCallback, 
+			Context context,
+			List<(string title, ImageSource icon, bool tabEnabled)> items,
+			int maxItemCount)
+		{
+			var bottomSheetDialog = new BottomSheetDialog(context);
+			var bottomSheetLayout = new LinearLayout(context);
+			using (var bottomShellLP = new LP(LP.MatchParent, LP.WrapContent))
+				bottomSheetLayout.LayoutParameters = bottomShellLP;
+			bottomSheetLayout.Orientation = Orientation.Vertical;
+
+			// handle the more tab
+			for (int i = maxItemCount - 1; i < items.Count; i++)
+			{
+				var i_local = i;
+				var shellContent = items[i];
+
+				using (var innerLayout = new LinearLayout(context))
+				{
+					innerLayout.ClipToOutline = true;
+					innerLayout.SetBackground(CreateItemBackgroundDrawable());
+					innerLayout.SetPadding(0, (int)context.ToPixels(6), 0, (int)context.ToPixels(6));
+					innerLayout.Orientation = Orientation.Horizontal;
+					using (var param = new LP(LP.MatchParent, LP.WrapContent))
+						innerLayout.LayoutParameters = param;
+
+					// technically the unhook isn't needed
+					// we dont even unhook the events that dont fire
+					void clickCallback(object s, EventArgs e)
+					{
+						selectCallback(i_local, bottomSheetDialog);
+						if (!innerLayout.IsDisposed())
+							innerLayout.Click -= clickCallback;
+					}
+					innerLayout.Click += clickCallback;
+
+					var image = new ImageView(context);
+					var lp = new LinearLayout.LayoutParams((int)context.ToPixels(32), (int)context.ToPixels(32))
+					{
+						LeftMargin = (int)context.ToPixels(20),
+						RightMargin = (int)context.ToPixels(20),
+						TopMargin = (int)context.ToPixels(6),
+						BottomMargin = (int)context.ToPixels(6),
+						Gravity = GravityFlags.Center
+					};
+					image.LayoutParameters = lp;
+					lp.Dispose();
+
+					image.ImageTintList = ColorStateList.ValueOf(Color.Black.MultiplyAlpha(0.6).ToAndroid());
+					image.SetImage(shellContent.icon, context);
+
+					innerLayout.AddView(image);
+
+					using (var text = new TextView(context))
+					{
+						text.SetTypeface(Typeface.Create("sans-serif-medium", TypefaceStyle.Normal), TypefaceStyle.Normal);
+						text.SetTextColor(AColor.Black);
+						text.Text = shellContent.title;
+						lp = new LinearLayout.LayoutParams(0, LP.WrapContent)
+						{
+							Gravity = GravityFlags.Center,
+							Weight = 1
+						};
+						text.LayoutParameters = lp;
+						lp.Dispose();
+
+						innerLayout.AddView(text);
+					}
+
+					bottomSheetLayout.AddView(innerLayout);
+				}
+			}
+
+			bottomSheetDialog.SetContentView(bottomSheetLayout);
+			bottomSheetLayout.Dispose();
+
+			return bottomSheetDialog;
+		}
+
+
 		public static void SetShiftMode(this BottomNavigationView bottomNavigationView, bool enableShiftMode, bool enableItemShiftMode)
 		{
 			try

Xamarin.Forms.Platform.Android/Renderers/ShellItemRenderer.cs

@@ -48,6 +48,7 @@ namespace Xamarin.Forms.Platform.Android
 		FrameLayout _navigationArea;
 		AView _outerLayout;
 		IShellBottomNavViewAppearanceTracker _appearanceTracker;
+		BottomNavigationViewTracker _bottomNavigationTracker;
 
 		public ShellItemRenderer(IShellContext shellContext) : base(shellContext)
 		{
@@ -64,13 +65,14 @@ namespace Xamarin.Forms.Platform.Android
 			_bottomView.SetBackgroundColor(Color.White.ToAndroid());
 			_bottomView.SetOnNavigationItemSelectedListener(this);
 
-			if(ShellItem == null)
+			if (ShellItem == null)
 				throw new ArgumentException("Active Shell Item not set. Have you added any Shell Items to your Shell?", nameof(ShellItem));
 
 			HookEvents(ShellItem);
 			SetupMenu();
 
 			_appearanceTracker = ShellContext.CreateBottomNavViewAppearanceTracker(ShellItem);
+			_bottomNavigationTracker = new BottomNavigationViewTracker();
 			((IShellController)ShellContext.Shell).AddAppearanceObserver(this, ShellItem);
 
 			return _outerLayout;
@@ -111,20 +113,30 @@ namespace Xamarin.Forms.Platform.Android
 
 		protected virtual Drawable CreateItemBackgroundDrawable()
 		{
-			var stateList = ColorStateList.ValueOf(Color.Black.MultiplyAlpha(0.2).ToAndroid());
-			return new RippleDrawable(stateList, new ColorDrawable(AColor.White), null);
+			return BottomNavigationViewUtils.CreateItemBackgroundDrawable();
 		}
 
+		[Obsolete("Use CreateMoreBottomSheet(Action<int, BottomSheetDialog> selectCallback)")]
 		protected virtual BottomSheetDialog CreateMoreBottomSheet(Action<ShellSection, BottomSheetDialog> selectCallback)
+		{
+			return CreateMoreBottomSheet((int index, BottomSheetDialog dialog) =>
+			{
+				selectCallback(ShellItem.Items[index], dialog);
+			});
+		}
+
+		protected virtual BottomSheetDialog CreateMoreBottomSheet(Action<int, BottomSheetDialog> selectCallback)
 		{
 			var bottomSheetDialog = new BottomSheetDialog(Context);
 			var bottomSheetLayout = new LinearLayout(Context);
 			using (var bottomShellLP = new LP(LP.MatchParent, LP.WrapContent))
 				bottomSheetLayout.LayoutParameters = bottomShellLP;
 			bottomSheetLayout.Orientation = Orientation.Vertical;
+
 			// handle the more tab
-			for (int i = 4; i < ShellItem.Items.Count; i++)
+			for (int i = _bottomView.MaxItemCount - 1; i < ShellItem.Items.Count; i++)
 			{
+				var closure_i = i;
 				var shellContent = ShellItem.Items[i];
 
 				using (var innerLayout = new LinearLayout(Context))
@@ -140,10 +152,11 @@ namespace Xamarin.Forms.Platform.Android
 					// we dont even unhook the events that dont fire
 					void clickCallback(object s, EventArgs e)
 					{
-						selectCallback(shellContent, bottomSheetDialog);
+						selectCallback(closure_i, bottomSheetDialog);
 						if (!innerLayout.IsDisposed())
 							innerLayout.Click -= clickCallback;
 					}
+
 					innerLayout.Click += clickCallback;
 
 					var image = new ImageView(Context);
@@ -228,7 +241,8 @@ namespace Xamarin.Forms.Platform.Android
 			var id = item.ItemId;
 			if (id == MoreTabId)
 			{
-				var bottomSheetDialog = CreateMoreBottomSheet(OnMoreItemSelected);
+				var items = CreateTabList(ShellItem);
+				var bottomSheetDialog = BottomNavigationViewUtils.CreateMoreBottomSheet(OnMoreItemSelected, Context, items, _bottomView.MaxItemCount);
 				bottomSheetDialog.Show();
 				bottomSheetDialog.DismissEvent += OnMoreSheetDismissed;
 			}
@@ -248,6 +262,11 @@ namespace Xamarin.Forms.Platform.Android
 			return true;
 		}
 
+		void OnMoreItemSelected(int shellSectionIndex, BottomSheetDialog dialog)
+		{
+			OnMoreItemSelected(ShellItem.Items[shellSectionIndex], dialog);
+		}
+
 		protected virtual void OnMoreItemSelected(ShellSection shellSection, BottomSheetDialog dialog)
 		{
 			ChangeSection(shellSection);
@@ -256,6 +275,18 @@ namespace Xamarin.Forms.Platform.Android
 			dialog.Dispose();
 		}
 
+		List<(string title, ImageSource icon, bool tabEnabled)> CreateTabList(ShellItem shellItem)
+		{
+			var items = new List<(string title, ImageSource icon, bool tabEnabled)>();
+
+			for (int i = 0; i < shellItem.Items.Count; i++)
+			{
+				var item = shellItem.Items[i];
+				items.Add((item.Title, item.Icon, item.IsEnabled));
+			}
+			return items;
+		}
+
 		protected virtual void OnMoreSheetDismissed(object sender, EventArgs e) => OnShellSectionChanged();
 
 		protected override void OnShellItemsChanged(object sender, NotifyCollectionChangedEventArgs e)
@@ -296,57 +327,20 @@ namespace Xamarin.Forms.Platform.Android
 
 		protected virtual void ResetAppearance() => _appearanceTracker.ResetAppearance(_bottomView);
 
-		protected virtual async void SetupMenu(IMenu menu, int maxBottomItems, ShellItem shellItem)
+		protected virtual void SetupMenu(IMenu menu, int maxBottomItems, ShellItem shellItem)
 		{
-			menu.Clear();
-			bool showMore = ShellItem.Items.Count > maxBottomItems;
-
-			int end = showMore ? maxBottomItems - 1 : ShellItem.Items.Count;
-
 			var currentIndex = shellItem.Items.IndexOf(ShellSection);
+			var items = CreateTabList(shellItem);
 
-			List<IMenuItem> menuItems = new List<IMenuItem>();
-			List<Task> loadTasks = new List<Task>();
-			for (int i = 0; i < end; i++)
-			{
-				var item = shellItem.Items[i];
-				using (var title = new Java.Lang.String(item.Title))
-				{
-					var menuItem = menu.Add(0, i, 0, title);
-					menuItems.Add(menuItem);
-					loadTasks.Add(ShellContext.ApplyDrawableAsync(item, ShellSection.IconProperty, icon =>
-					{
-						if (icon != null)
-							menuItem.SetIcon(icon);
-					}));
-					UpdateShellSectionEnabled(item, menuItem);
-					if (item == ShellSection)
-					{
-						menuItem.SetChecked(true);
-					}
-				}
-			}
-
-			if (showMore)
-			{
-				var moreString = new Java.Lang.String("More");
-				var menuItem = menu.Add(0, MoreTabId, 0, moreString);
-				moreString.Dispose();
-
-				menuItem.SetIcon(Resource.Drawable.abc_ic_menu_overflow_material);
-				if (currentIndex >= maxBottomItems - 1)
-					menuItem.SetChecked(true);
-			}
+			BottomNavigationViewUtils.SetupMenu(
+				menu,
+				maxBottomItems,
+				items,
+				currentIndex,
+				_bottomView,
+				Context);
 
 			UpdateTabBarVisibility();
-
-			_bottomView.SetShiftMode(false, false);
-
-			if (loadTasks.Count > 0)
-				await Task.WhenAll(loadTasks);
-
-			foreach (var menuItem in menuItems)
-				menuItem.Dispose();
 		}
 
 		protected virtual void UpdateShellSectionEnabled(ShellSection shellSection, IMenuItem menuItem)

Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj

@@ -167,6 +167,7 @@
     <Compile Include="IPickerRenderer.cs" />
     <Compile Include="PickerManager.cs" />
     <Compile Include="EntryAccessibilityDelegate.cs" />
+    <Compile Include="Renderers\BottomNavigationViewTracker.cs" />
     <Compile Include="Renderers\CircularProgress.cs" />
     <Compile Include="Renderers\PickerEditText.cs" />
     <Compile Include="Renderers\FontImageSourceHandler.cs" />