VoiceOver accessibility issues in RN .62 #603

tom-un · 2020-09-17T18:42:06Z

Please select one of the following

I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
I am making a fix / change for the macOS implementation of react-native
I am making a change required for Microsoft usage of react-native

Summary

There were a number of VoiceOver accessibility issues in react-native-macos since merging down .62.

accessibilityRole

When react-native-macos was first ported from iOS to macOS, there was no accessibilityRole prop. There was a now deprecated accessibilityTraits prop, which corresponded to the -[UIAccessibility accessibilityTraits] property on iOS. On macOS there is no such property, but there is -[NSAccessibility accessibilityRole] property, so for react-native-macos the accessibilityTraits prop implementation was made to map to -[NSAccessibility accessibilityRole]. The iOS traits button, image, link, and text were mapped to the macOS roles NSAccessibilityButtonRole, NSAccessibilityImageRole, NSAccessibilityLinkRole, and NSAccessibilityStaticTextRole. Also, the accessibilityTraits prop was extended to accept group and list which mapped to NSAccessibilityGroupRole and NSAccessibilityListRole and are used in the macOS implementation of VirtualList, SectionList, FlatList, etc.

In later RN versions an accessibilityRole prop was added to replace accessibilityTraits. It contains a list of platform agnostic role names that map to traits on iOS and roles on Android. This change extends the macOS support for the full set of these RN accessibilityRole values, and extends that list to include group and list.

accessibilityLabel

The native accessibilityLabel property in react-native is supposed to return the JS accessibilityLabel prop value if one was set, or return a concatenation of the child views including any text views. This recursive ability was broken in .62 such that even simple <Button>'s were not having their label's announced. This change restores it.

accessibilityStates

RN used to have an accessibilityStates prop (plural) but it was deprecated and replaced by accessibilityState. This change adds macOS support for settings states such as checked/unchecked, expanded/collapsed, disabled, and selected. These are all working, however for selected the corresponding isAccessibilitySelected property on macOS only is announced when very particular accessibilityRole and accessibilitySubRole combinations are set such as those for tables and outline views.

onAccessibilityAction / accessibilityActions

The onAccessibilityAction / accessibilityActions props allow the app to respond to gestures such as swipe up/down to adjust sliders, double tap to activate, and set custom actions in the rotor. react-native-macos has never had proper support for these. Although the custom rotor actions now work in macOS, there is no announcement like on iOS saying "actions available" -- it will be up to the app developer to add an appropriate accessibilityHint prop (which maps to the native accessibilityHelp on mac) saying custom actions are available.

Changelog

[Fix] [macOS] - VoiceOver accessibility issues in RN .62

Test Plan

Every case in the Accessibility test page in macOS RNTester was tested and compared against the functionality in the iOS RNTester. The follow list is a comparison of the iOS results versus the macOS results with notes where behavior differs.

TextView without label

[x] iOS: reads label
[x] macOS: reads label

TextView with label

[x] iOS: read accessibilityLabel instead of label
[x] macOS: read accessibilityLabel instead of label

Non-accessible view with TextView

[x] iOS: green and blue text separately reachable and reads each.
[x] macOS: green and blue text separately reachable and reads each.

Accessible view with TextViews without label

[x] iOS: green and blue reachable as one, reads both as one.
[x] macOS: green and blue reachable as one, reads both as one.

Accessible view with TextView with label

[x] iOS: green and blue reachable as one, reads accessibilityLabel instead of visible text
[x] macOS: green and blue reachable as one, reads accessibilityLabel instead of visible text

Accessible view with TextViews with hint

[x] iOS: green and blue reachable as one, reads both as one, reads accessibilityHint
[x] macOS: green and blue reachable as one, reads both as one, reads accessibilityHint

Accessible view TextViews with label and hint

[x] iOS: green and blue reachable as one, reads accessibilityLabel and accessibilityHint
[x] macOS: green and blue reachable as one, reads accessibilityLabel and accessibilityHint

Text with accessibilityRole = header

[x] iOS: "This is a title, heading"
[x] macOS: "This is a title" ... "you are currently on a text element inside a scroll area"

