Add accessibilityValueDescription support.

[marcmulcahy]

Summary

React Native components need a mechanism to specify their value to assistive technologies. This PR adds the notion of accessibilityValueDescription-- a property which either contains a textual description of a component's value, or for range-based components, such as sliders and progress bars, it contains range information (minimum, current, and maximum).

On iOS, the range-based info if present is converted into a percentage and added to the accessibilityValue property of the UIView. If text is present as part of the accessibilityValueDescription, it is used instead of the range-based information.

On Android, any range-based information in accessibilityValueDescription is exposed in the AccessibilityNodeInfo's RangeInfo. Text which is part of accessibilityValueDescription is appended to the content description.

Changelog

[GENERAL] [Change] - add accessibilityValuedescription property.

Test Plan

Added two new accessibility examples to RNTester, one which uses text and another which uses range-based info in accessibilityValueDescription. Verified that they both behave correctly on both Android and iOS.

[pull-bot]

Messages

📖

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message. CATEGORY may be: General iOS Android JavaScript Internal (for changes that do not need to be called out in the release notes) TYPE may be: Added, for new features. Changed, for changes in existing functionality. Deprecated, for soon-to-be removed features. Removed, for now removed features. Fixed, for any bug fixes. Security, in case of vulnerabilities. MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against 7a66ede

[necolas]

Internally, the equivalent we have (for web) follows the ARIA properties a bit more closely:

type AccessibilityValue = $ReadOnly<{|
  max?: number,
  min?: number,
  now?: number,
  text?: string,
|}>;

type Props = {
  accessibilityValue: AccessibilityValue
}

[marcmulcahy]

I would have done something like that, but I didn't know how to workaround the fact that iOS also has a property named accessibilityValue, which we're using to cover lots of metadata that doesn't fall logically somewhere else (such as some roles nd states). Can you scribble me a quick code example of how to have a property named accessibiltyValue which is of type AccessibliityValue and also have the iOS implementation properly return a string to iOS for accessibilityValue?

[necolas]

I didn't know how to workaround the fact that iOS also has a property named accessibilityValue, which we're using to cover lots of metadata that doesn't fall logically somewhere else

Is "we" Amazon? Could you expand on what you're using iOS accessibilityValue for and what its role is on the iOS platform?

[marcmulcahy]

accessibilityValue is used to present the value of a component in iOS-- something that's obviously not its label, but should be presented to the user. It's used, for example, to present the current value of a slider. on iOS, accessibilityValue is a string.

As part of the addition of numerous roles and states in PR #24095, we use accessibilityValue to present a few things:

• It's required to make VoiceOver present the state of switches correctly (accessibilityValue must be set to "0" for off or "1" for on)
• Any roles which do not have an associated iOS trait (including checkbox)
• Any states which do not have an associated iOS trait (such as checked)

So we'll want the implementation of accessibilityValue on iOS to continue behaving as it does, but also retrieve the accessibilityValue property from the underlying component (which isn't a string), and present its elements as well.

Incidentally, for range-based values, iOS does not expose a structure like the one you described or the one we're proposing, so we convert the min, current, max range info into a string and place it into accessibliityValue when presenting the range on iOS.

[necolas]

Thanks, I see. In which case it might make sense to use a name other than accessibilityValue

[marcmulcahy]

I've re-factored the code to match your definition of accessibilityValue-- exposing the iOS dictionary property as accessibilityValueInternal, which gets things working as I think we want.

I've also renamed "current" to "now" as your definition suggests.

[blavalla]

By default, Talkback will present the range info first in the announcement string (it's essentially treated like "state"), so adding it here at the very end (when its text) seems a bit backwards.

[marcmulcahy]

My intension is that text and RangeInfo would be mutually exclusive, so if there is text, there wouldn't be RangeInfo set. Do you still have a concern?

[blavalla]

If you are using a "text" accessibilityValueDescription, why disallow an announcement on change? If you add the text to the AccessibilityEvent's "text" property it will read this out instead of the RangeInfo on TYPE_VIEW_SELECTED events.

That way, you could have an element with custom text that is continually changed on adjustment. For example, a star-rating component might be announced as "1 star, product rating, seek control", and on increment it could read "2 stars", "3 stars", etc.

[marcmulcahy]

I believe that updateContentDescription() (I.E. setting the view's content description will generate a WINDOW_CONTENT_CHANGED event. The example second example in RNTester shows an equalizer that only has a textual accessibilityValue, which is read when it is manipulated with the volume keys.

[blavalla]

I just checked it out in the RNTester app and this actually works exactly like you described (and not how I initially assumed it would). Thanks for another great addition!

[blavalla]

Overall this looks good, but I had 2 minor comments inline.

If you wanted to add support for incrementing/decrementing "text" values, that can be a different PR if you'd like, but I think it'd be pretty easy to add to this one as well.

[cpojer]

@marcmulcahy could you address @blavalla's comments so we can merge this? :)

[marcmulcahy]

Sorry, I assumed they were already addressed based on @blavalla 's last comment. Let me know what changes are suggested.

[cpojer]

Ohh sorry my mistake. In that case do you mind rebasing so we can merge it?

[cpojer]

Let's ship it! Thanks @marcmulcahy

[facebook-github-bot]

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by Marc Mulcahy in 7df3eea.

^{When will my fix make it into a release? | Upcoming Releases}