Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

[Android] TalkBack now reads name and helptext on buttons. #5237

Closed
wants to merge 83 commits into from

Conversation

ryl
Copy link
Contributor

@ryl ryl commented Feb 13, 2019

Description of Change

Modified AutomationPropertiesProvider such that AutomationProperties.Name and AutomationProperties.HelpText will be read by TalkBack.

Issues Resolved

API Changes

None

Platforms Affected

  • Android

Behavioral/Visual Changes

TalkBack will read AutomationProperties.Name and AutomationProperties.HelpText on a button if provided.

Before/After Screenshots

Not applicable

Testing Procedure

In the Control Gallery, visit G5150. Labels containing the ContentDescription for each button should appear with appropriate text.

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

@dnfclas
Copy link

dnfclas commented Feb 13, 2019

CLA assistant check
All CLA requirements met.

@ryl
Copy link
Contributor Author

ryl commented Feb 13, 2019

I followed the instructions here https://blog.xamarin.com/beginners-guide-contributing-xamarin-forms/ more or less.

Are there any instructions I can follow to run a specific UI test, or all of them, using VS Mac? I have not actually tried my UI Test case.

@samhouts samhouts changed the title Fix for 5150. TalkBack now reads name and helptext on buttons. [Android] TalkBack now reads name and helptext on buttons. Feb 14, 2019
@ryl ryl force-pushed the automation-name-helptext branch from 6cfbf26 to 5fcc2c4 Compare February 17, 2019 03:40
@samhouts samhouts requested a review from paymicro March 11, 2019 19:09
Copy link
Contributor

@paymicro paymicro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
A few comments on the test

Copy link
Contributor

@paymicro paymicro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -55,6 +55,10 @@ internal static void SetAutomationId(AView control, VisualElement element, strin
defaultContentDescription = control.ContentDescription;

string value = ConcatenateNameAndHelpText(element);

if (control is IButtonLayoutRenderer button)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're going to have to revisit this later, since this is only fixing the issue for Buttons, and it now likely affecting other controls, too. This can work for now, though. Thanks!

Copy link
Contributor

@samhouts samhouts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue5150 test doesn't work on iOS :( Need to either make it work or make it only run on Android. :)

@samhouts
Copy link
Contributor

Also, UI test is now unable to find the Buttons on Android at all. Not good!

@ryl
Copy link
Contributor Author

ryl commented Mar 20, 2019

How would I go about making the UI Test only run only on Android? This is my first attempt at contributing so I'm pretty green on procedures. I took a look at another issuse (3415) that seems to be Android only and didn't see anything remarkably different between that and my test. If you can point me in the right direction I'll try to get it polished up.

@ryl
Copy link
Contributor Author

ryl commented Mar 21, 2019

I did some more digging around and found some tests that were using [Ignore] for certain platforms. I added it to 5150's UI Test. It should only run on the Android platform now. Since the bug/fix only applies to Android, that seems like the way to go to me.

@paymicro paymicro requested a review from samhouts March 25, 2019 09:34
@ryl
Copy link
Contributor Author

ryl commented Mar 29, 2019

Also, UI test is now unable to find the Buttons on Android at all. Not good!

I see what you're saying about not being able to find buttons. Looking through the core tests it looks like this fix breaks AutomationId, which seems to come back to #1529.

Seems that AutomationId and Name + HelpText are competing for use of ContentDescription.

PureWeen and others added 12 commits February 14, 2020 15:50
…amarin#5349

* RadioButton

* Removed unused files

* Rebase and make it run

* First round of feedback

* Revert AppCompatButton -> AButton

* Cleaned minor usings

* Fix unselecting radiobutton on iOS

* Fixed Mac OS grouping

* [Android] Fix API29 usages

Co-authored-by: Andrei Nitescu <[email protected]>
Co-authored-by: Rui Marinho <[email protected]>
…amarin#9187)

* Fix BottomNavigationItemView with MasterDetailPage.

* Added repro sample

Co-authored-by: Javier Suárez Ruiz <[email protected]>
fixes xamarin#9143
hartez and others added 4 commits February 21, 2020 16:54
* Add a platform test to check for Bugzilla 35738

* Delete the Bugzilla35738 UI test

* Bold the start of fixtures

Fix typo

* Add some background color tests for iOS

* Add test for iOS Button BackgroundColor

* Remove changes to PerformanceTrackerRenderer

* Split classes into files

* Add UWP testing platform

* Cache default font size

* Run UWP tests on main thread

* Use GetOrCreateRenderer

* Split Test and Description attributes; clean up iOS background color tests

* Opacity tests on iOS

* Some background color tests for UWP

* BackgroundColorTests for UWP for most controls

* Shortcut key to get to platform tests on UWP

* Button background color test for Android

* BackgroundColor tests for Android controls

* Opacity tests for Android

* Ignore Opacity tests for iOS/Android (replaced by platform tests)

* Remove unused empty BackgroundColor test

* IsEnabled tests for Android

* Consolidate basic VisualElement list

* Apply categories for generate test cases

* Consolidate element list

* IsEnabled tests for iOS

* Ignore tests covered by platform tests

* Fix incorrect overrides

* Rotation tests for Android

* Rotation tests for iOS

* Set UI test versions to Ignore

* Rotation tests for UWP

* Scale tests for iOS

* ScaleX and ScaleY tests for Android

* Android TranslationX and TranslationY tests

* Ignore TranslationX/Y for manual review on Android

* Ignore tests covered by platform tests

* Add Sliders to the Android/iOS tests

* Add Slider to UWP tests

* Ignore tests for Slider that're already covered

* Add ImageButton to the platform tests

* Ignore duplicate tests for ImageButton

* Add IsVisible tests for Android/iOS

* Ignore IsVisible tests covered by platform tests

* Fix usings for AndroidX

* BoxView tests for Android

* Add test counter to platform test console

* BoxView tests for iOS

* Ignore duplicate tests for BoxView

* Frame tests for Android

* Add Frame tests on iOS

* Ignore duplicate tests

* Add Button and Label text tests for Android/iOS

* Ignore duplicate text test for Button

* Add more controls to UWP tests; add IsEnabled tests for UWP

* Add scale tests for UWP

* Ignore UWP tests covered by platform tests

* Mark Issue968 test as manual review

* Replace 8269 UI test with platform test

* Move 8682 from UI test to platform test

* Mark Issue8004 test for manual review

* Adding manual review attribute for Bugzilla29128

* Show bitmaps for failed color tests on Android; create BoxView CornerRadius test

* Functional color-at-point tests

* Clean up Android pixel checking code; add more CornerRadius tests

* Fix NRE in FrameRenderer

* Ignore Frame

* Fix cross-thread dispose issues with StepperRenderer and SliderRenderer

* Add tolerance for color comparisons

* Parent Frame for older API versions

* Remove assertion not supported before iOS 12

* Address fast label renderer, changes in BoxView, and a bug in Android fast button
# Conflicts:
#	Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
#	Xamarin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs
#	Xamarin.Forms.Platform.Android/Xamarin.Forms.Platform.Android.csproj
@ryl
Copy link
Contributor Author

ryl commented Feb 26, 2020

@samhouts I accidentally deleted my pull request branch when I meant to do a force push to resolve some conflicts. Any chance I can get this re-opened, or should I just create a new one?

@samhouts
Copy link
Contributor

Oops! Looks like you're going to need to create a new one. I don't have the ability to reopen it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.