added accessibility props for touchables #5346

fangmobile · 2016-01-15T17:18:03Z

accessibilityLabels are missing in these touchable*.js files.
for #5322
@ide This is not tested yet. I will update with test.

facebook-github-bot · 2016-01-15T17:19:57Z

By analyzing the blame information on this pull request, we identified @spicyj, @mkonicek and @vjeux to be potential reviewers.

mkonicek · 2016-01-18T21:30:27Z

lgtm

cc @sahrens

facebook-github-bot · 2016-01-22T20:08:15Z

@fangmobile updated the pull request.

fangmobile · 2016-01-26T20:35:32Z

my coworker @annjose verified this fix works.
Basically she build react native from source off my fork, added accibilityLabel in touchable highlight in UI explorer example like so

<TouchableHighlight
            accessibilityLabel="AnnAccessLabel_TouchableHighlight_WithImage"
            style={styles.wrapper}
            onPress={() => console.log('stock THW image - highlight')}>
            <Image
              source={heartImage}
              style={styles.image}
            />

And verified the the label shows up in uiautomator viewer.
Please see attached image
Before this fix the content description will be empty even if you set the accessibilityLabel in code.
@ide @mkonicek @sahrens Please let us know whether this is good enough?

facebook-github-bot · 2016-01-28T22:02:15Z

@fangmobile updated the pull request.

facebook-github-bot · 2016-01-28T22:12:15Z

@fangmobile updated the pull request.

fangmobile · 2016-01-28T22:25:45Z

As we was working on another task in our project we noticed textinput does not have accessibility because it is using a touchable. This is more serious than touchable. In touchables sometimes you can get the child view's accessibility label, such as the "Press Me" string in TcouchaExample.js. In textinput.js there is just touchable. And if it is not accessible then the text input field is totally not accessible.

I update this PR with a fix in textinput.js. Attaching some "before" and "after" screenshots. This is tested for Both iOS and Android. BTW the previous commit was also tested both in iOS and Android but there was not screen shot for iOS.

Before this fix there is no accessibility label even if you set them in TextInputExample.android.js or TextINnputExample.ios.js.

iOS before the fix. There is only "value", no accessibility label.

ios after the fix

Android before the fix.

Android after the fix

mkonicek · 2016-01-29T01:01:12Z

Libraries/Components/TextInput/TextInput.js

