Skip to content

Commit

Permalink
adding importantForAccessibility for Text, Button, ImageBackground (#…
Browse files Browse the repository at this point in the history
…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 #30850 related #33690

## Changelog

[Android] [Fixed] - adding importantForAccessibility for Text, Button, ImageBackground

Pull Request resolved: #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]: #31296 (comment) ""
[2]: https://github.com/grgr-dkrk "grgr-dkrk"
[3]: #31296 "#31296"
[4]: #31296 (comment) "expected behaviour with prop importantForAccessibility in Button component"
[5]: #30850 (comment) "importantForAccessibility=no does not allow screenreader focus on nested Text Components with accessibilityRole=link or inline Images"
[6]: #34245 (comment) "testing ImageBackground importantForAccessiblity"
[7]: #34245 (comment) "Button with importantForAccessibility=no"

Reviewed By: cipolleschi

Differential Revision: D38121992

Pulled By: dmitryrykun

fbshipit-source-id: 368b4dcb47d7940274820aa2e39ed5e2ca068821
  • Loading branch information
fabOnReact authored and facebook-github-bot committed Aug 10, 2022
1 parent 03de197 commit 62021eb
Show file tree
Hide file tree
Showing 7 changed files with 400 additions and 1 deletion.
13 changes: 13 additions & 0 deletions Libraries/Components/Button.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ type ButtonProps = $ReadOnly<{|
accessibilityActions?: ?$ReadOnlyArray<AccessibilityActionInfo>,
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,
|}>;
Expand Down Expand Up @@ -264,6 +269,7 @@ class Button extends React.Component<ButtonProps> {
render(): React.Node {
const {
accessibilityLabel,
importantForAccessibility,
color,
onPress,
touchSoundDisabled,
Expand Down Expand Up @@ -315,6 +321,12 @@ class Button extends React.Component<ButtonProps> {
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 (
<Touchable
accessible={accessible}
Expand All @@ -325,6 +337,7 @@ class Button extends React.Component<ButtonProps> {
accessibilityLanguage={accessibilityLanguage}
accessibilityRole="button"
accessibilityState={accessibilityState}
importantForAccessibility={_importantForAccessibility}
hasTVPreferredFocus={hasTVPreferredFocus}
nextFocusDown={nextFocusDown}
nextFocusForward={nextFocusForward}
Expand Down
10 changes: 10 additions & 0 deletions Libraries/Components/__tests__/Button-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,14 @@ describe('<Button />', () => {
expect(ReactTestRenderer.create( <Button title="Test Button" disabled={false} accessibilityState={{disabled: false}} />)
).toMatchSnapshot();
});

it('should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no-hide-descendants}', () => {
expect(ReactTestRenderer.create( <Button title="Test Button" importantForAccessibility={'no-hide-descendants'} />)
).toMatchSnapshot();
});

it('should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no}', () => {
expect(ReactTestRenderer.create( <Button title="Test Button" importantForAccessibility={'no'} />)
).toMatchSnapshot();
});
});
90 changes: 90 additions & 0 deletions Libraries/Components/__tests__/__snapshots__/Button-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,96 @@ exports[`<Button /> should be disabled when disabled={true} and accessibilitySta
</View>
`;

exports[`<Button /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no} 1`] = `
<View
accessibilityRole="button"
accessible={true}
collapsable={false}
focusable={false}
importantForAccessibility="no-hide-descendants"
onClick={[Function]}
onResponderGrant={[Function]}
onResponderMove={[Function]}
onResponderRelease={[Function]}
onResponderTerminate={[Function]}
onResponderTerminationRequest={[Function]}
onStartShouldSetResponder={[Function]}
style={
Object {
"opacity": 1,
}
}
>
<View
style={
Array [
Object {},
]
}
>
<Text
style={
Array [
Object {
"color": "#007AFF",
"fontSize": 18,
"margin": 8,
"textAlign": "center",
},
]
}
>
Test Button
</Text>
</View>
</View>
`;

exports[`<Button /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no-hide-descendants} 1`] = `
<View
accessibilityRole="button"
accessible={true}
collapsable={false}
focusable={false}
importantForAccessibility="no-hide-descendants"
onClick={[Function]}
onResponderGrant={[Function]}
onResponderMove={[Function]}
onResponderRelease={[Function]}
onResponderTerminate={[Function]}
onResponderTerminationRequest={[Function]}
onStartShouldSetResponder={[Function]}
style={
Object {
"opacity": 1,
}
}
>
<View
style={
Array [
Object {},
]
}
>
<Text
style={
Array [
Object {
"color": "#007AFF",
"fontSize": 18,
"margin": 8,
"textAlign": "center",
},
]
}
>
Test Button
</Text>
</View>
</View>
`;

exports[`<Button /> should not be disabled when disabled={false} and accessibilityState={{disabled: false}} 1`] = `
<View
accessibilityRole="button"
Expand Down
12 changes: 11 additions & 1 deletion Libraries/Image/ImageBackground.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,25 @@ class ImageBackground extends React.Component<ImageBackgroundProps> {
};

render(): React.Node {
const {children, style, imageStyle, imageRef, ...props} = this.props;
const {
children,
style,
imageStyle,
imageRef,
importantForAccessibility,
...props
} = this.props;

const flattenedStyle = flattenStyle(style);
return (
<View
accessibilityIgnoresInvertColors={true}
importantForAccessibility={importantForAccessibility}
style={style}
ref={this._captureRef}>
<Image
{...props}
importantForAccessibility={importantForAccessibility}
style={[
StyleSheet.absoluteFill,
{
Expand Down
73 changes: 73 additions & 0 deletions Libraries/Image/__tests__/ImageBackground-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
* @emails oncall+react_native
* @flow strict-local
*/

'use strict';

const React = require('react');
const ImageBackground = require('../ImageBackground');
const render = require('../../../jest/renderer');

describe('<ImageBackground />', () => {
it('should render as <ImageBackground> when mocked', () => {
const instance = render.create(
<ImageBackground
style={{width: 150, height: 50}}
source={{uri: 'foo-bar.jpg'}}
/>,
);
expect(instance).toMatchSnapshot();
});

it('should shallow render as <ImageBackground> when mocked', () => {
const output = render.shallow(
<ImageBackground
style={{width: 150, height: 50}}
source={{uri: 'foo-bar.jpg'}}
/>,
);
expect(output).toMatchSnapshot();
});

it('should shallow render as <ForwardRef(ImageBackground)> when not mocked', () => {
jest.dontMock('../ImageBackground');

const output = render.shallow(
<ImageBackground
style={{width: 150, height: 50}}
source={{uri: 'foo-bar.jpg'}}
/>,
);
expect(output).toMatchSnapshot();
});

it('should render as <RCTImageView> when not mocked', () => {
jest.dontMock('../ImageBackground');

const instance = render.create(
<ImageBackground
style={{width: 150, height: 50}}
source={{uri: 'foo-bar.jpg'}}
/>,
);
expect(instance).toMatchSnapshot();
});

it('should be set importantForAccessibility in <View> and <Image>', () => {
const instance = render.create(
<ImageBackground
importantForAccessibility={'no'}
style={{width: 150, height: 50}}
source={{uri: 'foo-bar.jpg'}}
/>,
);
expect(instance).toMatchSnapshot();
});
});
143 changes: 143 additions & 0 deletions Libraries/Image/__tests__/__snapshots__/ImageBackground-test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<ImageBackground /> should be set importantForAccessibility in <View> and <Image> 1`] = `
<View
accessibilityIgnoresInvertColors={true}
importantForAccessibility="no"
style={
Object {
"height": 50,
"width": 150,
}
}
>
<Image
importantForAccessibility="no"
source={
Object {
"uri": "foo-bar.jpg",
}
}
style={
Array [
Object {
"bottom": 0,
"left": 0,
"position": "absolute",
"right": 0,
"top": 0,
},
Object {
"height": 50,
"width": 150,
},
undefined,
]
}
/>
</View>
`;
exports[`<ImageBackground /> should render as <ImageBackground> when mocked 1`] = `
<View
accessibilityIgnoresInvertColors={true}
style={
Object {
"height": 50,
"width": 150,
}
}
>
<Image
source={
Object {
"uri": "foo-bar.jpg",
}
}
style={
Array [
Object {
"bottom": 0,
"left": 0,
"position": "absolute",
"right": 0,
"top": 0,
},
Object {
"height": 50,
"width": 150,
},
undefined,
]
}
/>
</View>
`;
exports[`<ImageBackground /> should render as <RCTImageView> when not mocked 1`] = `
<View
accessibilityIgnoresInvertColors={true}
style={
Object {
"height": 50,
"width": 150,
}
}
>
<Image
source={
Object {
"uri": "foo-bar.jpg",
}
}
style={
Array [
Object {
"bottom": 0,
"left": 0,
"position": "absolute",
"right": 0,
"top": 0,
},
Object {
"height": 50,
"width": 150,
},
undefined,
]
}
/>
</View>
`;
exports[`<ImageBackground /> should shallow render as <ForwardRef(ImageBackground)> when not mocked 1`] = `
<ImageBackground
source={
Object {
"uri": "foo-bar.jpg",
}
}
style={
Object {
"height": 50,
"width": 150,
}
}
/>
`;
exports[`<ImageBackground /> should shallow render as <ImageBackground> when mocked 1`] = `
<ImageBackground
source={
Object {
"uri": "foo-bar.jpg",
}
}
style={
Object {
"height": 50,
"width": 150,
}
}
/>
`;
Loading

0 comments on commit 62021eb

Please sign in to comment.