From ccfd6170ccbcbadc98140f3b32ddc4f1d703dcb2 Mon Sep 17 00:00:00 2001 From: Pavel Yakovlev Date: Wed, 19 Jun 2019 16:23:00 -0700 Subject: [PATCH] [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 * Update Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3454.cs Co-Authored-By: paymicro --- .../Issue3454.cs | 83 ++++++++++++++++++++++ .../Xamarin.Forms.Controls.Issues.Shared.projitems | 1 + .../AppCompat/PickerRenderer.cs | 9 +++ .../EntryAccessibilityDelegate.cs | 36 ++++++++++ .../FastRenderers/AutomationPropertiesProvider.cs | 10 ++- .../Renderers/PickerRenderer.cs | 8 +++ .../Xamarin.Forms.Platform.Android.csproj | 1 + 7 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3454.cs create mode 100644 Xamarin.Forms.Platform.Android/EntryAccessibilityDelegate.cs 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 new file mode 100644 index 0000000..c47e7c0 --- /dev/null +++ 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(); + 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 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/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index 910ed11..da56010 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -68,6 +68,7 @@ + diff --git a/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs b/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs index 3d3de46..0ae6200 100644 --- a/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs +++ b/Xamarin.Forms.Platform.Android/AppCompat/PickerRenderer.cs @@ -16,6 +16,7 @@ namespace Xamarin.Forms.Platform.Android.AppCompat { AlertDialog _dialog; bool _disposed; + EntryAccessibilityDelegate _pickerAccessibilityDelegate; public PickerRendererBase(Context context) : base(context) { @@ -38,6 +39,9 @@ namespace Xamarin.Forms.Platform.Android.AppCompat _disposed = true; ((INotifyCollectionChanged)Element.Items).CollectionChanged -= RowsCollectionChanged; + + _pickerAccessibilityDelegate?.Dispose(); + _pickerAccessibilityDelegate = null; } base.Dispose(disposing); @@ -54,7 +58,10 @@ namespace Xamarin.Forms.Platform.Android.AppCompat if (Control == null) { var textField = CreateNativeControl(); + SetNativeControl(textField); + + ControlUsedForAutomation.SetAccessibilityDelegate(_pickerAccessibilityDelegate = new EntryAccessibilityDelegate(Element)); } UpdateFont(); UpdatePicker(); @@ -153,6 +160,8 @@ namespace Xamarin.Forms.Platform.Android.AppCompat 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 new file mode 100644 index 0000000..56c34c0 --- /dev/null +++ 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)}"; + } + } + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs b/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs index 59b7455..ecc9ab4 100644 --- a/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs +++ b/Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs @@ -118,14 +118,20 @@ namespace Xamarin.Forms.Platform.Android.FastRenderers 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 index 92bc534..26d25d6 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/PickerRenderer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/PickerRenderer.cs @@ -20,6 +20,7 @@ namespace Xamarin.Forms.Platform.Android bool _isDisposed; TextColorSwitcher _textColorSwitcher; int _originalHintTextColor; + EntryAccessibilityDelegate _pickerAccessibilityDelegate; public PickerRenderer(Context context) : base(context) { @@ -41,6 +42,9 @@ namespace Xamarin.Forms.Platform.Android { _isDisposed = true; ((INotifyCollectionChanged)Element.Items).CollectionChanged -= RowsCollectionChanged; + + _pickerAccessibilityDelegate?.Dispose(); + _pickerAccessibilityDelegate = null; } base.Dispose(disposing); @@ -63,6 +67,8 @@ namespace Xamarin.Forms.Platform.Android { var textField = CreateNativeControl(); + textField.SetAccessibilityDelegate(_pickerAccessibilityDelegate = new EntryAccessibilityDelegate(Element)); + var useLegacyColorManagement = e.NewElement.UseLegacyColorManagement(); _textColorSwitcher = new TextColorSwitcher(textField.TextColors, useLegacyColorManagement); @@ -203,6 +209,8 @@ namespace Xamarin.Forms.Platform.Android 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 index 76e5fd0..746e811 100644 --- a/Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj +++ b/Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj @@ -152,6 +152,7 @@ + -- 2.7.4