Touchable with accessibilityRole = link

[x] iOS: "Click me, link", click: Alert shown
[x] macOS: "link, Click me", click: Alert shown

Touchable with accessibilityRole = button

[x] iOS: "Click me, button", click: Alert shown
[x] macOS: "Click me, button", click: Alert shown

Disabled Touchable with role

[x] iOS: "I am disabled..., dimmed, button", click: nothing
[x] macOS: "I am disabled..., dimmed, button", click: nothing

View with multiple states

[x] iOS: "selected, This view is selected..., dimmed"
[-] macOS: "This view is selected..., dimmed" selected not announced, but it IS shown as Selected in Accessibility Inspector -- only certain roles can announce selection

View with label, hint, role, and state

[x] iOS: "selected, accessibilityLabel, button, ... accessibilityHint"
[-] macOS: "accessibilityLabel, button, ... accessibilityHint" selected not announced, but it IS shown as Selected in Accessibility Inspector -- only certain roles can announce selection

New accessibility roles and states

Alert example

[x] iOS: "element 1, alert"
[-] macOS: "element 1" no exact match on macos

Checkbox example

[x] iOS: "element 2, checkbox, checked, click be to change state"
[x] macOS: "element 2, checked, checkbox

Combobox example

[x] iOS: "element 3, combo box"
[x] macOS: "element 3, combo box"

Menu example

[x] iOS: "element 4, menu"
[x] macOS: "element 4, menu"

Menu bar example

[x] iOS: "element 5, menu bar"
[x] macOS: "element 5, menu bar"

Menu item example

[x] iOS: "element 6, menu item"
[x] macOS: "element 6, ... you are currently on a menu item"

Progress bar example

[x] iOS: "element 7, progress bar"
[x] macOS: "element 7, progress bar"

Radio button example

[x] iOS: "element 8, radio button"
[x] macOS: "element 8, radio button"

Radio button group

[x] iOS: "element 9, radio group"
[x] macOS: "element 9, radio group"

Scrollbar example

[x] iOS: "element 10, scrollbar"
[ ] macOS: cannot focus on this element

Spin button example

[x] iOS: "element 11, spin button"
[x] macOS: "element 11, stepper"

Switch example

[x] iOS: "element 12, switch button, on, ... double tap to toggle setting"
[x] macOS: "element 12, checked, checkbox"

Tab example

[x] iOS: "element 13, tab"
[-] macOS: "element 13, button" no exact match on macos

Tab list example

[x] iOS: "element 14, tab list"
[x] macOS: "element 14, tab group"

Timer example

[x] iOS: "element 15, timer"
[-] macOS: "element 15" no exact match on macos

Toolbar example

[x] iOS: "element 16, toolbar"
[x] macOS: "element 16, toolbar item palette"

State busy example

[x] iOS: "element 17, busy"
[-] macOS: "element 17" no exact match on macos

Expandable element example

[x] iOS: "element 18, collapsed, ... click me to change state", double tap toggles expand/collapse
[x] macOS: "element 18, collapsed, ... click me to change state", Ctrl+Opt+space toggles expand/collapse

Selectable element example

[x] iOS: "selected, element 19, dimmed, ... use the button on the right to enable selection". Clicking Enable selection then changes element 19 to say: "selected, element 19, click me to unselect". Double tap: "element 19"
[-] macOS: "element 19, dimmed" selected not announced, but it IS shown as Selected in Accessibility Inspector -- only certain roles can announce selection

Nested checkbox with delayed state changed

[x] iOS: "Meat, checkbox, not checked, ... state changes in two seconds after clicking"
[x] macOS: "Meat, unchecked, checkbox"

Accessibility action examples

non-touchable with activate action

[x] iOS: "click me", click: shows alert
[x] macOS: "click me", click: shows alert

View with multiple actions

[x] iOS: "this view support many actions ... actions available". Rotor to Active "this view supports many actions ... swipe up and down to select a custom action, then double tap to activate". switch up/down: "cut label", "copy label", "paste label", "activate ... default"
[-] macOS: "this view support many actions", no actions available announced, Ctrl+Opt+Cmd+Space shows Action Menu with "cut label", "copy label", "paste label"

