[Shell, iOS, Android] added tab order on Shell flyout menu items (#5930)

* [Shell, Android] added tab order on Shell flyout menu items

* [iOS mac] fix build

* support iOS

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

@@ -0,0 +1,54 @@
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using NUnit.Framework;
+using Xamarin.UITest;
+using Xamarin.Forms.Core.UITests;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+#if UITEST
+	[Category(UITestCategories.ManualReview)]
+#endif
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 5131, "Tab order on Shell flyout menu items", PlatformAffected.Default)]
+	public class Issue5131 : TestShell
+	{
+		ShellItem GenerateItem(string title, int tabIndex, bool tabStop)
+		{
+			return new ShellItem
+			{
+				TabIndex = tabIndex,
+				IsTabStop = tabStop,
+				Title = title,
+				Route = $"{title}.{tabIndex}",
+				Items =
+				{
+					new ShellSection
+					{
+						Items =
+						{
+							new Forms.ShellContent
+							{
+								Content = new ContentPage()
+							}
+						}
+					}
+				}
+			};
+		}
+
+		protected override void Init()
+		{
+			StackLayout flyout = new StackLayout();
+			FlowDirection = FlowDirection.RightToLeft;
+			FlyoutHeader = flyout;
+			Items.Add(GenerateItem("First", 1, true));
+			Items.Add(GenerateItem("Third", 3, true));
+			Items.Add(GenerateItem("Skip", 2, false));
+			Items.Add(GenerateItem("Second", 2, true));
+		}
+	}
+}

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

@@ -11,6 +11,7 @@
   <ItemGroup>
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla59172.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue4684.xaml.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue5131.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue5376.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Bugzilla60787.xaml.cs">
       <DependentUpon>Bugzilla60787.xaml</DependentUpon>

Xamarin.Forms.Core/Shell/BaseShellItem.cs