+     * @platform android
+     */
+    accessibilityComponentType: React.PropTypes.oneOf(View.AccessibilityComponentType),
+    /**


All of these should be already pulled in via the ...View.propTypes above (see View.js). Is there a need to duplicate the props here?

mkonicek · 2016-01-29T01:03:31Z

Besides the inline comment looks good. Thanks for adding the screenshots!

facebook-github-bot · 2016-01-29T03:44:13Z

@fangmobile updated the pull request.

fangmobile · 2016-01-29T04:04:31Z

Thanks @mkonicek for catching this.
You are right that they are not needed. I put them in by mistake because I saw testID there but the rest missing. So when I remove them I also removed testId which should also come from View.js.
Here is the quick test I did. With the accessibility props still set in the example js file:

Remove them from the propTypes part but keep them in return function. Still works.
Remove them from both locations. Accessibility Label is gone.

This show they are not needed in propTypes but need to be returned.

Also squshed commits. Please let me know if there is anything I should do.

ide · 2016-01-29T06:47:46Z

Examples/UIExplorer/TextInputExample.android.js

@@ -208,7 +208,11 @@ exports.examples = [
  {
    title: 'Auto-focus',
    render: function() {
-      return <TextInput autoFocus={true} style={styles.singleLine} />;
+      return <TextInput


code style, please write this as:

return ( <TextInput props..... /> );

ide · 2016-01-29T06:48:24Z

@fangmobile thanks for the test plan!

facebook-github-bot · 2016-01-29T07:12:16Z

@fangmobile updated the pull request.

fangmobile · 2016-01-29T07:27:38Z

@ide Thanks. Amended another commit.

…e examples with accessibility label

fangmobile · 2016-01-29T23:35:42Z

builds were failing due to an unrelated commit on master that was reverted. Now all good.
@ide @mkonicek good to go?

ide · 2016-01-29T23:42:33Z

@facebook-github-bot shipit

facebook-github-bot · 2016-01-29T23:43:43Z

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1558082531149705/int_phab to review.

fangmobile · 2016-01-29T23:53:08Z

👍

fangmobile · 2016-02-02T21:28:12Z

@ mkonicek anything else I should do?
Sorry for the harassment. We need to have accessibility for them for compliance requirement. Also our Android automation is blocked by this because our Android functional tests use Appium which can not read testId (view tag) so we replying on content descriptions.

bestander · 2016-02-04T12:34:19Z

@fangmobile, sorry, our internal CI got stuck with this one.
A colleague is taking control over it right now.
Please ping us in a few hours if it does not get resolved, this is a very important PR for the community 👍

fangmobile · 2016-02-04T15:53:31Z

👍
Thanks guys.

Summary: accessibilityLabels are missing in these touchable*.js files. for #5322 ide This is not tested yet. I will update with test. Closes #5346 Reviewed By: svcscm Differential Revision: D2882061 Pulled By: gkassabli fb-gh-sync-id: dff0ef373e5f5895027cb1cc08c8887a6ace8eee

Summary: accessibilityLabels are missing in these touchable*.js files. for facebook#5322 ide This is not tested yet. I will update with test. Closes facebook#5346 Reviewed By: svcscm Differential Revision: D2882061 Pulled By: gkassabli fb-gh-sync-id: dff0ef373e5f5895027cb1cc08c8887a6ace8eee

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jan 15, 2016

fangmobile force-pushed the accessiblityLabel_for_touchables branch from 69b4421 to 74319ba Compare January 22, 2016 20:07

fangmobile force-pushed the accessiblityLabel_for_touchables branch from b506df5 to 816130a Compare January 28, 2016 22:11

mkonicek reviewed Jan 29, 2016
View reviewed changes

fangmobile force-pushed the accessiblityLabel_for_touchables branch from 816130a to b9298fd Compare January 29, 2016 03:43

ide reviewed Jan 29, 2016
View reviewed changes

fangmobile force-pushed the accessiblityLabel_for_touchables branch from b9298fd to 6a4db1c Compare January 29, 2016 07:11

fangmobile force-pushed the accessiblityLabel_for_touchables branch from 6a4db1c to 2091350 Compare January 29, 2016 16:02

add accessibility properties for touchables and textinput, update som…

bb594ad

…e examples with accessibility label

fangmobile force-pushed the accessiblityLabel_for_touchables branch from 2091350 to bb594ad Compare January 29, 2016 21:16

ide added GH Review: accepted and removed GH Review: review-needed labels Jan 29, 2016

fangmobile mentioned this pull request Jan 29, 2016

accessibilityLabel missing in Touchables ? #5322

Closed

AlbertBrand mentioned this pull request Feb 1, 2016

add accessibilityLabel to TouchableOpacity #5469

Closed

ghost closed this in 33d8db5 Feb 4, 2016

bestander mentioned this pull request Feb 4, 2016

[release-0.20] Commits to cherry pick #5762

Closed

20 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added accessibility props for touchables #5346

added accessibility props for touchables #5346

fangmobile commented Jan 15, 2016

facebook-github-bot commented Jan 15, 2016

mkonicek commented Jan 18, 2016

facebook-github-bot commented Jan 22, 2016

fangmobile commented Jan 26, 2016

facebook-github-bot commented Jan 28, 2016

facebook-github-bot commented Jan 28, 2016

fangmobile commented Jan 28, 2016

mkonicek Jan 29, 2016

mkonicek commented Jan 29, 2016

facebook-github-bot commented Jan 29, 2016

fangmobile commented Jan 29, 2016

ide Jan 29, 2016

ide commented Jan 29, 2016

facebook-github-bot commented Jan 29, 2016

fangmobile commented Jan 29, 2016

fangmobile commented Jan 29, 2016

ide commented Jan 29, 2016

facebook-github-bot commented Jan 29, 2016

fangmobile commented Jan 29, 2016

fangmobile commented Feb 2, 2016

bestander commented Feb 4, 2016

fangmobile commented Feb 4, 2016

added accessibility props for touchables #5346

added accessibility props for touchables #5346

Conversation

fangmobile commented Jan 15, 2016

facebook-github-bot commented Jan 15, 2016

mkonicek commented Jan 18, 2016

facebook-github-bot commented Jan 22, 2016

fangmobile commented Jan 26, 2016

facebook-github-bot commented Jan 28, 2016

facebook-github-bot commented Jan 28, 2016

fangmobile commented Jan 28, 2016

mkonicek Jan 29, 2016

Choose a reason for hiding this comment

mkonicek commented Jan 29, 2016

facebook-github-bot commented Jan 29, 2016

fangmobile commented Jan 29, 2016

ide Jan 29, 2016

Choose a reason for hiding this comment

ide commented Jan 29, 2016

facebook-github-bot commented Jan 29, 2016

fangmobile commented Jan 29, 2016

fangmobile commented Jan 29, 2016

ide commented Jan 29, 2016

facebook-github-bot commented Jan 29, 2016

fangmobile commented Jan 29, 2016

fangmobile commented Feb 2, 2016

bestander commented Feb 4, 2016

fangmobile commented Feb 4, 2016