[Accessibility] Fix Image does not announce "disabled" #31252

fabOnReact · 2021-03-26T18:14:42Z

Summary

This issue fixes #30935 fixes #30936 screenreader does not announce Image disabled accessibilityState.

As stated in AOSP View.java, the framework will handle routine focus movement, views indicate their willingness to take focus through the isFocusable method https://bit.ly/3dCnyHb

* <p>The framework will handle routine focus movement in response to user input. This includes
* changing the focus as views are removed or hidden, or as new views become available. Views
* indicate their willingness to take focus through the {@link #isFocusable} method. To change
* whether a view can take focus, call {@link #setFocusable(boolean)}.

The property is updated through its shadow node ReactImageManager method setAccessible https://bit.ly/3dDuK5L

 * <p>Instances of this class receive property updates from JS via @{link UIManagerModule}.
 * Subclasses may use {@link #updateShadowNode} to persist some of the updated fields in the node
 * instance that corresponds to a particular view type.

Changelog

[Android] [Fixed] - adding setAccessible to ReactImageManager to allow screenreader announce Image accessibilityState of "disabled"

Test Plan

CLICK TO OPEN TESTS RESULTS

Enable audio to hear the screenreader

TEST SCENARIO

The user moves the screenreader focus to an image and the screenreader reads the Image accessibilityLabel "plain network image"

RESULT

The screenreader announces the accessibilityState disabled after reading the Image accessibilityLabel "plain network image"

<Image
  accessible={true}
  accessibilityLabel="plain network image"
  accessibilityState={{disabled: true}}
  source={fullImage}
  style={styles.base}
/>

2021-03-26.18-27-36.mp4

fabOnReact · 2021-03-26T18:15:46Z

Further Analyse Image does not announce "disabled" #30935 (comment)
Study the difference in the inheritance rules of all the ShadowNodes ViewManagers on Android
write accessibility test with espresso see Quick Start doesn't build buck#112 (comment)

analysis-bot · 2021-03-26T19:10:48Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: da899c0

amarlette · 2021-03-26T22:02:16Z

Thank you for working on this issue @fabriziobertoglio1987! Excited to see your finished PR.

Got you on the project board and will assign a reviewer when you are out of drafts.

lunaleaps · 2021-04-22T22:44:53Z

@fabriziobertoglio1987 ~~is this ready for review?~~ Ah sorry looks like it is, I'll move it to the right column then

fabOnReact · 2021-04-23T14:56:50Z

sorry @lunaleaps, I did set this as ready for review. Currently I'm away from office, but I will be doing opensource again in 1-2 weeks. Thanks a lot. I wish you a good day

kacieb · 2021-04-23T16:57:26Z

cc @blavalla for Android review!

kacieb · 2021-04-23T16:59:34Z

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

@@ -929,4 +932,18 @@ exports.examples = [
      return <EnabledExamples />;
    },
  },
+  {
+    title: 'Check if these properties are enabled',


Nit: It would be good to specify what properties we're testing here - in this case it's image focusability and disabled state.

@kacieb thanks for the code review. I experienced some issues with flipper after the last rebase and I published pr #31468 which includes solution to the runtime in the rn-tester app. I added the changes you required and tested them using the fix #31468. The functionality works like before. Thanks a lot 🙏

blavalla

Android code looks good to me!

lunaleaps · 2021-04-26T22:38:07Z

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

@@ -30,6 +30,9 @@ const checkImageSource = require('./check.png');
 const uncheckImageSource = require('./uncheck.png');
 const mixedCheckboxImageSource = require('./mixed.png');
 const {createRef} = require('react');
+const fullImage = {
+  uri: 'https://www.facebook.com/ads/pics/successstories.png',


Do you think we should reference a local asset here instead of fetching over network? I know there are some assets under js/assets/..

thanks a lot for the code review @lunaleaps. I added the changes required. More info in comment #31252 (comment) 🙏

analysis-bot · 2021-04-27T05:18:16Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,864,454	-122,281
android	hermes	armeabi-v7a	8,386,258	-107,725
android	hermes	x86	9,321,886	-117,136
android	hermes	x86_64	9,264,852	-115,931
android	jsc	arm64-v8a	10,593,478	-11,656
android	jsc	armeabi-v7a	10,098,377	-7,030
android	jsc	x86	10,612,415	-11,729
android	jsc	x86_64	11,195,638	-12,174

Base commit: da899c0

lunaleaps · 2021-05-03T17:20:59Z

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

@@ -48,8 +48,8 @@ const styles = StyleSheet.create({
    borderStyle: 'solid',
  },
  image: {
-    width: 20,
-    height: 20,
+    width: 120,


@fabriziobertoglio1987 Would this break other accessibility examples that use styles.image? Could create a separate styles.disabledImage? Did you want me to add during import? Otherwise I think this looks good!

@lunaleaps thanks a lot for the codereview.. sorry for this. I just updated the code with commit f5e4a16 I remain available 🙏

lunaleaps

Great! Thank you for your help! Importing this now :)

facebook-github-bot · 2021-05-04T17:02:28Z

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

facebook-github-bot · 2021-05-04T21:30:14Z

@lunaleaps merged this pull request in 333b46c.

amarlette · 2021-05-05T01:33:07Z

Thank you @fabriziobertoglio1987 for another great contribution to a more accessible React Native! 😄

I'll highlight you with the same info from your other PR.

fabOnReact · 2021-05-10T09:08:28Z

@lunaleaps thanks a lot for merging this pr. I will look for my next issues in the Accessibility Project 🙏
@amarlette thanks a lot for the mention 🙏

…ality when disabled (#33076) Summary: This issue fixes #30937 fixes #30947 fixes #30840 ([Test Case 7.1][7.1], [Test Case 7.3][7.3], [Test Case 7.5][7.5]) . The issue is caused by: 1) The missing javascript logic on the `accessibilityState` in the Text component fabOnReact@6ab7ab3 (as previously implemented in [Button][20]). 2) The missing setter for prop `accessible` in `ReactTextAnchorViewManager` fabOnReact@17095c6 (More information in previous PR #31252) Related PR #33070 PR callstack/react-native-slider#354 [20]: https://github.com/facebook/react-native/pull/31001/files#diff-4f225d043edf4cf5b8288285b6a957e2187fc0242f240bde396e41c4c25e4124R281-R289 ## Changelog [Android] [Fixed] - Text Component does not announce disabled and disables click functionality when disabled Pull Request resolved: #33076 Test Plan: [1]. Text has `disabled` and `accessibilityState={{disabled: false}}` ([link][1]) [2]. Text has `disabled` ([link][2]) [3]. Text has `accessibilityState={{disabled: true}}` ([link][3]) [4]. Text has `accessibilityState={{disabled:false}}` ([link][4]) [5]. Text has `disabled={false}` and `accessibilityState={{disabled:true}}` ([link][5]) [6]. Text has `accessibilityState={{disabled:true}}` and method `setAccessible` in `ReactTextAnchorViewManager` (tested on commit [b4cd8][10]) ([link][6]) 7. Test Cases on the main branch [7.1]. Text has `disabled` and `accessibilityState={{disabled: false}}` ([link][7.1]) [7.3] Text has `accessibilityState={{disabled: true}}` ([link][7.3]) [7.5] Text has `disabled={false}` and `accessibilityState={{disabled:true}}` ([link][7.5]) [7.6] Text has `onPress callback` and `accessibilityState={{disabled: true}}` ([link][7.6]) [7.7] Text has `accessibilityState={{disabled:true}}` and no method `setAccessible` in `ReactTextAnchorViewManager` (tested on commit [c4f98dd][11]) ([link][7.7]) [1]: fabOnReact/react-native-notes#1 (comment) [2]: fabOnReact/react-native-notes#1 (comment) [3]: fabOnReact/react-native-notes#1 (comment) [4]: fabOnReact/react-native-notes#1 (comment) [5]: fabOnReact/react-native-notes#1 (comment) [6]: fabOnReact/react-native-notes#1 (comment) [7.1]: fabOnReact/react-native-notes#1 (comment) [7.3]: fabOnReact/react-native-notes#1 (comment) [7.5]: fabOnReact/react-native-notes#1 (comment) [7.6]: fabOnReact/react-native-notes#1 (comment) [7.7]: fabOnReact/react-native-notes#1 (comment) [10]: 17095c6 [11]: 6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

…ality when disabled (facebook#33076) Summary: This issue fixes facebook#30937 fixes facebook#30947 fixes facebook#30840 ([Test Case 7.1][7.1], [Test Case 7.3][7.3], [Test Case 7.5][7.5]) . The issue is caused by: 1) The missing javascript logic on the `accessibilityState` in the Text component fabOnReact@6ab7ab3 (as previously implemented in [Button][20]). 2) The missing setter for prop `accessible` in `ReactTextAnchorViewManager` fabOnReact@17095c6 (More information in previous PR facebook#31252) Related PR facebook#33070 PR callstack/react-native-slider#354 [20]: https://github.com/facebook/react-native/pull/31001/files#diff-4f225d043edf4cf5b8288285b6a957e2187fc0242f240bde396e41c4c25e4124R281-R289 [Android] [Fixed] - Text Component does not announce disabled and disables click functionality when disabled Pull Request resolved: facebook#33076 Test Plan: [1]. Text has `disabled` and `accessibilityState={{disabled: false}}` ([link][1]) [2]. Text has `disabled` ([link][2]) [3]. Text has `accessibilityState={{disabled: true}}` ([link][3]) [4]. Text has `accessibilityState={{disabled:false}}` ([link][4]) [5]. Text has `disabled={false}` and `accessibilityState={{disabled:true}}` ([link][5]) [6]. Text has `accessibilityState={{disabled:true}}` and method `setAccessible` in `ReactTextAnchorViewManager` (tested on commit [b4cd8][10]) ([link][6]) 7. Test Cases on the main branch [7.1]. Text has `disabled` and `accessibilityState={{disabled: false}}` ([link][7.1]) [7.3] Text has `accessibilityState={{disabled: true}}` ([link][7.3]) [7.5] Text has `disabled={false}` and `accessibilityState={{disabled:true}}` ([link][7.5]) [7.6] Text has `onPress callback` and `accessibilityState={{disabled: true}}` ([link][7.6]) [7.7] Text has `accessibilityState={{disabled:true}}` and no method `setAccessible` in `ReactTextAnchorViewManager` (tested on commit [c4f98dd][11]) ([link][7.7]) [1]: fabOnReact/react-native-notes#1 (comment) [2]: fabOnReact/react-native-notes#1 (comment) [3]: fabOnReact/react-native-notes#1 (comment) [4]: fabOnReact/react-native-notes#1 (comment) [5]: fabOnReact/react-native-notes#1 (comment) [6]: fabOnReact/react-native-notes#1 (comment) [7.1]: fabOnReact/react-native-notes#1 (comment) [7.3]: fabOnReact/react-native-notes#1 (comment) [7.5]: fabOnReact/react-native-notes#1 (comment) [7.6]: fabOnReact/react-native-notes#1 (comment) [7.7]: fabOnReact/react-native-notes#1 (comment) [10]: facebook@17095c6 [11]: facebook@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

adding accessible prop to ReactImageManager

351817d

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 26, 2021

amarlette added the Accessibility label Mar 26, 2021

fabOnReact added 3 commits March 29, 2021 08:55

AccessibilityExample add image example

1a14145

adding basic Java Unit test

b2988c3

rename Test case

cf665dc

fabOnReact mentioned this pull request Mar 29, 2021

Quick Start doesn't build facebook/buck#112

Closed

Merge branch 'master' into fix/image-accessibility

326e338

fabOnReact marked this pull request as ready for review April 6, 2021 16:33

kacieb assigned lunaleaps Apr 23, 2021

kacieb requested review from kacieb, lunaleaps and blavalla April 23, 2021 16:55

kacieb reviewed Apr 23, 2021

View reviewed changes

blavalla approved these changes Apr 23, 2021

View reviewed changes

lunaleaps reviewed Apr 26, 2021

View reviewed changes

Merge branch 'master' into fix/image-accessibility

5b06ede

fabOnReact added 4 commits May 2, 2021 14:20

update test example title

3e9fc4e

use local instead of network asset

5c16b18

remove example from rn-tester ImageExample

43ba382

rename example accessibilityLabel

3485fa0

lunaleaps reviewed May 3, 2021

View reviewed changes

moving style changes from image to disabledImage

f5e4a16

lunaleaps approved these changes May 4, 2021

View reviewed changes

facebook-github-bot closed this in 333b46c May 4, 2021

facebook-github-bot added the Merged This PR has been merged. label May 4, 2021

This was referenced Feb 10, 2022

ImageBackground does not announce "disabled" #30936

Closed

Text Component does not announce disabled and disables click functionality when disabled #33076

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Accessibility] Fix Image does not announce "disabled" #31252

[Accessibility] Fix Image does not announce "disabled" #31252

fabOnReact commented Mar 26, 2021 •

edited

Loading

fabOnReact commented Mar 26, 2021 •

edited

Loading

analysis-bot commented Mar 26, 2021 •

edited

Loading

amarlette commented Mar 26, 2021 •

edited

Loading

lunaleaps commented Apr 22, 2021 •

edited

Loading

fabOnReact commented Apr 23, 2021

kacieb commented Apr 23, 2021

kacieb Apr 23, 2021

fabOnReact May 2, 2021

blavalla left a comment

lunaleaps Apr 26, 2021 •

edited

Loading

fabOnReact May 2, 2021

analysis-bot commented Apr 27, 2021 •

edited

Loading

lunaleaps May 3, 2021 •

edited

Loading

fabOnReact May 4, 2021

lunaleaps left a comment

facebook-github-bot commented May 4, 2021

facebook-github-bot commented May 4, 2021

amarlette commented May 5, 2021

fabOnReact commented May 10, 2021

[Accessibility] Fix Image does not announce "disabled" #31252

[Accessibility] Fix Image does not announce "disabled" #31252

Conversation

fabOnReact commented Mar 26, 2021 • edited Loading

Summary

Changelog

Test Plan

fabOnReact commented Mar 26, 2021 • edited Loading

analysis-bot commented Mar 26, 2021 • edited Loading

amarlette commented Mar 26, 2021 • edited Loading

lunaleaps commented Apr 22, 2021 • edited Loading

fabOnReact commented Apr 23, 2021

kacieb commented Apr 23, 2021

kacieb Apr 23, 2021

Choose a reason for hiding this comment

fabOnReact May 2, 2021

Choose a reason for hiding this comment

blavalla left a comment

Choose a reason for hiding this comment

lunaleaps Apr 26, 2021 • edited Loading

Choose a reason for hiding this comment

fabOnReact May 2, 2021

Choose a reason for hiding this comment

analysis-bot commented Apr 27, 2021 • edited Loading

lunaleaps May 3, 2021 • edited Loading

Choose a reason for hiding this comment

fabOnReact May 4, 2021

Choose a reason for hiding this comment

lunaleaps left a comment

Choose a reason for hiding this comment

facebook-github-bot commented May 4, 2021

facebook-github-bot commented May 4, 2021

amarlette commented May 5, 2021

fabOnReact commented May 10, 2021

fabOnReact commented Mar 26, 2021 •

edited

Loading

fabOnReact commented Mar 26, 2021 •

edited

Loading

analysis-bot commented Mar 26, 2021 •

edited

Loading

amarlette commented Mar 26, 2021 •

edited

Loading

lunaleaps commented Apr 22, 2021 •

edited

Loading

lunaleaps Apr 26, 2021 •

edited

Loading

analysis-bot commented Apr 27, 2021 •

edited

Loading

lunaleaps May 3, 2021 •

edited

Loading