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

Fix up Android Accessibility behind a flag #14089

Merged
merged 8 commits into from
Jun 9, 2021
Merged

Conversation

PureWeen
Copy link
Contributor

@PureWeen PureWeen commented Mar 31, 2021

Description of Change

  • added Xamarin.AndroidX.Navigation.UI so I can use the resource id for reading the back button as "Navigate up"
  • added AccessibilityDelegateAutomationId so that if you set an AutomationId and the HelpText it'll read back the HelpText instead of the AutomationID
  • When setting AutomationId on controls that aren't implicitly accessible we need to turn "ImportantForAccessibility" off so that TalkBack doesn't start reading it
  • Disable extracting the defaults for hint and CD
  • Remove the "Touch" settings on the PlatformRenderer
  • Remove Clickable on the PageRenderer
  • Fix Shell so that it doesn't read out the ViewPager if the page has no top tabs

API Changes

Added:

  • Accessibility_Experimental Flag so that you can toggle these changes on and off.

Platforms Affected

  • Android

Testing Procedure

  • Turn this flag on and then click around in Control Gallery on the various TalkBack scenarios for Android. You'll see a flag you can uncomment inside FormsAppCompatActivity. The AutomationId page is a pretty good one to test. Before the flag it reads the AutomationId but then after the flag it continues to read the text

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

if (!String.IsNullOrWhiteSpace(ConcatenateNameAndHelpText(element)))
return false;

if (element is Layout ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This set of controls isn't going to change

This is a bug that's very specific to XF 5 and won't surface in MAUI as we won't be using ContentDescription


namespace Xamarin.Forms.Platform.Android
{
class AccessibilityDelegateAutomationId : AAccessibilityDelegate
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -30,52 +32,102 @@ internal static void GetDrawerAccessibilityResources(global::Android.Content.Con
resourceIdClose = context.Resources.GetIdentifier($"{automationIdParent}{s_defaultDrawerIdCloseSuffix}", "string", context.ApplicationInfo.PackageName);
}

static bool ShoudISetImportantForAccessibilityToNoIfAutomationIdIsSet(AView control, Element element)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we come up with a better, shorter name for this bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the name is clear about what it's doing then no
If it's not clear then yes

It's better to be verbose and clear with methods names then short/clever/and confusing

if(CanNavigateBack)
toolbar.SetNavigationContentDescription(Resource.String.nav_app_bar_navigate_up_description);
else
toolbar.SetNavigationContentDescription(R.String.Ok);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we still set it to ok? maybe we should just not set it to anything? then maybe it'll read like the native default that just reads as "unlabeled button"

Choose a reason for hiding this comment

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

Is there an option for a "null" or "default"? Because if I read the code it's not really OK....

Copy link
Contributor Author

@PureWeen PureWeen Apr 13, 2021

Choose a reason for hiding this comment

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

Because if I read the code it's not really OK....

What do you mean "read the code?"

If you set it to null then it just reads out as "Button Unlabeled"

@PureWeen PureWeen changed the title [WiP] Fix up Android Accessibility behind a flag Fix up Android Accessibility behind a flag Apr 13, 2021
@PureWeen PureWeen added this to the 5.0.0 milestone May 7, 2021
@PureWeen PureWeen added this to Issue In progress in 5.0.0 SR 4 (Planning) via automation May 7, 2021
@PureWeen PureWeen moved this from Issue In progress to PR Needs Review in 5.0.0 SR 4 (Planning) May 7, 2021
@PureWeen
Copy link
Contributor Author

/azp run

@PureWeen PureWeen closed this May 28, 2021
5.0.0 SR 4 (Planning) automation moved this from PR Needs Review to Done May 28, 2021
@PureWeen PureWeen reopened this May 28, 2021
5.0.0 SR 4 (Planning) automation moved this from Done to Issue In progress May 28, 2021
@PureWeen
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rmarinho rmarinho merged commit 5209ec5 into 5.0.0 Jun 9, 2021
@rmarinho rmarinho deleted the accessibilityFlags branch June 9, 2021 08:25
5.0.0 SR 4 (Planning) automation moved this from Issue In progress to Done Jun 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants