From 06fba4cea260c8aac9dcf2dfa4e318171fdc1488 Mon Sep 17 00:00:00 2001 From: fabriziobertoglio1987 Date: Wed, 10 Aug 2022 05:30:28 -0700 Subject: [PATCH] adding importantForAccessibility for Text, Button, ImageBackground (#34245) Summary: Previously published by [grgr-dkrk][2] as [https://github.com/facebook/react-native/issues/31296][3], fixes the following issues: 1) ImportantForAccessibility="no" does not work on Text, Button 2) ImportantForAccessibility="no-hide-descendants" does not work on Text, Button, or ImageBackground. Note: On ImageBackground, focus is prevented on the imageBackground node itself, but focus is not prevented on its descendants. Note: [Button component expected behavior for prop importantForAccessibility][4] >Some components like Button seem like atomic units, but they end up rendering a hierarchy of components for example a wrapper View with a Text inside them. Allowing those descendants to become focusable, breaks the model of these being a single component to consider and forcing no-hide-descendants makes sense here. >The other option is always to render any descendants of these elements with importantForAccessibility="no", so they can never be focusable on their own. This would have the same result, **BUT may potentially cause issues when the descendant content is supposed to automatically get moved up to the focusable ancestor to act as a label** (which is what Talkback does with unfocusable text elements by default). Note: [importantForAccessibility="no" does not allow screenreader focus on nested Text Components with accessibilityRole="link" or inline Images][5] fixes https://github.com/facebook/react-native/issues/30850 related https://github.com/facebook/react-native/pull/33690 ## Changelog [Android] [Fixed] - adding importantForAccessibility for Text, Button, ImageBackground Pull Request resolved: https://github.com/facebook/react-native/pull/34245 Test Plan: 1) testing ImageBackground importantForAccessiblity ([link to test][1]) 2) importantForAccessibility="no" does not allow screenreader focus on nested Text Components with accessibilityRole="link" or inline Images ([link to test][5]) 3) testing ImageBackground importantForAccessiblity ([link to test][6]) 4) Button with importantForAccessibility=no ([link to test][7]) [1]: https://github.com/facebook/react-native/pull/31296#issuecomment-1192341626 "" [2]: https://github.com/grgr-dkrk "grgr-dkrk" [3]: https://github.com/facebook/react-native/pull/31296 "https://github.com/facebook/react-native/issues/31296" [4]: https://github.com/facebook/react-native/pull/31296#discussion_r616184584 "expected behaviour with prop importantForAccessibility in Button component" [5]: https://github.com/facebook/react-native/issues/30850#issuecomment-1192286477 "importantForAccessibility=no does not allow screenreader focus on nested Text Components with accessibilityRole=link or inline Images" [6]: https://github.com/facebook/react-native/pull/34245#issuecomment-1192446124 "testing ImageBackground importantForAccessiblity" [7]: https://github.com/facebook/react-native/pull/34245#issuecomment-1192443589 "Button with importantForAccessibility=no" Reviewed By: cipolleschi Differential Revision: D38121992 Pulled By: dmitryrykun fbshipit-source-id: 368b4dcb47d7940274820aa2e39ed5e2ca068821 --- Libraries/Components/Button.js | 13 ++ Libraries/Components/__tests__/Button-test.js | 10 ++ .../__snapshots__/Button-test.js.snap | 90 +++++++++++ Libraries/Image/ImageBackground.js | 12 +- .../Image/__tests__/ImageBackground-test.js | 73 +++++++++ .../ImageBackground-test.js.snap | 143 ++++++++++++++++++ .../Accessibility/AccessibilityExample.js | 60 ++++++++ 7 files changed, 400 insertions(+), 1 deletion(-) create mode 100644 Libraries/Image/__tests__/ImageBackground-test.js create mode 100644 Libraries/Image/__tests__/__snapshots__/ImageBackground-test.js.snap diff --git a/Libraries/Components/Button.js b/Libraries/Components/Button.js index 89ec384fe2c1b7..318e82773d1678 100644 --- a/Libraries/Components/Button.js +++ b/Libraries/Components/Button.js @@ -145,6 +145,11 @@ type ButtonProps = $ReadOnly<{| accessibilityActions?: ?$ReadOnlyArray, onAccessibilityAction?: ?(event: AccessibilityActionEvent) => mixed, accessibilityState?: ?AccessibilityState, + + /** + * [Android] Controlling if a view fires accessibility events and if it is reported to accessibility services. + */ + importantForAccessibility?: ?('auto' | 'yes' | 'no' | 'no-hide-descendants'), accessibilityHint?: ?string, accessibilityLanguage?: ?Stringish, |}>; @@ -264,6 +269,7 @@ class Button extends React.Component { render(): React.Node { const { accessibilityLabel, + importantForAccessibility, color, onPress, touchSoundDisabled, @@ -315,6 +321,12 @@ class Button extends React.Component { const Touchable = Platform.OS === 'android' ? TouchableNativeFeedback : TouchableOpacity; + // If `no` is specified for `importantForAccessibility`, it will be changed to `no-hide-descendants` because the text inside should not be focused. + const _importantForAccessibility = + importantForAccessibility === 'no' + ? 'no-hide-descendants' + : importantForAccessibility; + return ( { accessibilityLanguage={accessibilityLanguage} accessibilityRole="button" accessibilityState={accessibilityState} + importantForAccessibility={_importantForAccessibility} hasTVPreferredFocus={hasTVPreferredFocus} nextFocusDown={nextFocusDown} nextFocusForward={nextFocusForward} diff --git a/Libraries/Components/__tests__/Button-test.js b/Libraries/Components/__tests__/Button-test.js index d2f976371aa9d8..3c370b4e36b000 100644 --- a/Libraries/Components/__tests__/Button-test.js +++ b/Libraries/Components/__tests__/Button-test.js @@ -46,4 +46,14 @@ describe('