Adjustable with increment/decrement actions

[x] iOS: "Slider, adjustable, swipe up or down with one finger to adjust the value", Swipe up or down: Alert
[x] macOS: "slider, Slider", Ctrl+Opt+Space show Alert that increment has happened. Ctrl+Opt+Cmd+Space shows Action menu with "increment", "decrement"

Button with custom accessibility actions

[x] iOS: "click me, button, ... actions available". Rotor to Active "this view supports many actions ... swipe up and down to select a custom action, then double tap to activate", switch up/down: "cut label", "copy label", "paste label", "activate ... default"
[-] macOS: "click me, button", no actions available announced, Ctrl+Opt+Cmd+Space shows Action Menu with "cut label", "copy label", "paste label"

Fake Slider Example

Fake Slider

[x] iOS: "Fake slider, 50%, adjustable, swipe up or down with one finger to adjust the value", Swipe: "52%"
[x] macOS: "50%, Fake slider, slider", Ctrl+Opt+Space: "Increment 52%"

Equalizer

[x] iOS: "equalizer, center, adjustable, swipe up or down with one finger to adjust the value", Swipe: "right"
[x] macOS: "center, equalizer, slider", Ctrl+Opt+Space: "Increment right"

Check if the screen reader is enabled

[x] iOS: The screen read is enabled/disabled
[x] macOS: The screen read is enabled/disabled

Check if the display options are enabled

[x] iOS: Invert colors is disabled/enabled
[x] macOS: Invert colors is disabled/enabled

[x] iOS: Reduce motion is disabled/enabled
[x] macOS: Reduce motion is disabled/enabled

[x] iOS: Reduce transparency is disabled/enabled
[x] macOS: Reduce transparency is disabled/enabled

Check if the screen reader announces

[x] iOS: "Announce for Accessibility, button", double tapping says "Announce for Accessibility"
[x] macOS: "Announce for Accessibility, button", Ctrl+Opt+Space: "Announcement Test"

Check if the screen reader focus sets

[x] iOS: "Set Accessibility Focus, button", double tapping moves to label below
[x] macOS: "Set Accessibility Focus, button", Ctrl+Opt+Space: moves to label below

Microsoft Reviewers: Open in CodeFlow

…iew." This reverts commit 5a67ae0.

pull-bot · 2020-09-17T18:45:14Z

	Messages
📖	📋 Verify Changelog Format - A changelog entry has the following format: `[CATEGORY] [TYPE] - Message`. Details CATEGORY may be: General macOS 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 4f19714

HeyImChris · 2020-09-17T18:59:23Z

This will presumably have to go in to the 0.62-stable branch as well

React/Views/RCTView.m

alloy

Nice one, looks great 👌

Some of my comments are more about me trying to learn about the MS objc style, feel free to ignore those. I’ll defer to @HeyImChris for how much of a requirement these objc guide changes are.

React/Base/RCTConvert.m

React/Views/RCTView.m

alloy · 2020-09-18T09:36:36Z

React/Views/RCTView.m

  }
+#if TARGET_OS_OSX // [TODO(macOS ISS#2323203)
+  // calling super.accessibilityLabel above on macOS causes the return value of this accessor to be ignored by VoiceOver.
+  // Calling the super's setAccessibilityLabel with nil ensures that the return value of this accessor is used by VoiceOver.


Does the nil setting break things on iOS or can we remove the platform checks and upstream it?

It probably is fine to nil it on iOS too -- but AX is such a touchy area with lots of undocumented behavior in VoiceOver that I'd rather not risk any deltas to the iOS path.

I’m going to unhide this one, just in case I ever want to come back to this while unforking.

React/Views/RCTView.m

alloy · 2020-09-18T09:56:29Z

React/Views/RCTView.m

+    }
+  }
+  return nil;
+}


Just for my understanding, what else can these values be that they can be ignored?

