Added support for importantForAccessibility for some components #31296
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,201 @@ | ||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
|
||
| exports[`<Picker /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no}: should deep render when mocked (please verify output manually) 1`] = ` | ||
| <View> | ||
| <RCTPicker | ||
|
There was a problem hiding this comment. I don't see anything about the There was a problem hiding this comment. @kacieb |
||
| items={ | ||
| Array [ | ||
| Object { | ||
| "label": "foo", | ||
| "textColor": undefined, | ||
| "value": "foo", | ||
| }, | ||
| Object { | ||
| "label": "bar", | ||
| "textColor": undefined, | ||
| "value": "bar", | ||
| }, | ||
| ] | ||
| } | ||
| onChange={[Function]} | ||
| selectedIndex={0} | ||
| style={ | ||
| Array [ | ||
| Object { | ||
| "height": 216, | ||
| }, | ||
| undefined, | ||
| ] | ||
| } | ||
| /> | ||
| </View> | ||
| `; | ||
|
|
||
| exports[`<Picker /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no}: should deep render when not mocked (please verify output manually) 1`] = ` | ||
| <View> | ||
| <RCTPicker | ||
| items={ | ||
| Array [ | ||
| Object { | ||
| "label": "foo", | ||
| "textColor": undefined, | ||
| "value": "foo", | ||
| }, | ||
| Object { | ||
| "label": "bar", | ||
| "textColor": undefined, | ||
| "value": "bar", | ||
| }, | ||
| ] | ||
| } | ||
| onChange={[Function]} | ||
| selectedIndex={0} | ||
| style={ | ||
| Array [ | ||
| Object { | ||
| "height": 216, | ||
| }, | ||
| undefined, | ||
| ] | ||
| } | ||
| /> | ||
| </View> | ||
| `; | ||
|
|
||
| exports[`<Picker /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no}: should shallow render as <Picker /> when mocked 1`] = ` | ||
| <Picker | ||
| importantForAccessibility="no" | ||
| mode="dialog" | ||
| onValueChange={[MockFunction]} | ||
| selectedValue="foo" | ||
| > | ||
| <PickerItem | ||
| label="foo" | ||
| value="foo" | ||
| /> | ||
| <PickerItem | ||
| label="bar" | ||
| value="bar" | ||
| /> | ||
| </Picker> | ||
| `; | ||
|
|
||
| exports[`<Picker /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no}: should shallow render as <Picker /> when not mocked 1`] = ` | ||
| <Picker | ||
| importantForAccessibility="no" | ||
| mode="dialog" | ||
| onValueChange={[MockFunction]} | ||
| selectedValue="foo" | ||
| > | ||
| <PickerItem | ||
| label="foo" | ||
| value="foo" | ||
| /> | ||
| <PickerItem | ||
| label="bar" | ||
| value="bar" | ||
| /> | ||
| </Picker> | ||
| `; | ||
|
|
||
| exports[`<Picker /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no-hide-descendants}: should deep render when mocked (please verify output manually) 1`] = ` | ||
| <View> | ||
| <RCTPicker | ||
| items={ | ||
| Array [ | ||
| Object { | ||
| "label": "foo", | ||
| "textColor": undefined, | ||
| "value": "foo", | ||
| }, | ||
| Object { | ||
| "label": "bar", | ||
| "textColor": undefined, | ||
| "value": "bar", | ||
| }, | ||
| ] | ||
| } | ||
| onChange={[Function]} | ||
| selectedIndex={0} | ||
| style={ | ||
| Array [ | ||
| Object { | ||
| "height": 216, | ||
| }, | ||
| undefined, | ||
| ] | ||
| } | ||
| /> | ||
| </View> | ||
| `; | ||
|
|
||
| exports[`<Picker /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no-hide-descendants}: should deep render when not mocked (please verify output manually) 1`] = ` | ||
| <View> | ||
| <RCTPicker | ||
| items={ | ||
| Array [ | ||
| Object { | ||
| "label": "foo", | ||
| "textColor": undefined, | ||
| "value": "foo", | ||
| }, | ||
| Object { | ||
| "label": "bar", | ||
| "textColor": undefined, | ||
| "value": "bar", | ||
| }, | ||
| ] | ||
| } | ||
| onChange={[Function]} | ||
| selectedIndex={0} | ||
| style={ | ||
| Array [ | ||
| Object { | ||
| "height": 216, | ||
| }, | ||
| undefined, | ||
| ] | ||
| } | ||
| /> | ||
| </View> | ||
| `; | ||
|
|
||
| exports[`<Picker /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no-hide-descendants}: should shallow render as <Picker /> when mocked 1`] = ` | ||
| <Picker | ||
| importantForAccessibility="no-hide-descendants" | ||
| mode="dialog" | ||
| onValueChange={[MockFunction]} | ||
| selectedValue="foo" | ||
| > | ||
| <PickerItem | ||
| label="foo" | ||
| value="foo" | ||
| /> | ||
| <PickerItem | ||
| label="bar" | ||
| value="bar" | ||
| /> | ||
| </Picker> | ||
| `; | ||
|
|
||
| exports[`<Picker /> should be set importantForAccessibility={no-hide-descendants} when importantForAccessibility={no-hide-descendants}: should shallow render as <Picker /> when not mocked 1`] = ` | ||
| <Picker | ||
| importantForAccessibility="no-hide-descendants" | ||
| mode="dialog" | ||
| onValueChange={[MockFunction]} | ||
| selectedValue="foo" | ||
| > | ||
| <PickerItem | ||
| label="foo" | ||
| value="foo" | ||
| /> | ||
| <PickerItem | ||
| label="bar" | ||
| value="bar" | ||
| /> | ||
| </Picker> | ||
| `; | ||
|
|
||
| exports[`<Picker /> should render as expected: should deep render when mocked (please verify output manually) 1`] = ` | ||
| <View> | ||
| <RCTPicker | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! I just wanted to ask why this is needed? What happens if someone specifies "no" without this code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. If set to
no, the Picker will not focus, but it will focus on the TextView inside that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @grgr-dkrk !
@kacieb , this is due to the fact that while some components like
<Button>seem like atomic units, they actually end up rendering a hierarchy of components for example a wrapper<View>with a<Text>inside them.By allowing those descendants to become focusable, it sort of breaks the model of these being a single component to consider, so I think forcing no-hide-descendants makes sense here.
The other option is to always render any descendants of these elements with importantForAccessibility="no", so that they can never be focusable on their own. This would have the same end 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).