[Android] fix accessibility of Picker (#5125)

* [Android] fix accessibility of Picker * fix NRE * fixes read selected item with accessibility * refactoring test * added dispose of accessibility delegate * fix build uitest * Update Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3454.cs Co-Authored-By: paymicro <v-payako@microsoft.com> * Update Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3454.cs Co-Authored-By: paymicro <v-payako@microsoft.com>
xamarin · Jun 19, 2019 · ccfd617 · ccfd617
1 parent 3df78bc
commit ccfd617
Show file tree

Hide file tree

Showing 7 changed files with 146 additions and 2 deletions.
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3454.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3454.cs
@@ -0,0 +1,83 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 3454, "Picker accessibility text is wrong", PlatformAffected.Android)]
+	public class Issue3454 : TestContentPage
+	{
+		protected override void Init()
+		{
+			var pickers = new List<Picker>();
+			var grid = new Grid();
+			int row = 0, col = 0;
+			grid.AddChild(new Label { Text = "Default Style" }, col, row++);
+#if APP
+			void AddPicker(string title, Func<Picker> getPicker)
+			{
+				grid.AddChild(new Label { Text = title }, col, row++);
+				var picker = getPicker();
+				picker.ItemsSource = Enumerable.Range(1, 10).Select(i => $"item {i}").ToList();
+				pickers.Add(picker);
+				grid.AddChild(picker, col, row++);
+			}
+
+			AddPicker("AutomationProperties", () => 
+			{
+				var picker = new Picker();
+				picker.SetAutomationPropertiesName("First accessibility");
+				picker.SetAutomationPropertiesHelpText("This is the accessibility text");
+				return picker;
+			});
+			AddPicker("Default", () => new Picker ());
+			AddPicker("Default + Title", () => new Picker { Title = "Title1" });
+			AddPicker("AutomationProperties + Title", () =>
+			{
+				var picker = new Picker { Title = "Title2" };
+				picker.SetAutomationPropertiesName("Last accessibility");
+				picker.SetAutomationPropertiesHelpText("This is the accessibility text");
+				return picker;
+			});
+
+			row = 0; col++;
+			grid.AddChild(new Label { Text = "Material Style" }, col, row++);
+			AddPicker("AutomationProperties", () =>
+			{
+				var picker = new Picker { Visual = VisualMarker.Material };
+				picker.SetAutomationPropertiesName("First accessibility");
+				picker.SetAutomationPropertiesHelpText("This is the accessibility text");
+				return picker;
+			});
+			AddPicker("Default", () => new Picker { Visual = VisualMarker.Material });
+			AddPicker("Default + Title", () => new Picker { Title = "Title1", Visual = VisualMarker.Material });
+			AddPicker("AutomationProperties + Title", () =>
+			{
+				var picker = new Picker { Title = "Title2", Visual = VisualMarker.Material };
+				picker.SetAutomationPropertiesName("Last accessibility");
+				picker.SetAutomationPropertiesHelpText("This is the accessibility text");
+				return picker;
+			});
+#endif
+
+			Content = new ScrollView
+			{
+				Content = new StackLayout
+				{
+					Children =
+					{
+						grid,
+						new Button
+						{
+							Text = "Clear pickers",
+							Command = new Command(() => pickers.ForEach(p => p.SelectedItem = null))
+						}
+					}
+				}
+			};
+		}
+	}
+}
diff --git a/...ssues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/...ssues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
@@ -68,6 +68,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue3809.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue2894.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3306.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue3454.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3308.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3788.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1724.cs" />

diff --git a/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs
@@ -16,6 +16,7 @@ public abstract class PickerRendererBase<TControl> : ViewRenderer<Picker, TContr
 	{
 		AlertDialog _dialog;
 		bool _disposed;
+		EntryAccessibilityDelegate _pickerAccessibilityDelegate;
 
 		public PickerRendererBase(Context context) : base(context)
 		{
@@ -38,6 +39,9 @@ protected override void Dispose(bool disposing)
 				_disposed = true;
 
 				((INotifyCollectionChanged)Element.Items).CollectionChanged -= RowsCollectionChanged;
+
+				_pickerAccessibilityDelegate?.Dispose();
+				_pickerAccessibilityDelegate = null;
 			}
 
 			base.Dispose(disposing);
@@ -54,7 +58,10 @@ protected override void OnElementChanged(ElementChangedEventArgs<Picker> e)
 				if (Control == null)
 				{
 					var textField = CreateNativeControl();
+
 					SetNativeControl(textField);
+
+					ControlUsedForAutomation.SetAccessibilityDelegate(_pickerAccessibilityDelegate = new EntryAccessibilityDelegate(Element));
 				}
 				UpdateFont();
 				UpdatePicker();
@@ -153,6 +160,8 @@ void UpdatePicker()
 				EditText.Text = null;
 			else
 				EditText.Text = Element.Items[Element.SelectedIndex];
+
+			_pickerAccessibilityDelegate.ValueText = EditText.Text;
 		}
 
 		abstract protected void UpdateTextColor();

