-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix up Android Accessibility behind a flag #14089
Conversation
if (!String.IsNullOrWhiteSpace(ConcatenateNameAndHelpText(element))) | ||
return false; | ||
|
||
if (element is Layout || |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very similar to what @ryl created here
@@ -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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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"
There was a problem hiding this comment.
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....
There was a problem hiding this comment.
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"
/azp run |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
719adf5
to
42f0595
Compare
Description of Change
API Changes
Added:
Platforms Affected
Testing Procedure
PR Checklist