Extended accessibility actions support #24190

marcmulcahy · 2019-03-28T21:23:12Z

Summary

Extend accessibility actions to include both name (non-localized) and label (localized). this lets components provide programmatic access to custom actions, as well as standard actions such as slider adjustment and scrolling. This also means that when applications process actions, they need not compare against a localized name, which will be different for every language.

Changelog

[general] [change] - Extend accessibility actions support.

Test Plan

We have added tests in RNTester which exercise both custom actions, as well as standard actions.

analysis-bot

Code analysis results:

eslint found some issues. Run yarn lint --fix to automatically fix problems.

analysis-bot · 2019-03-28T21:26:07Z

RNTester/js/AccessibilityIOSExample.js

-          onAccessibilityEscape={() => alert('onAccessibilityEscape success')}
-          accessible={true}>
+          onAccessibilityAction={event => {
+            if (event.nativeEvent.actionName === 'escape') {


no-alert: Unexpected alert.

pull-bot · 2019-03-28T21:26:12Z

	Messages
📖	📋 Changelog Format - Did you include a Changelog? A changelog entry has the following format: `[CATEGORY] [TYPE] - Message`. CATEGORY may be: General iOS Android 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 4f0bfb7

marcmulcahy · 2019-03-28T21:39:45Z

One of the challenges with the current design is the inability for action callback functions to return success or failure. So the implementation assumes that actions always succeed.

This generally works for most types of custom actions, but is problematic for actions like slider adjustment which might not always work.

Instead of specifying actions as a list, we could consider adding API to component to allow actions to be added and removed dynamically. This would allow components to remove actions that aren't expected to work ("increment" when slider is at max, "cut" when no text on clipboard, etc.).

Short of that, is there a way for JavaScript code to return a value to native code?

blavalla · 2019-03-28T23:16:48Z

ReactAndroid/src/main/java/com/facebook/react/uimanager/AccessibilityDelegateUtil.java

+  static {
+      sActionIdMap.put("activate", AccessibilityActionCompat.ACTION_CLICK.getId());
+      sActionIdMap.put("increment", AccessibilityActionCompat.ACTION_SCROLL_FORWARD.getId());
+      sActionIdMap.put("decrement", AccessibilityActionCompat.ACTION_SCROLL_BACKWARD.getId());      


Is the idea here that these are common actions that you might want to override the label for? For example instead of saying "Double tap to [activate]" you'd set your own label on the activate action and have it say "Double tap to [custom label]"? If so, I think AccessibilityActionCompat.ACTION_LONG_CLICK should definitely be included on this list as well.

Sort of. The purpose of actions isn't to customize the usage hint ("double tap to...").

Android conflates the two ideas together, in that in order to add a usage hint to a component, you add a description to it's click action. And that's how usage hints are hooked up in our proposed React Native implementation of usage hints on Android.

Adding long click to the list of supported standard actions might be a good idea, but there wouldn't be any way to have that action force TalkBack to say something.

The other complication is that because of the way Android implements usage hints, all usage hints start with "double tap to...". On iOS, usage hints are simply strings, so can start with anything.

I suspect adding a "longclick" standard action name makes sense-- I'll look to see what the iOS equivalent would be.

analysis-bot

Code analysis results:

eslint found some issues. Run yarn lint --fix to automatically fix problems.

analysis-bot · 2019-03-29T20:33:44Z

RNTester/js/AccessibilityIOSExample.js

-          onAccessibilityEscape={() => alert('onAccessibilityEscape success')}
-          accessible={true}>
+          onAccessibilityAction={event => {
+            if (event.nativeEvent.actionName === 'escape') {


no-alert: Unexpected alert.

ikenwoo

Looks good, thanks!

Just a few comments regarding type safety and some implied logic that isn't clear from the js side.

ikenwoo · 2019-04-02T20:07:45Z

React/Views/RCTView.h

@@ -29,7 +29,8 @@
 /**
 * Accessibility properties
 */
-@property (nonatomic, copy) NSArray <NSString *> *accessibilityActions;
+@property (nonatomic, copy) NSArray <NSDictionary *> *accessibilityActions;
+@property (nonatomic, copy) NSDictionary *accessibilityActionsMap;


can we add the type information for the NSDictionary

ikenwoo · 2019-04-02T20:09:37Z

React/Views/RCTView.m

@@ -99,6 +99,8 @@ - (UIView *)react_findClipView
 @implementation RCTView
 {
  UIColor *_backgroundColor;
+  NSMutableDictionary *accessibilityActionsNameMap;
+  NSMutableDictionary *accessibilityActionsLabelMap;


annotate the type of the collections here as well

ikenwoo · 2019-04-02T20:10:16Z

React/Views/RCTView.m

+-(void)setAccessibilityActions:(NSArray *)actions
+{
+  if (!actions || !actions.count) {
+    return;


It doesn't look like the dictionaries get emptied out if someone tries to remove actions, e.g. [view setAccessibilityActions:@[]]

ikenwoo · 2019-04-02T20:11:58Z

React/Views/RCTView.m

+  if (!actions || !actions.count) {
+    return;
+  }
+  _accessibilityActions = [NSMutableArray array];


the assignment here is not needed since you set this with a copy at L175.

ikenwoo · 2019-04-02T20:17:28Z

React/Views/RCTView.m

 - (NSArray <UIAccessibilityCustomAction *> *)accessibilityCustomActions
 {
-  if (!_accessibilityActions.count) {
+  if (!_accessibilityActions || !_accessibilityActions.count) {


extra nil check isn't needed

ikenwoo · 2019-04-02T20:54:43Z

React/Views/RCTView.m

  }
+       else {


ikenwoo · 2019-04-02T20:55:10Z

React/Views/RCTView.m

+  if ([self performAccessibilityAction:@"magicTap"]) {
+    return YES;
+  }
+  else if (_onMagicTap) {


} else if ...

ikenwoo · 2019-04-02T20:55:23Z

React/Views/RCTView.m

    _onMagicTap(nil);
    return YES;
-  } else {
+  }
+  else {


ikenwoo · 2019-04-02T20:56:20Z

React/Views/RCTView.m

    _onAccessibilityEscape(nil);
    return YES;
-  } else {
+  }
+  else {


ikenwoo · 2019-04-02T20:56:32Z

React/Views/RCTView.m

+  if ([self performAccessibilityAction:@"escape"]) {
+    return YES;
+  }
+  else if (_onAccessibilityEscape) {


} else if ... {

analysis-bot

Code analysis results:

eslint found some issues. Run yarn lint --fix to automatically fix problems.

analysis-bot · 2019-04-05T20:51:43Z

RNTester/js/AccessibilityIOSExample.js

-          onAccessibilityEscape={() => alert('onAccessibilityEscape success')}
-          accessible={true}>
+          onAccessibilityAction={event => {
+            if (event.nativeEvent.actionName === 'escape') {


no-alert: Unexpected alert.

blavalla · 2019-04-10T23:01:28Z

Instead of specifying actions as a list, we could consider adding API to component to allow actions to be added and removed dynamically. This would allow components to remove actions that aren't expected to work ("increment" when slider is at max, "cut" when no text on clipboard, etc.).

Short of that, is there a way for JavaScript code to return a value to native code?

@shergin, do you know if there is a way to do this?

shergin · 2019-04-29T16:41:02Z

@marcmulcahy Totally unrelated to this diff: Feel free to implement the same for Fabric here: https://github.com/facebook/react-native/blob/master/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm#L522-L570

shergin · 2019-04-29T16:43:54Z

Instead of specifying actions as a list, we could consider adding API to component to allow actions to be added and removed dynamically. This would allow components to remove actions that aren't expected to work ("increment" when slider is at max, "cut" when no text on clipboard, etc.).

Do you mean some kinda imperative API (like scrollTo(x,y) or .focus())? If so, IMHO, no. It's totally overkill. Props is a the most simple and idiomatic way to deal with it.

shergin · 2019-04-29T16:52:46Z

React/Views/RCTView.m

@@ -99,6 +99,8 @@ - (UIView *)react_findClipView
 @implementation RCTView
 {
  UIColor *_backgroundColor;
+  NSMutableDictionary<NSString *, NSDictionary *> *accessibilityActionsNameMap;


Not sure I understand why we need to have two maps here.

@shergin In this design, actions have a non-localized name and a localized label.

in iOS, custom accessibility actions only have a localized label, which is what is passed to didActivateAccessibilityCustomAction. The label map is used to find the action name to include with the event to JS.

For standard actions like increment, decrement, and activate, we get specific calls from iOS (such as didActivate). We use the name map to validate whether the JS component claims to support that standard action.

…t of strings.

…ions support includes 'increment', 'decrement', 'magticTap', 'escape', and 'activate'.

… way to implement this on iOS.

analysis-bot

Code analysis results:

flow found some issues.
eslint found some issues. Run yarn lint --fix to automatically fix problems.

analysis-bot · 2019-05-01T17:16:43Z

Libraries/Components/View/ViewPropTypes.js

   */
-  onAccessibilityAction?: ?(string) => void,
+  onAccessibilityAction?: ?(event: AccessibilityActionEvent) => void,


Cannot resolve name AccessibilityActionEvent.

analysis-bot · 2019-05-01T17:16:43Z

Libraries/Components/View/ViewPropTypes.js

@@ -417,6 +409,12 @@ export type ViewProps = $ReadOnly<{|
   */
  accessibilityStates?: ?AccessibilityStates,

+  /**
+   * Provides an array of custom actions available for accessibility.
+   *


Cannot resolve name AccessibilityActionInfo.

analysis-bot · 2019-05-01T17:16:44Z

RNTester/js/AccessibilityExample.js

+            <Text>Slider</Text>
+          </View>
+        </RNTesterBlock>
+        </View>


prettier/prettier: Delete ··

marcmulcahy · 2019-05-01T20:43:39Z

This breaks double tap on Android 9.0. closing while I investigate and fix the issue, and will submit a new pull request once finished.

marcmulcahy · 2019-05-13T21:39:19Z

@janicduplessis Can you make sure I haven't broken things again. I've moved actions back into RCTView, as we need to keep some class-specific maps around to match action names to labels and vice versa.

If you think UIView+React.* is the right place for actions, could you suggest how setAccessibilityActions (and the associated actionsN ameMNap and actionsLabelMap) could be factored out of RCTView?

Summary: This is a reconstitution of #24190. It extends accessibility actions to include both a name and user facing label. These extensions support both standard and custom actions. We've also added actions support on Android, and added examples to RNTester showing how both standard and custom accessibility actions are used. ## Changelog [general] [changed] - Enhanced accessibility actions support Pull Request resolved: #24695 Differential Revision: D15391408 Pulled By: cpojer fbshipit-source-id: 5ed48004d46d9887da53baea7fdcd0e7e15c5739

Summary: This is a reconstitution of facebook#24190. It extends accessibility actions to include both a name and user facing label. These extensions support both standard and custom actions. We've also added actions support on Android, and added examples to RNTester showing how both standard and custom accessibility actions are used. ## Changelog [general] [changed] - Enhanced accessibility actions support Pull Request resolved: facebook#24695 Differential Revision: D15391408 Pulled By: cpojer fbshipit-source-id: 5ed48004d46d9887da53baea7fdcd0e7e15c5739

marcmulcahy requested a review from shergin as a code owner March 28, 2019 21:23

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Amazon Partner: Amazon labels Mar 28, 2019

analysis-bot suggested changes Mar 28, 2019

View reviewed changes

blavalla reviewed Mar 28, 2019

View reviewed changes

analysis-bot suggested changes Mar 29, 2019

View reviewed changes

ikenwoo suggested changes Apr 2, 2019

View reviewed changes

analysis-bot suggested changes Apr 5, 2019

View reviewed changes

xuelgong force-pushed the a11y-actions branch from 2856913 to 39de224 Compare April 5, 2019 23:46

estevaolucas mentioned this pull request Apr 24, 2019

Additional Accessibility Roles and States #24095

Closed

shergin reviewed Apr 29, 2019

View reviewed changes

xuelgong and others added 7 commits May 1, 2019 12:13

Modify accessibilityActions to be a list of objects rather than a lis…

d7b065b

…t of strings.

Add iOS support for standard and custom actions. Initial standard act…

a537332

…ions support includes 'increment', 'decrement', 'magticTap', 'escape', and 'activate'.

Add Android support for accessibility actions.

2feb6e8

Add examples of accessibility actions.

7968cf4

Add "longpress" as a standard action on Android. there seems to be no…

b5b8ef5

… way to implement this on iOS.

Code cleanup.

bf7187e

Fix lint error.

3f80e84

marcmulcahy force-pushed the a11y-actions branch from 39de224 to 3f80e84 Compare May 1, 2019 17:14

analysis-bot suggested changes May 1, 2019

View reviewed changes

Fixed lint errors and missing imports

4f0bfb7

marcmulcahy closed this May 1, 2019

marcmulcahy mentioned this pull request May 2, 2019

Extended Accessibility Actions Support #24695

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extended accessibility actions support #24190

Extended accessibility actions support #24190

marcmulcahy commented Mar 28, 2019

analysis-bot left a comment

analysis-bot Mar 28, 2019

pull-bot commented Mar 28, 2019 •

edited

Loading

marcmulcahy commented Mar 28, 2019

blavalla Mar 28, 2019

marcmulcahy Mar 28, 2019

analysis-bot left a comment

analysis-bot Mar 29, 2019

ikenwoo left a comment

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

ikenwoo Apr 2, 2019

analysis-bot left a comment

analysis-bot Apr 5, 2019

blavalla commented Apr 10, 2019 •

edited

Loading

shergin commented Apr 29, 2019

shergin commented Apr 29, 2019

shergin Apr 29, 2019

marcmulcahy May 6, 2019

analysis-bot left a comment

analysis-bot May 1, 2019

analysis-bot May 1, 2019

analysis-bot May 1, 2019

marcmulcahy commented May 1, 2019

marcmulcahy commented May 13, 2019

Extended accessibility actions support #24190

Extended accessibility actions support #24190

Conversation

marcmulcahy commented Mar 28, 2019

Summary

Changelog

Test Plan

analysis-bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pull-bot commented Mar 28, 2019 • edited Loading

marcmulcahy commented Mar 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

analysis-bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikenwoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

analysis-bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blavalla commented Apr 10, 2019 • edited Loading

shergin commented Apr 29, 2019

shergin commented Apr 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

analysis-bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcmulcahy commented May 1, 2019

marcmulcahy commented May 13, 2019

pull-bot commented Mar 28, 2019 •

edited

Loading

blavalla commented Apr 10, 2019 •

edited

Loading