Add accessibilityHint for iOS #18093

draperunner · 2018-02-26T09:37:25Z

This adds the accessibilityHint for View, Text and Touchable* on iOS.
The accessibilityHint provides some more information about an element
when the accessibilityLabel is not enough.

Motivation

The accessibilityHint is a core accessibility property on iOS.

From https://developer.apple.com/documentation/objectivec/nsobject/1615093-accessibilityhint:

An accessibility hint helps users understand what will happen when they perform an action on the accessibility element when that result is not obvious from the accessibility label.

Related issue: #14706

Test Plan

The npm scripts test, flow, lint and prettier are satisfied.

I added a couple of examples to the RNTester app. The Accessibility Inspector on Mac helps debugging accessibility stuff on a simulator, but it does not show the accessibilityHint. Therefore I tested the RNTester app on an iPhone 8 device using VoiceOver to verify the hint functionality. It works fine, and I've tested disabling and enabling "read hints" in the VoiceOver settings on the phone.

Related PRs

facebook/react-native-website#222

Release Notes

[IOS][FEATURE][Accessibility] - Add accessibilityHint for View, Text, Touchable* on iOS

janicduplessis

Looks good, just one question inline.

janicduplessis · 2018-03-01T23:21:35Z

React/Views/RCTView.m

@@ -154,6 +154,15 @@ - (NSString *)accessibilityLabel
  return RCTRecursiveAccessibilityLabel(self);
 }

+- (NSString *)accessibilityHint


Why is this needed? Can't we just use the prop directly?

You're right, it is not needed. I tested after removing it, and it still works. I've updated this in the latest commit, 196ce06

janicduplessis · 2018-03-12T05:26:38Z

Thanks for the PR!

@facebook-github-bot shipit

facebook-github-bot · 2018-03-12T05:27:00Z

Something went wrong executing that command, @hramos could you take a look?

facebook-github-bot

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

facebook-github-bot · 2018-03-12T17:48:13Z

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

draperunner · 2018-03-15T09:52:52Z

Hi @janicduplessis, seems like facebook-github-bot met some issues. Anything I can do?

hramos · 2018-03-15T18:49:58Z

@draperunner Janic does not have visibility into the failure but I can help in this regard. It looks like this is held back due to a failing snapshot test. Getting this PR merged will require some manual intervention on my part.

It's possible that the failing test is unrelated to this PR, so let me run the internal diff through the test suite one more time. Please do not add any more commits to this PR as doing so will prevent it from landing. Thanks!

janicduplessis · 2018-03-15T18:53:58Z

@hramos This adds a prop to View so it is very possible that some snapshots need to be updated.

hramos · 2018-03-15T21:31:04Z

Indeed that's the case. The failing snapshot tests are all due to accessibilityHint being added to the props. I'll take care of this.

hramos · 2018-03-15T23:15:54Z

Snapshot tests updated and test suites are currently running. I'll check again in tomorrow morning and land this if all looks green.

hramos · 2018-03-17T00:26:21Z

** BUILD FAILED **
The following commands produced analyzer issues:
Analyze Base/RCTModuleMethod.mm
Analyze Modules/RCTUIManager.m
(2 commands with analyzer issues)
The following build commands failed:
Ld /Users/facebook/Library/Developer/Xcode/DerivedData/RNTester-gmzkrbslacaqklccqmdfkejruuag/Build/Products/Debug-iphonesimulator/RNTester.app/RNTester normal x86_64

draperunner · 2018-04-16T10:07:54Z

Any updates on this @hramos?

janicduplessis · 2018-04-17T06:14:18Z

@hramos @draperunner The PR doesn't touch any of the files from the log, maybe rebase on master and we can try shipping again.

This adds the accessibilityHint for View, Text and Touchable* on iOS. The accessibilityHint provides some more information about an element when the accessibilityLabel is not enough.

It is enough to set the prop directly

…utFeedback

draperunner · 2018-06-19T15:30:36Z

@hramos @janicduplessis Hi guys. I have now rebased this PR and added Flow types.

janicduplessis · 2018-06-19T15:36:55Z

@hramos Wanna try landing this again? It will still require some snapshot updates.

facebook-github-bot

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

knitcodemonkey · 2018-07-12T02:24:02Z

Cc @TheSavior

hramos · 2018-07-25T20:14:36Z

This will be landing soon.

react-native-bot · 2018-07-26T01:05:14Z

This pull request was closed by @draperunner in 253b29d.

Once this commit is added to a release, you will see the corresponding version tag below the description at 253b29d. If the commit has a single master tag, it is not yet part of a release.