diff --git a/Xamarin.Forms.Platform.Android/EntryAccessibilityDelegate.cs b/Xamarin.Forms.Platform.Android/EntryAccessibilityDelegate.cs
@@ -0,0 +1,36 @@
+using Android.Views.Accessibility;
+using Xamarin.Forms.Platform.Android.FastRenderers;
+
+namespace Xamarin.Forms.Platform.Android
+{
+	class EntryAccessibilityDelegate : global::Android.Views.View.AccessibilityDelegate
+	{
+		BindableObject _element;
+
+		public EntryAccessibilityDelegate(BindableObject Element) : base()
+		{
+			_element = Element;
+		}
+
+		protected override void Dispose(bool disposing)
+		{
+			_element = null;
+			base.Dispose(disposing);
+		}
+
+		public string ValueText { get; set; }
+
+		public string ClassName { get; set; } = "android.widget.Button";
+
+		public override void OnInitializeAccessibilityNodeInfo(global::Android.Views.View host, AccessibilityNodeInfo info)
+		{
+			base.OnInitializeAccessibilityNodeInfo(host, info);
+			info.ClassName = ClassName;
+			if (_element != null)
+			{
+				var value = string.IsNullOrWhiteSpace(ValueText) ? string.Empty : $"{ValueText}. ";
+				host.ContentDescription = $"{value}{AutomationPropertiesProvider.ConcatenateNameAndHelpText(_element)}";
+			}
+		}
+	}
+}
diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs b/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs
@@ -118,14 +118,20 @@ static bool SetHint(AView Control, BindableObject Element, ref string defaultHin
 				return false;
 			}
 
+			if (Element is Picker)
+			{
+				return false;
+			}
+
 			var textView = Control as TextView;
 			if (textView == null)
 			{
 				return false;
 			}
 
-			// Let the specified Title/Placeholder take precedence, but don't set the ContentDescription (won't work anyway)
-			if (((Element as Picker)?.Title ?? (Element as Entry)?.Placeholder) != null)
+			// TODO: add EntryAccessibilityDelegate to Entry
+			// Let the specified Placeholder take precedence, but don't set the ContentDescription (won't work anyway)
+			if ((Element as Entry)?.Placeholder != null)
 			{
 				return true;
 			}

diff --git a/Xamarin.Forms.Platform.Android/Renderers/PickerRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/PickerRenderer.cs
@@ -20,6 +20,7 @@ public class PickerRenderer : ViewRenderer<Picker, EditText>, IPickerRenderer
 		bool _isDisposed;
 		TextColorSwitcher _textColorSwitcher;
 		int _originalHintTextColor;
+		EntryAccessibilityDelegate _pickerAccessibilityDelegate;
 
 		public PickerRenderer(Context context) : base(context)
 		{
@@ -41,6 +42,9 @@ protected override void Dispose(bool disposing)
 			{
 				_isDisposed = true;
 				((INotifyCollectionChanged)Element.Items).CollectionChanged -= RowsCollectionChanged;
+
+				_pickerAccessibilityDelegate?.Dispose();
+				_pickerAccessibilityDelegate = null;
 			}
 
 			base.Dispose(disposing);
@@ -63,6 +67,8 @@ protected override void OnElementChanged(ElementChangedEventArgs<Picker> e)
 				{
 					var textField = CreateNativeControl();
 
+					textField.SetAccessibilityDelegate(_pickerAccessibilityDelegate = new EntryAccessibilityDelegate(Element));
+
 					var useLegacyColorManagement = e.NewElement.UseLegacyColorManagement();
 					_textColorSwitcher = new TextColorSwitcher(textField.TextColors, useLegacyColorManagement);
 
@@ -203,6 +209,8 @@ void UpdatePicker()
 
 			if (oldText != Control.Text)
 				((IVisualElementController)Element).NativeSizeChanged();
+
+			_pickerAccessibilityDelegate.ValueText = Control.Text;
 		}
 
 		void UpdateTextColor()

diff --git a/Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj b/Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj
@@ -152,6 +152,7 @@
     <Compile Include="ITabStop.cs" />
     <Compile Include="IPickerRenderer.cs" />
     <Compile Include="PickerManager.cs" />
+    <Compile Include="EntryAccessibilityDelegate.cs" />
     <Compile Include="Renderers\CircularProgress.cs" />
     <Compile Include="Renderers\PickerEditText.cs" />
     <Compile Include="Renderers\FontImageSourceHandler.cs" />