The accessibilityMinValue and accessibilityMaxValue are used for sliders and steppers roles. When the appropriate role, like slider, is set, VoiceOver will automatically compute percentages and announce them. i.e. if accessibilityMinValue=0, accessibilityMaxValue=100, and accessibilityValue = 50, voiceover will say "50%"

Unhiding for posterity.

alloy · 2020-09-18T10:00:54Z

React/Views/RCTView.m

+  } else if (_onClick) {
+    _onClick(nil);
+    return YES;
+#endif // ]TODO(macOS ISS#2323203)


Do you imagine that at some point we might be able to upstream things like this and just have it not do anything on platforms that don’t have the concept of clicks?

This is a weird one. On iOS returning NO from accessibilityActivate can make the system simulate a touch on the view instead which ends up indirectly causing an onPress on a touchable get called. On macOS this doesn't happen. There are a number of cases where a View or Touchable does not have an onAccessiblityAction handler but does have on onPress and expects a VoiceOver double tap to call it. It's an important case to support and this was the most obvious way to simulate a click -- we do it in other places such as performKeyEquivalent:.

Good to know, thanks for the update! (I’ll actually unresolve this thread, as it’s just good general info for posterity.)

BTW, I think I found how mac VoiceOver can simulate a click on VO activate: if the accessibilityActivationPoint prop is set https://developer.apple.com/documentation/appkit/nsaccessibility/1535149-accessibilityactivationpoint?language=objc. Might be worth further investigation in the future, but don't want to add risk to this change by introducing another property.

tido64

LGTM. Only a few nitpicks.

React/Base/RCTConvert.m

React/Views/RCTView.m

React/Views/RCTViewManager.m

alloy · 2020-09-18T14:20:40Z

React/Base/RCTUIKit.h

 //

+// UIAccessibility.h/NSAccessibility.h
+@compatibility_alias UIAccessibilityCustomAction NSAccessibilityCustomAction;


@tom-un When we move RCTUIKit upstream at some point, do you think it makes sense to rename these symbols to be platform agnostic? E.g. in this case RCTAccessibilityCustomAction?

Yes, they should all be normalized to a consistent convention if/when we contribute upstream. I'm partial to things like RCTUIView -> UIView/NSView, because there are already several UIKit subclass upstream that use just the RCT prefix, like RCTView.

React/Base/RCTConvert.m

React/Views/RCTView.m

HeyImChris · 2020-09-18T17:47:54Z

React/Views/RCTView.m

+  if ([self accessibilityRole] == NSAccessibilityCheckBoxRole) {
+    for (NSString *state in [self accessibilityState]) {
+      id val = [self accessibilityState][state];
+      if (val != nil) {


This and the if statement below can be combined into one

HeyImChris · 2020-09-18T17:49:19Z

React/Views/RCTView.m

 #endif // ]TODO(macOS ISS#2323203)
 {
+#if TARGET_OS_OSX // [TODO(macOS ISS#2323203)
+  if ([self isAccessibilityEnabled] == NO) {


This method is plagued by early returns. Would be awesome if we could fix those up instead of adding one more to it :)

…angleRole. Make accessibilityValue return "checked" state for checkbox, radiobutton, and disclosure roles.

…anary builds set the version to 0.0.0-<commithash> via `node scripts/bump-oss-version.js --nigh tly` which causes the "Verify react-native-macos-init MacDebug" CI test to fail bec ause `pod install` fails due to `.podspec` versions that are 0.0.0. Add a `--testi ng` switch to `bump-oss-version.js` that changes the version to 1000.0.0 and use th at switch in `react-native-macos-init.yml`.

VoiceOver accessibility issues in RN .62 (microsoft#603) * Update scripts to publish react-native-macos-init * Clean up merge markers * Restored ios:macos RNTester parity except for InputAccessoryView. * Revert "Restored ios:macos RNTester parity except for InputAccessoryView." This reverts commit 5a67ae0. * Remove unnecessary android builds and tar file upload. * RN .62 accessiblity fixes. * Fix regression in recursive accessiblityLabel text. * Changed per PR feedback. * Add "disclosure" role for mac mapping to NSAccessibilityDisclosureTriangleRole. Make accessibilityValue return "checked" state for checkbox, radiobutton, and disclosure roles. * CocoaPods does not like podspec version numbers that are 0.0.0. The canary builds set the version to 0.0.0-<commithash> via `node scripts/bump-oss-version.js --nigh tly` which causes the "Verify react-native-macos-init MacDebug" CI test to fail bec ause `pod install` fails due to `.podspec` versions that are 0.0.0. Add a `--testi ng` switch to `bump-oss-version.js` that changes the version to 1000.0.0 and use th at switch in `react-native-macos-init.yml`. Co-authored-by: React-Native Bot <53619745+rnbot@users.noreply.github.com>

VoiceOver accessibility issues in RN .62 (#603) * Update scripts to publish react-native-macos-init * Clean up merge markers * Restored ios:macos RNTester parity except for InputAccessoryView. * Revert "Restored ios:macos RNTester parity except for InputAccessoryView." This reverts commit 5a67ae0. * Remove unnecessary android builds and tar file upload. * RN .62 accessiblity fixes. * Fix regression in recursive accessiblityLabel text. * Changed per PR feedback. * Add "disclosure" role for mac mapping to NSAccessibilityDisclosureTriangleRole. Make accessibilityValue return "checked" state for checkbox, radiobutton, and disclosure roles. * CocoaPods does not like podspec version numbers that are 0.0.0. The canary builds set the version to 0.0.0-<commithash> via `node scripts/bump-oss-version.js --nigh tly` which causes the "Verify react-native-macos-init MacDebug" CI test to fail bec ause `pod install` fails due to `.podspec` versions that are 0.0.0. Add a `--testi ng` switch to `bump-oss-version.js` that changes the version to 1000.0.0 and use th at switch in `react-native-macos-init.yml`. Co-authored-by: React-Native Bot <53619745+rnbot@users.noreply.github.com> Co-authored-by: React-Native Bot <53619745+rnbot@users.noreply.github.com>

tom-un and others added 30 commits April 3, 2020 20:53

Update scripts to publish react-native-macos-init

685f103

Merge remote-tracking branch 'ms/master'

8ccd26c

Merge remote-tracking branch 'ms/master'

45d2ee2

Clean up merge markers

1e2c329

Merge remote-tracking branch 'ms/master'

408dae3

Merge remote-tracking branch 'ms/master'

bdaa8ad

Restored ios:macos RNTester parity except for InputAccessoryView.

5a67ae0

Revert "Restored ios:macos RNTester parity except for InputAccessoryV…

e5a6df2

…iew." This reverts commit 5a67ae0.

Merge remote-tracking branch 'ms/master'

4bca23c

Merge remote-tracking branch 'ms/master'

d067629

Merge remote-tracking branch 'ms/master'

6ed01c8

Merge remote-tracking branch 'ms/master'

ae5cc57

Merge remote-tracking branch 'ms/master'

caf0975

Merge remote-tracking branch 'ms/master'

e3fbee9

Merge remote-tracking branch 'ms/master'

a75050c

Merge remote-tracking branch 'ms/master'

3a77e37

Merge remote-tracking branch 'ms/master'

d10ff74

Merge remote-tracking branch 'ms/master'

dd11d73

Merge remote-tracking branch 'ms/master'

ba2730b

Remove unnecessary android builds and tar file upload.

5da14a9

Merge remote-tracking branch 'ms/master'

b1a873b

Merge remote-tracking branch 'ms/master'

bca9ae9

Merge remote-tracking branch 'ms/master'

7ff5624

Merge remote-tracking branch 'ms/master'

1056600

Merge remote-tracking branch 'ms/master'

d3422e3

Merge remote-tracking branch 'ms/master'

92c66ac

Merge remote-tracking branch 'ms/master'

b295454

Merge remote-tracking branch 'ms/master'

1bdf8c7

Merge remote-tracking branch 'ms/master'

b880837

Merge remote-tracking branch 'ms/master'

3c7c7c3

tom-un added 3 commits September 17, 2020 18:00

RN .62 accessiblity fixes.

98a37d6

Fix regression in recursive accessiblityLabel text.

274a1c1

Merge remote-tracking branch 'ms/master' into tomun/rn62-ax-fixes

0d9af6a

tom-un requested review from HeyImChris, alloy and tido64 September 17, 2020 18:42

This was linked to issues Sep 17, 2020

Clean up merges: onAccessibilityTap, accessibilityHint, accessibilityRole and accessibilityStates #237

Closed

[React Native: Mac] AccessibilityActionInfo name component in mac is different from win32 #411

Closed

HeyImChris suggested changes Sep 17, 2020

View reviewed changes

ghost added the Needs: Author Feedback label Sep 17, 2020

alloy approved these changes Sep 18, 2020

View reviewed changes

ghost removed the Needs: Author Feedback label Sep 18, 2020

tido64 approved these changes Sep 18, 2020

View reviewed changes

Changed per PR feedback.

c7829d7

tom-un mentioned this pull request Sep 18, 2020

List items are not showing selection highlight color #605

Closed

alloy reviewed Sep 18, 2020

View reviewed changes

harrieshin reviewed Sep 18, 2020

View reviewed changes

React/Base/RCTConvert.m Show resolved Hide resolved

harrieshin reviewed Sep 18, 2020

View reviewed changes

React/Base/RCTConvert.m Outdated Show resolved Hide resolved

harrieshin reviewed Sep 18, 2020

View reviewed changes

React/Views/RCTView.m Outdated Show resolved Hide resolved

HeyImChris approved these changes Sep 18, 2020

View reviewed changes

tom-un added 2 commits September 20, 2020 19:36

Add "disclosure" role for mac mapping to NSAccessibilityDisclosureTri…

b94713f

…angleRole. Make accessibilityValue return "checked" state for checkbox, radiobutton, and disclosure roles.

tom-un merged commit aac33da into microsoft:master Sep 21, 2020

This was referenced Sep 21, 2020

cherry-pick VoiceOver accessibility issues from master to 0.62-stable #606

Merged

Port --ci switch into bump-oss-version.js and remove superfluous --testing switch. #607

Merged

tom-un linked an issue Sep 21, 2020 that may be closed by this pull request

Accessibility props for macOS need to be normalized with core #266

Closed

3 tasks

VoiceOver accessibility issues in RN .62 #603

VoiceOver accessibility issues in RN .62 #603

Uh oh!

Conversation

tom-un commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Please select one of the following

Summary

accessibilityRole

accessibilityLabel

accessibilityStates

onAccessibilityAction / accessibilityActions

Changelog

Test Plan

TextView without label

TextView with label

Non-accessible view with TextView

Accessible view with TextViews without label

Accessible view with TextView with label

Accessible view with TextViews with hint

Accessible view TextViews with label and hint

Text with accessibilityRole = header

Touchable with accessibilityRole = link

Touchable with accessibilityRole = button

Disabled Touchable with role

View with multiple states

View with label, hint, role, and state

New accessibility roles and states

Alert example

Checkbox example

Combobox example

Menu example

Menu bar example

Menu item example

Progress bar example

Radio button example

Radio button group

Scrollbar example

Spin button example

Switch example

Tab example

Tab list example

Timer example

Toolbar example

State busy example

Expandable element example

Selectable element example

Nested checkbox with delayed state changed

Accessibility action examples

non-touchable with activate action

View with multiple actions

Adjustable with increment/decrement actions

Button with custom accessibility actions

Fake Slider Example

Fake Slider

Equalizer

Check if the screen reader is enabled

Check if the display options are enabled

Check if the screen reader announces

Check if the screen reader focus sets

Microsoft Reviewers: Open in CodeFlow

Uh oh!

pull-bot commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HeyImChris commented Sep 17, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alloy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tom-un commented Sep 17, 2020 •

edited

Loading

pull-bot commented Sep 17, 2020 •

edited

Loading

alloy left a comment •

edited

Loading