Summary: This adds the accessibilityHint for View, Text and Touchable* on iOS. The accessibilityHint provides some more information about an element when the accessibilityLabel is not enough. The accessibilityHint is a core accessibility property on iOS. From https://developer.apple.com/documentation/objectivec/nsobject/1615093-accessibilityhint: > An accessibility hint helps users understand what will happen when they perform an action on the accessibility element when that result is not obvious from the accessibility label. Related issue: facebook/react-native#14706 The npm scripts `test`, `flow`, `lint` and `prettier` are satisfied. I added a couple of examples to the RNTester app. The Accessibility Inspector on Mac helps debugging accessibility stuff on a simulator, but it does not show the accessibilityHint. Therefore I tested the RNTester app on an iPhone 8 device using VoiceOver to verify the hint functionality. It works fine, and I've tested disabling and enabling "read hints" in the VoiceOver settings on the phone. facebook/react-native-website#222 [IOS][FEATURE][Accessibility] - Add accessibilityHint for View, Text, Touchable* on iOS Closes facebook/react-native#18093 Reviewed By: hramos Differential Revision: D7230780 Pulled By: ziqichen6 fbshipit-source-id: 172ad28dc9ae2b67ea256100f6acb939f2466d0b

draperunner requested a review from shergin as a code owner February 26, 2018 09:37

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. cla signed labels Feb 26, 2018

draperunner mentioned this pull request Feb 26, 2018

Add documentation for accessibilityHint facebook/react-native-website#222

Merged

sdwilsh removed the cla signed label Mar 1, 2018

janicduplessis suggested changes Mar 1, 2018

View reviewed changes

janicduplessis approved these changes Mar 12, 2018

View reviewed changes

facebook-github-bot added Failed Commands Import Started This pull request has been imported. This does not imply the PR has been approved. labels Mar 12, 2018

facebook-github-bot approved these changes Mar 12, 2018

View reviewed changes

facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Mar 12, 2018

hramos added the Platform: iOS iOS applications. label Mar 13, 2018

hramos added the PR Waiting on FB label Mar 15, 2018

draperunner and others added 4 commits June 19, 2018 16:53

Add accessibilityHint for iOS

ec377b1

This adds the accessibilityHint for View, Text and Touchable* on iOS. The accessibilityHint provides some more information about an element when the accessibilityLabel is not enough.

Remove unnecessary accessibilityHint method

30a0456

Remove unnecessary accessibilityHint method from RCTTextView.m

10adf2e

It is enough to set the prop directly

Remove whitespace change

b6b441f

draperunner force-pushed the accessibility-hint branch from efa01b2 to b6b441f Compare June 19, 2018 14:55

Add accessibilityHint to Props flow-types for View and TouchableWitho…

d42cf36

…utFeedback

facebook-github-bot reviewed Jun 19, 2018

View reviewed changes

ziqichen6 approved these changes Jul 24, 2018

View reviewed changes

knitcodemonkey mentioned this pull request Jul 24, 2018

Add accessibilityHint for View, Text, Touchable* on iOS FormidableLabs/eslint-plugin-react-native-a11y#34

Closed

matthargett approved these changes Jul 25, 2018

View reviewed changes

facebook-github-bot closed this in 253b29d Jul 26, 2018

facebook locked as resolved and limited conversation to collaborators Jul 26, 2018

react-native-bot added the Merged This PR has been merged. label Jul 26, 2018

draperunner deleted the accessibility-hint branch July 26, 2018 13:53

hramos added Needs: Imported Diff Waiting on Meta and removed Waiting for FB labels Jan 30, 2019

hramos removed Import Failed Needs: Imported Diff Waiting on Meta labels Feb 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add accessibilityHint for iOS #18093

Add accessibilityHint for iOS #18093

draperunner commented Feb 26, 2018 •

edited

Loading

janicduplessis left a comment

janicduplessis Mar 1, 2018

draperunner Mar 5, 2018

janicduplessis commented Mar 12, 2018

facebook-github-bot commented Mar 12, 2018

facebook-github-bot left a comment

facebook-github-bot commented Mar 12, 2018

draperunner commented Mar 15, 2018

hramos commented Mar 15, 2018

janicduplessis commented Mar 15, 2018

hramos commented Mar 15, 2018

hramos commented Mar 15, 2018

hramos commented Mar 17, 2018

draperunner commented Apr 16, 2018

janicduplessis commented Apr 17, 2018

draperunner commented Jun 19, 2018

janicduplessis commented Jun 19, 2018

facebook-github-bot left a comment

knitcodemonkey commented Jul 12, 2018

hramos commented Jul 25, 2018

react-native-bot commented Jul 26, 2018

Add accessibilityHint for iOS #18093

Add accessibilityHint for iOS #18093

Conversation

draperunner commented Feb 26, 2018 • edited Loading

Motivation

Test Plan

Related PRs

Release Notes

janicduplessis left a comment

Choose a reason for hiding this comment

janicduplessis Mar 1, 2018

Choose a reason for hiding this comment

draperunner Mar 5, 2018

Choose a reason for hiding this comment

janicduplessis commented Mar 12, 2018

facebook-github-bot commented Mar 12, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Mar 12, 2018

draperunner commented Mar 15, 2018

hramos commented Mar 15, 2018

janicduplessis commented Mar 15, 2018

hramos commented Mar 15, 2018

hramos commented Mar 15, 2018

hramos commented Mar 17, 2018

draperunner commented Apr 16, 2018

janicduplessis commented Apr 17, 2018

draperunner commented Jun 19, 2018

janicduplessis commented Jun 19, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

knitcodemonkey commented Jul 12, 2018

hramos commented Jul 25, 2018

react-native-bot commented Jul 26, 2018

draperunner commented Feb 26, 2018 •

edited

Loading