Prevent duplicate accessibilityLabel on parent views

[newyankeecodeshop]

Summary

In testing React Native apps using Appium, the accessibilityLabel is an important prop which provides an additional means to lookup elements in addition to the testID. More importantly, the accessibilityLabel provides detail to screen readers and other accessibility helpers. React Native on iOS has custom logic (compared to Android), whereby it concatenates the accessibility labels of children. This is useful in a case such as this:

<TouchableHighlight accessibilityRole="button" onPress={...}>
<Text>This button</Text>
<Text>Does something useful</Text>
</TouchableHighlight>

The above creates the following accessibility view:

<UIView accessibilityLabel="This button Does something useful" />

and when viewed in Appium Studio

<XCUIElementTypeButton name="This button Does something useful" />

However, if the above is in one or more views, such as:

<View>
<View>
<TouchableHighlight accessibilityRole="button" onPress={...}>
<Text>This button</Text>
<Text>Does something useful</Text>
</TouchableHighlight>
</View>
<FlatList ... />
</View>

You can get the following from XCUITest and Appium:

<XCUIElementTypeOther name="This button Does something useful">
<XCUIElementTypeOther name="This button Does something useful">
<XCUIElementTypeButton name="This button Does something useful" />
</XCUIElementTypeOther>
</XCUIElementTypeOther>

This happens because the parent Views are using RCTRecursiveAccessibilityLabel() to populate the label from the child Touchable. This doesn't make sense semantically because the TouchableHighlight is an accessible element. It benefits from the concatenation to create its label from the two Text elements, but the container views are not part of that label. They should not include it.

This PR should fix#21830, and it represents a possibly lower impact alternative to #24113, #24118, #29801, and #31222. It's not clear what functionality is relying on the string concatenation as it is currently implemented.

Changelog

Change the recursion algorithm to include labels from accessible elements only if they are not other types of RCTView instances. For example, RCTTextView should be included. This helps to minimize the impact of this change on other developers of custom views.

[iOS] [Changed] - When iterating over subviews, ignore subviews that are accessible elements (e.g. Touchable).

Test Plan

Verified that accessibility labels on <Touchable> elements maintain existing behavior with nested Text elements.
Verified that accessibility labels on non-accessible views do not include all child labels.

I added an example to AccessibilityIOSExample.js which can be used with VoiceOver to see the new behavior. In the example, text nested inside accessible subviews is not part of the spoken label.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: cbec66e

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	9,181,613	-59,787
android	hermes	armeabi-v7a	8,708,021	-45,284
android	hermes	x86	9,623,369	-60,564
android	hermes	x86_64	9,590,030	-62,751
android	jsc	arm64-v8a	10,818,191	-59,263
android	jsc	armeabi-v7a	9,735,875	-44,751
android	jsc	x86	10,855,347	-60,028
android	jsc	x86_64	11,463,530	-62,226

Base commit: cbec66e

[newyankeecodeshop]

Hi @nfriedly, @mrmacete, it looks like we're all interested in improving the recursive accessibility label algorithm. Do you think my change would meet your needs too? Whichever way we go, I'll try to advocate to get something merged too.

[nfriedly]

👍

[nfriedly]

At first glance, this looks fine to me.

I would recommend getting out a physical device, turning on the screen reader, and testing the behavior before and after this change. (The screen reader doesn't work in the iOS simulator.) I think this PR is going to behave better than mine in that regard, but I don't know this stuff well enough to say for sure.

In my case, the concerns were mainly performance related, and we mitigated it by special-casing React Native apps and bypassing RCTRecursiveAccessibilityLabel entirely. (I just now closed my PR.)

(Also, FWIW, #31222 is kind of orthogonal to this, as it only improves performance without changing behavior.)

[p-sun]

Hi! Could you create a test in AccessibilityExample.js or AccessibilityIOSExample.js for this?

[newyankeecodeshop]

@p-sun is my test adequate?

[newyankeecodeshop]

Hi @p-sun is there anything I can do to help move this forward?

[p-sun]

I will take a look at this within the next three days. Thanks for waiting!

[newyankeecodeshop]

Hi @p-sun will you have a chance to review this?

[ls-erin-walker]

Is there any chance this has been reviewed?

[newyankeecodeshop]

Hi @p-sun will this get a review or should I close it?