Skip to content

*ByRole: return only elements with implicit or explicit accessible={true} prop #1180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mdjastrzebski opened this issue Oct 17, 2022 · 6 comments
Assignees
Labels
a11y compat: react-native enhancement New feature or request help wanted Extra attention is needed

Comments

@mdjastrzebski
Copy link
Member

mdjastrzebski commented Oct 17, 2022

Describe the Feature

This is a proposed tweak to *ByRole query. It can be treated as either a fix or a (breaking) change.

Currently getByRole query is based on explicit role set in accessibilityRole prop. Other props are not taken into account (at least until #1064 is merged). This behaviour while basic can be problematic as according to screen readers, elements with accessible={false} (either explicit or implicit) are not focusable by screen reader.

This manifests in relatively common RN pattern:

<Pressable accessibilityRole="button" accessible={false}>
  <Text>Cell title</Text>
  <Pressable accessibilityRole="button">
    <Text>Action</Text>
  </Pressable>
</Pressable>

Under current implementation getByRole('button', { name: "Action" }) would select both outer and inner pressables. After implementing the proposed change it would select only the inner one.

Possible Implementations

  • element can be matched by getByRole using the same criteria as now (host component, equal accessibilityRole) but also needs to be considered accessibility element (i.e. be focusable).
  • each host view can be made focusable by explicitly passing accessible={true}
  • some host view types are considered focusable without the accessible prop (e.g. Text, TextInput), but can have their "focusability" disabled by setting accessible={false}
  • basic host View is not consider focusable
  • some RN built in components, e.g. Pressable, TouchableOpacity which render to host View actually set explicit accessible={true} prop on the view

In more practical terms we could use something like isAccessibilityElement to check for focusable state:

export function isAccessibilityElement(
  element: ReactTestInstance | null
): boolean {
  if (element == null) {
    return false;
  }

  if (element.props.accessible !== undefined) {
    return element.props.accessible;
  }

  return (
    isHostElementForType(element, Text) ||
    isHostElementForType(element, TextInput)
  );
}

Then this criteria should be added to *ByRole query.

This change should not affect other general purpose queries as *ByText or *ByTestId. It may be worth considering applying it to some other a11y-related queries, which would make only sense if selected view is accessible: *ByA11yValue, *ByA11State.

Related Issues

#1152 - initial source for issue
#1163 - related discussion
#1064 - hidden query option

CC: @AugustinLF @pierrezimmermannbam @MattAgn @thymikee

@MattAgn
Copy link
Collaborator

MattAgn commented Nov 20, 2022

I'm willing to work on that :)

@mdjastrzebski
Copy link
Member Author

@MattAgn great! Go ahead.

I've got some initial research done on this subject:

  1. View seems to default to accessible={false} when observed under Xcode Accessibility Inspector and/or iOS/Android screen reader. But some other controls like Text and TextInput behave the same without accessible prop as when set accessibile={true}. In order to implement this properly we should analyse all relevant host elements types, so beside View, Text, TextInput this might be Image, and maybe something more. Some other views, e.g. Pressable and TouchableOpacity render to host View with explicit accessible={true}.
  2. It would be good to have isAccessibilityElement(element) function that would return true when element either has explicit accessible={true} or is implicit accessibility element (i.e. behaves the same without accessible prop and with accessible={true} under Xcode Accessibility Inspector/Screen readers).
  3. This looks like a breaking change, so it would be included in the next major release along with other breaking changes. Alternatively, we could provide some option for that, but it would be rather short lived (till the major release).
  4. This check should initially be added to *ByRole queries. Adding it to any other a11y query should be preceeded by careful analysis of actual behaviour under Xcode Accessibility Inspector/Screen readers.

@MattAgn
Copy link
Collaborator

MattAgn commented Nov 22, 2022

Alright sounds good to me!
For the the function isAccessibilityElement, i would rather go for isElementAccessible in terms of naming. The term accessible element makes more to sense to me than accessibility element wdyt?

@mdjastrzebski
Copy link
Member Author

There seems to be a difference between "accessible element" vs "accessibility element". We already have isInaccessible which when negated would be isAccessible but actually conveys idea of "hidden from accessibility" rather than being accessibility element ("focusable by screen reader").

RN docs uses the "accessibility element" naming so I would stick with it.

@MattAgn
Copy link
Collaborator

MattAgn commented Nov 23, 2022

Oh indeed, did not see it was the term used in the RN documentation, fine by me then 👍

@MattAgn
Copy link
Collaborator

MattAgn commented Jan 18, 2023

Closed by #1244

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y compat: react-native enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants