Effect of accessible prop on *ByRole + hidden, inInaccessible, etc

[mdjastrzebski]

Describe the issue

According to accessible prop documentation, setting it to true "groups its children into a single selectable component". Then there is following example:

<View accessible={true}>
  <Text>text one</Text>
  <Text>text two</Text>
</View>

with following comment: "In the above example, we can't get accessibility focus separately on 'text one' and 'text two'. Instead we get focus on a parent view with 'accessible' property."

I understand the above statement that there is no concept nested accessible elements. If you are able to "focus" on some element, then it is treated as a whole by accessibility APIs, you cannot "focus" on any of its descendants. In other words there "focusable" components form a linear structure (similar to tabIndex attribute on Web) and not a tree structure when you could nest "focusable" element inside other "focusable" element.

We should consider including that behaviour in *ByRole queries, and how does it affect other A11y related functions like upcoming hidden query option #1064, isInaccessible function, etc.

Some examples:

1. getByRole('button', { hidden: false }) should return only outer Pressable, because Pressable renders host View with accessible={true} by default. Inner Pressable is not "focusable" because the outer Pressable forms a single focusable unit for assitive technology.

<Pressable accessibilityRole="button" testID="outer">
  <Pressable accessibilityRole="button" testID="inner">
    <Text>Press me</Text>
  </Pressable>
</Pressable>

2. getByRole('button', { hidden: false }) should return only inner Pressable, because overriding accessible prop of outer pressable excludes it from reaching by assistive technology:

<Pressable accessibilityRole="button" testID="outer" accessible={false}>
  <Pressable accessibilityRole="button" testID="inner">
    <Text>Press me</Text>
  </Pressable>
</Pressable>

In case of using hidden: true option, aka respectAccessibility: false, the above test cases would return both outer and inner Pressables.

Possible Implementations

We might modify isInaccessible helper that will form the basis of exclusion for hidden accessibility elements in #1064. That way descendants of element with accessible={true} could be marked as inaccessible, while the element itself will be accessible.

This behaviour is related to upcoming hidden option, with the default behaviour being initially hidden: true aka respectAccessibility: false, so that should not make a breaking change. In the longer term we plan change it to false, as in RTL/DTL and only then this would be a breaking change.

Related Issues

#1161
#1064
#1128

@AugustinLF @thymikee @pierrezimmermannbam @MattAgn I'd would like to gather your feedback on proposed change, whether you see any potential issues,

[pierrezimmermannbam]

If I understand correctly, if this gets implemented with hidden : true, for the following component :

 <Pressable>
    <Text>Press me</Text>
  </Pressable>

This wouldn't work anymore since the Text element is inaccessible

fireEvent.press(screen.getByText('Press me')

This means that we would systematically have to use byRole queries for pressing buttons by text (which if I'm correct requires to add the accessibilityRole prop for now). Even though it probably is a better way to do it and it seems interesting to enforce it, this would be a major breaking change and could be quite hard to fix on large codebases. However hidden : false could still be used in this kind of situation.

What I'm more concerned about is that the behavior of queries becomes more and more complicated and unnatural to users, especially the ones that are not very familiar with how accessibility works. So while I think this goes in the right direction, this would need to be really well documented before being adopted. There should be a way to make this work, for instance with very good error messages and really good documentation but in any case making users understand what's going on should be an important focus for this feature imo

[AugustinLF]

I agree with @pierrezimmermannbam. I do think that the example provided should be valid, regardless of the options provided by the developer:

const {getByText} = render(
 <Pressable>
    <Text>Press me</Text>
  </Pressable>
)

// this should work, regardless of `hidden/respectAccessibility` value. 
screen.getByText('Press me')

We might need more granularity when it comes to detecting hidden, or at least for some queries.

And we do want to raise an error for this case (because it is likely inaccessible), I don't think it should be done as part of any query, but a render check, akin to what we have for Text detection. That'd avoid a confusing experience for the user (rendering was fine, now I get an error?), and will likely allow us to have better error messages. But we could also discuss if this is something that should belong in RNTL, or not. We could even consider offering some integration with other libraries doing runtime checks, such as Formidable's react native aama.

[MattAgn]

I also agree with @pierrezimmermannbam and @AugustinLF, it would be a major change for most projects and also not very natural at first. I feel like pressing buttons is one of the most common actions we do in tests.
I think my main concern is that if we need to rely on testId or role props to click on buttons, we'll be going away from the philosophy of testing library which is to test as much as possible like a real user. I agree that respecting accessibility is important but I feel like sticking to the original philosophy and having a clear and natural api is even more so

However if we were to add accessible check in isInaccessible, maybe we could add a helper to click button by texts directly so that users don't have to always add the hidden: true all the time?

render(
 <Pressable>
    <Text>Press me</Text>
  </Pressable>
)

pressButtonByText("Press me")

[mdjastrzebski]

Thanks for feedback. I share your concerns about affecting other queries, existing user tests, etc.

accessible prop effect on screen reader
In order to learn some more about how the things really behave in RN, I've done some tests using iOS accessibility inspector and Android TalkBack and found that:

• on Android you can nest accessibility elements inside others, and screen ready by default focuses the outer component first, but you can have it focus also the inner elements too.
• on iOS you indeed cannot nest a11y elements, as only the outer will be focusable. This however seems to be a RN-specific limitations, as native iOS can have nested a11y elements, but RN does not (yet?) support it.

Based on these observations I think we should allow for searching inside elements with accessibility={true} despite the confusing docs. Let's not involve accessible prop logic in isInaccessible checks.

fireEvent.press(getByText("Press me")) should still work as is :-)

DTL hidden option
DTL only supports hidden option on *ByRole queries, they do not have it for any other query(!). The reasoning behind this seems to be that only *ByRole queries are a11y-aware, and other queries are lightweight. See here: testing-library/dom-testing-library#929 (comment)

I think that in our case we would want to see hidden option on other queries, as one of the reasons we started to implement this option was to be able to search on only the active screen, assuming that all screens in the back stack, e.g. from React Navigation, have proper a11y props set.

Potential usage for accessible prop
What we might consider is matching only these elements with *ByRole query that are having explicit accessible={true} or behave as if they have it by default: e.g. host Text and TextInput behave in such way, host View does not. If we would pursue this way, it would be in order to match the observe RN behaviour. I'm not sure we do it at this stage though, as it warrants some more research in that context.

If we went that way, then nested Pressables with outer being accessible={false} would not be matched by getByRole("button", { name: "Pressable" }). @AugustinLF iirc you had mentioned this case in some of the recent discussions regarding role queries.

[mdjastrzebski]

Thank you all for discussion. Closing in favour of #1180 with more clear scope.