@@ -6,7 +6,7 @@ using Xamarin.Forms.Internals;
 namespace Xamarin.Forms
 {
 	[DebuggerDisplay("Title = {Title}, Route = {Route}")]
-	public class BaseShellItem : NavigableElement, IPropertyPropagationController, IVisualController, IFlowDirectionController
+	public class BaseShellItem : NavigableElement, IPropertyPropagationController, IVisualController, IFlowDirectionController, ITabStopElement
 	{
 		#region PropertyKeys
 
@@ -29,6 +29,34 @@ namespace Xamarin.Forms
 		public static readonly BindableProperty TitleProperty =
 			BindableProperty.Create(nameof(Title), typeof(string), typeof(BaseShellItem), null, BindingMode.OneTime);
 
+		public static readonly BindableProperty TabIndexProperty =
+			BindableProperty.Create(nameof(TabIndex),
+							typeof(int),
+							typeof(BaseShellItem),
+							defaultValue: 0,
+							propertyChanged: OnTabIndexPropertyChanged,
+							defaultValueCreator: TabIndexDefaultValueCreator);
+
+		public static readonly BindableProperty IsTabStopProperty =
+			BindableProperty.Create(nameof(IsTabStop),
+									typeof(bool),
+									typeof(BaseShellItem),
+									defaultValue: true,
+									propertyChanged: OnTabStopPropertyChanged,
+									defaultValueCreator: TabStopDefaultValueCreator);
+
+		static void OnTabIndexPropertyChanged(BindableObject bindable, object oldValue, object newValue) =>
+			((BaseShellItem)bindable).OnTabIndexPropertyChanged((int)oldValue, (int)newValue);
+
+		static object TabIndexDefaultValueCreator(BindableObject bindable) =>
+			((BaseShellItem)bindable).TabIndexDefaultValueCreator();
+
+		static void OnTabStopPropertyChanged(BindableObject bindable, object oldValue, object newValue) =>
+			((BaseShellItem)bindable).OnTabStopPropertyChanged((bool)oldValue, (bool)newValue);
+
+		static object TabStopDefaultValueCreator(BindableObject bindable) =>
+			((BaseShellItem)bindable).TabStopDefaultValueCreator();
+
 		public ImageSource FlyoutIcon
 		{
 			get { return (ImageSource)GetValue(FlyoutIconProperty); }
@@ -61,6 +89,26 @@ namespace Xamarin.Forms
 			set { SetValue(TitleProperty, value); }
 		}
 
+		public int TabIndex
+		{
+			get => (int)GetValue(TabIndexProperty);
+			set => SetValue(TabIndexProperty, value);
+		}
+
+		protected virtual void OnTabIndexPropertyChanged(int oldValue, int newValue) { }
+
+		protected virtual int TabIndexDefaultValueCreator() => 0;
+
+		public bool IsTabStop
+		{
+			get => (bool)GetValue(IsTabStopProperty);
+			set => SetValue(IsTabStopProperty, value);
+		}
+
+		protected virtual void OnTabStopPropertyChanged(bool oldValue, bool newValue) { }
+
+		protected virtual bool TabStopDefaultValueCreator() => true;
+
 		IVisual _effectiveVisual = Xamarin.Forms.VisualMarker.Default;
 		IVisual IVisualController.EffectiveVisual
 		{

Xamarin.Forms.Core/Shell/ITabStopElement.cs

@@ -0,0 +1,8 @@
+namespace Xamarin.Forms
+{
+	public interface ITabStopElement
+	{
+		int TabIndex { get; set; }
+		bool IsTabStop { get; set; }
+	}
+}

Xamarin.Forms.Core/TabIndexExtensions.cs

@@ -1,42 +1,47 @@
 using System.Collections.Generic;
 using Xamarin.Forms.Internals;
 using System.Linq;
+using System;
 
 namespace Xamarin.Forms
 {
 	public static class TabIndexExtensions
 	{
-		public static SortedDictionary<int, List<VisualElement>> GetSortedTabIndexesOnParentPage(this VisualElement element, out int countChildrensWithTabStopWithoutThis)
+		public static SortedDictionary<int, List<ITabStopElement>> GetSortedTabIndexesOnParentPage(this VisualElement element, out int countChildrensWithTabStopWithoutThis)
 		{
-			return new SortedDictionary<int, List<VisualElement>>(TabIndexExtensions.GetTabIndexesOnParentPage(element, out countChildrensWithTabStopWithoutThis));
+			return new SortedDictionary<int, List<ITabStopElement>>(TabIndexExtensions.GetTabIndexesOnParentPage(element, out countChildrensWithTabStopWithoutThis));
 		}
 
-		public static IDictionary<int, List<VisualElement>> GetTabIndexesOnParentPage(this VisualElement element, out int countChildrensWithTabStopWithoutThis)
+		public static IDictionary<int, List<ITabStopElement>> GetTabIndexesOnParentPage(this ITabStopElement element, out int countChildrensWithTabStopWithoutThis, bool checkContainsElement = true)
 		{
 			countChildrensWithTabStopWithoutThis = 0;
 
-			Element parentPage = element.Parent;
+			Element parentPage = (element as NavigableElement).Parent;
 			while (parentPage != null && !(parentPage is Page))
 				parentPage = parentPage.Parent;
 
 			var descendantsOnPage = parentPage?.VisibleDescendants();
+
+			if (parentPage is Shell shell)
+				descendantsOnPage = shell.Items;
+
 			if (descendantsOnPage == null)
 				return null;
 
-			var childrensWithTabStop = new List<VisualElement>();
+			var childrensWithTabStop = new List<ITabStopElement>();
 			foreach (var descendant in descendantsOnPage)
 			{
-				if (descendant is VisualElement visualElement && visualElement.IsTabStop)
+				if (descendant is ITabStopElement visualElement && visualElement.IsTabStop)
 					childrensWithTabStop.Add(visualElement);
 			}
-			if (!childrensWithTabStop.Contains(element))
+			if (checkContainsElement && !childrensWithTabStop.Contains(element))
 				return null;
 
 			countChildrensWithTabStopWithoutThis = childrensWithTabStop.Count - 1;
 			return childrensWithTabStop.GroupToDictionary(c => c.TabIndex);
 		}
 
-		public static VisualElement FindNextElement(this VisualElement element, bool forwardDirection, IDictionary<int, List<VisualElement>> tabIndexes, ref int tabIndex)
+		public static ITabStopElement FindNextElement(this ITabStopElement element, bool forwardDirection, IDictionary<int, List<ITabStopElement>> tabIndexes, ref int tabIndex) 
 		{
 			if (!tabIndexes.TryGetValue(tabIndex, out var tabGroup))
 				return null;

Xamarin.Forms.Core/VisualElement.cs

@@ -6,7 +6,7 @@ using Xamarin.Forms.Internals;
 
 namespace Xamarin.Forms
 {
-	public partial class VisualElement : NavigableElement, IAnimatable, IVisualElementController, IResourcesProvider, IStyleElement, IFlowDirectionController, IPropertyPropagationController, IVisualController
+	public partial class VisualElement : NavigableElement, IAnimatable, IVisualElementController, IResourcesProvider, IStyleElement, IFlowDirectionController, IPropertyPropagationController, IVisualController, ITabStopElement
 	{
 		public new static readonly BindableProperty NavigationProperty = NavigableElement.NavigationProperty;
 

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

@@ -149,7 +149,7 @@ namespace Xamarin.Forms.Platform.Android
 			if (!am.IsEnabled)
 				return;
 
-			SortedDictionary<int, List<VisualElement>> tabIndexes = null;
+			SortedDictionary<int, List<ITabStopElement>> tabIndexes = null;
 			foreach (var child in Element.LogicalChildren)
 			{
 				if (!(child is VisualElement ve))

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

@@ -1,8 +1,10 @@
-using Android.Support.V7.Widget;
+using Android.Runtime;
+using Android.Support.V7.Widget;
 using Android.Views;
 using Android.Widget;
 using System;
 using System.Collections.Generic;
+using System.ComponentModel;
 using Xamarin.Forms.Internals;
 using AView = Android.Views.View;
 using LP = Android.Views.ViewGroup.LayoutParams;
@@ -68,15 +70,83 @@ namespace Xamarin.Forms.Platform.Android
 			elementHolder.Element = item.Element;
 		}
 
+		class LinearLayoutWithFocus : LinearLayout, ITabStop, IVisualElementRenderer
+		{
+			public LinearLayoutWithFocus(global::Android.Content.Context context) : base(context)
+			{
+			}
+
+			AView ITabStop.TabStop => this;
+
+#region IVisualElementRenderer
+
+			VisualElement IVisualElementRenderer.Element => Content?.BindingContext as VisualElement;
+
+			VisualElementTracker IVisualElementRenderer.Tracker => null;
+
+			ViewGroup IVisualElementRenderer.ViewGroup => this;
+
+			AView IVisualElementRenderer.View => this;
+
+			SizeRequest IVisualElementRenderer.GetDesiredSize(int widthConstraint, int heightConstraint) => new SizeRequest(new Size(100, 100));
+
+			void IVisualElementRenderer.SetElement(VisualElement element) { }
+
+			void IVisualElementRenderer.SetLabelFor(int? id) { }
+
+			void IVisualElementRenderer.UpdateLayout() { }
+
+#pragma warning disable 67
+			public event EventHandler<VisualElementChangedEventArgs> ElementChanged;
+			public event EventHandler<PropertyChangedEventArgs> ElementPropertyChanged;
+#pragma warning restore 67
+
+#endregion IVisualElementRenderer
+
+			internal View Content { get; set; }
+
+			public override AView FocusSearch([GeneratedEnum] FocusSearchDirection direction)
+			{
+				var element = Content?.BindingContext as ITabStopElement;
+				if (element == null)
+					return base.FocusSearch(direction);
+
+				int maxAttempts = 0;
+				var tabIndexes = element?.GetTabIndexesOnParentPage(out maxAttempts);
+				if (tabIndexes == null)
+					return base.FocusSearch(direction);
+
+				int tabIndex = element.TabIndex;
+				AView control = null;
+				int attempt = 0;
+				bool forwardDirection = !(
+					(direction & FocusSearchDirection.Backward) != 0 ||
+					(direction & FocusSearchDirection.Left) != 0 ||
+					(direction & FocusSearchDirection.Up) != 0);
+
+				do
+				{
+					element = element.FindNextElement(forwardDirection, tabIndexes, ref tabIndex);
+					var renderer = (element as BindableObject).GetValue(Platform.RendererProperty);
+					control = (renderer as ITabStop)?.TabStop;
+				} while (!(control?.Focusable == true || ++attempt >= maxAttempts));
+
+				return control?.Focusable == true ? control : null;
+			}
+		}
+
 		public override RecyclerView.ViewHolder OnCreateViewHolder(ViewGroup parent, int viewType)
 		{
 			var template = _templateMap[viewType];
 
 			var content = (View)template.CreateContent();
 
-			var linearLayout = new LinearLayout(parent.Context);
-			linearLayout.Orientation = Orientation.Vertical;
-			linearLayout.LayoutParameters = new RecyclerView.LayoutParams(LP.MatchParent, LP.WrapContent);
+			var linearLayout = new LinearLayoutWithFocus(parent.Context)
+			{
+				Orientation = Orientation.Vertical,
+				LayoutParameters = new RecyclerView.LayoutParams(LP.MatchParent, LP.WrapContent),
+				Content = content
+			};
 
 			var bar = new AView(parent.Context);
 			bar.SetBackgroundColor(Color.Black.MultiplyAlpha(0.14).ToAndroid());
@@ -183,9 +253,11 @@ namespace Xamarin.Forms.Platform.Android
 		{
 			readonly Action<Element> _selectedCallback;
 			Element _element;
+			AView _itemView;
 
 			public ElementViewHolder(View view, AView itemView, AView bar, Action<Element> selectedCallback) : base(itemView)
 			{
+				_itemView = itemView;
 				itemView.Click += OnClicked;
 				View = view;
 				Bar = bar;
@@ -203,13 +275,17 @@ namespace Xamarin.Forms.Platform.Android
 						return;
 
 					if (_element != null && _element is BaseShellItem)
+					{
+						_element.ClearValue(Platform.RendererProperty);
 						_element.PropertyChanged -= OnElementPropertyChanged;
+					}
 
 					_element = value;
 					View.BindingContext = value;
 
 					if (_element != null)
 					{
+						_element.SetValue(Platform.RendererProperty, _itemView);
 						_element.PropertyChanged += OnElementPropertyChanged;
 						UpdateVisualState();
 					}

Xamarin.Forms.Platform.Android/VisualElementRenderer.cs

@@ -166,7 +166,7 @@ namespace Xamarin.Forms.Platform.Android
 			if (CheckCustomNextFocus(focused, direction))
 				return base.FocusSearch(focused, direction);
 
-			VisualElement element = Element as VisualElement;
+			var element = Element as ITabStopElement;
 			int maxAttempts = 0;
 			var tabIndexes = element?.GetTabIndexesOnParentPage(out maxAttempts);
 			if (tabIndexes == null)
@@ -183,7 +183,7 @@ namespace Xamarin.Forms.Platform.Android
 			do
 			{
 				element = element.FindNextElement(forwardDirection, tabIndexes, ref tabIndex);
-				var renderer = element.GetRenderer();
+				var renderer = (element as VisualElement)?.GetRenderer();
 				control = (renderer as ITabStop)?.TabStop;
 			} while (!(control?.Focusable == true || ++attempt >= maxAttempts));
 

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

@@ -43,7 +43,7 @@ namespace Xamarin.Forms.Platform.iOS
 				return null;
 
 			var children = Element.Descendants();
-			SortedDictionary<int, List<VisualElement>> tabIndexes = null;
+			SortedDictionary<int, List<ITabStopElement>> tabIndexes = null;
 			List<NSObject> views = new List<NSObject>();
 			foreach (var child in children)
 			{

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

@@ -158,6 +158,38 @@ namespace Xamarin.Forms.Platform.iOS
 			}));
 		}
 
+		public void FocusSearch(bool forwardDirection)
+		{
+			var element = Shell.CurrentItem as ITabStopElement;
+			var tabIndexes = element?.GetTabIndexesOnParentPage(out _, checkContainsElement: false);
+			if (tabIndexes == null)
+				return;
+
+			int tabIndex = element.TabIndex;
+			element = element.FindNextElement(forwardDirection, tabIndexes, ref tabIndex);
+			if (element is ShellItem item)
+				Shell.CurrentItem = item;
+			else if (element is VisualElement ve)
+				ve.Focus();
+		}
+
+		UIKeyCommand[] tabCommands = {
+			UIKeyCommand.Create ((Foundation.NSString)"\t", 0, new ObjCRuntime.Selector ("tabForward:")),
+			UIKeyCommand.Create ((Foundation.NSString)"\t", UIKeyModifierFlags.Shift, new ObjCRuntime.Selector ("tabBackward:"))
+		};
+
+		public override UIKeyCommand[] KeyCommands => tabCommands;
+
+		public UIView NativeView => throw new NotImplementedException();
+
+		public UIViewController ViewController => throw new NotImplementedException();
+
+		[Foundation.Export("tabForward:")]
+		void TabForward(UIKeyCommand cmd) => FocusSearch(forwardDirection: true);
+
+		[Foundation.Export("tabBackward:")]
+		void TabBackward(UIKeyCommand cmd) => FocusSearch(forwardDirection: false);
+
 		void HandlePanGesture(UIPanGestureRecognizer pan)
 		{
 			var translation = pan.TranslationInView(View).X;

Xamarin.Forms.Platform.iOS/VisualElementRenderer.cs

@@ -192,7 +192,7 @@ namespace Xamarin.Forms.Platform.MacOS
 
 			do
 			{
-				element = element.FindNextElement(forwardDirection, tabIndexes, ref tabIndex);
+				element = element.FindNextElement(forwardDirection, tabIndexes, ref tabIndex) as VisualElement;
 #if __MACOS__
 				var renderer = Platform.GetRenderer(element);
 				var control = (renderer as ITabStop)?.TabStop;