Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android: Disabled States does not announce or disable function for some components #30840

Closed
13 of 19 tasks
amarlette opened this issue Feb 3, 2021 · 3 comments · Fixed by callstack/react-native-slider#354
Closed
13 of 19 tasks
Labels
Accessibility Team - Evaluated Accessibility Good first issue Interested in collaborating? Take a stab at fixing one of these issues. Platform: Android Android applications. React Native Engineering - Evaluated Resolution: Locked This issue was locked by the bot.

Comments

@amarlette
Copy link

amarlette commented Feb 3, 2021

Description

Does not announce "disabled" on the following components:

Doesn't disable click functionality of the following components:

This behavior is usually due to there being both a "disabled" prop on the component itself, that does not necessarily sync with the accessibilityState's "disabled" field. When "disabled" is set, we should just automatically set accessibilityState: disabled as well so that they stay in sync. (You can see an example of this fix in the linked Pressable issue).

When testing a component, be sure to test both the accessibilityState and the components own disabled prop if one exists, and make sure all combinations of them work as expected. This commit has a great example of snapshot tests added to ensure the component behaves as expected.

Issue #30246 is similar to this issue and should be taken into account when creating a solution.

React Native version:

v0.63

Expected Results

When a component is "disabled", screen readers should announce this as part of their focus announcement. Clicks to these elements should also have no effect.

If a component has both a "disabled" prop and allows "accessibilityState: disabled", the "disabled" prop should be the authority on the state of the component (ignoring accessibilityState: disabled).

Snack

https://snack.expo.io/d1Ikg7nqo

Android details

To disable an element for Talkback, the AccessibilityNodeInfo's enabled property should be set to false and the clickable, longClickable, or focusable attribute should be true (one of these is likely already true if the element was clickable, it's important that it remains that way).

If an AccessibilityAction of AccessibilityNodeInfo.ACTION_CLICK, AccessibilityNodeInfo.ACTION_LONG_CLICK, or AccessibilityAction.ACTION_CONTEXT_CLICK is set on the AccessibilityNodeInfo, it should be removed.

All of the above can be achieved automatically by simply setting the enabled property of the View itself to false, but this means that the components own "disabled" prop (if one exists, such as with ), and the accessibilityState's "disabled" field, must remain in sync.

@amarlette amarlette added the Platform: Android Android applications. label Feb 3, 2021
@blavalla blavalla added Accessibility Team - Evaluated Good first issue Interested in collaborating? Take a stab at fixing one of these issues. labels Feb 12, 2021
@lunaleaps
Copy link
Contributor

@blavalla Is the expectation that accessibilityState: disabled and disabled prop to function the same way? What is the purpose of having both options then? Should we consolidate to one way?

@blavalla
Copy link
Contributor

@lunaleaps, I would expect both properties to have the same experience. As for why they are separate, it's likely because the "disabled" prop also applies some sort of visual indicator of its state (dimming the element, etc.) which is what it is often used for. The accessibilityState: disabled only applies to the accessibility system, so won't do anything visually to the element itself. The accessibilityState: disabled prop is also part of a set of props that are on many different components (here), and disabled seems to be something implemented on a component-by-component basis.

When disabled is set we should probably just automatically set accessibilityState: disabled as well, or alternatively in the mapping of RN props to the native Views, we should consider this prop in the same way we consider accessibilityState: disabled. I can't think of a valid reason for these two properties to ever be out of sync, so we should probably make sure they don't get that way.

@fabOnReact
Copy link
Contributor

@blavalla

I published PRs #33076 #33070 callstack/react-native-slider#354 to fix this issues.

The PRs are in DRAFT. I will complete the PRs in a couple of days.
Thanks 🙏

facebook-github-bot pushed a commit that referenced this issue Feb 16, 2022
Summary:
This issue fixes #30944 fixes #30840 ([Test Case 7.1][7.1], [Test Case 7.3][7.3], [Test Case 7.5][7.5]) which affects Platform Android. Previous PR #31199.
The issue is caused by the missing prop `accessibilityState` in the Switch component.

The solution consists of passing the accessibilityState to the `AndroidSwitchNativeComponent` component as previously implemented in other components (for example, [Button][8]).

Relevant discussions #30840 (comment) and https://github.com/facebook/react-native/pull/31001/files#r578827409.

[8]: https://github.com/facebook/react-native/pull/31001/files#diff-4f225d043edf4cf5b8288285b6a957e2187fc0242f240bde396e41c4c25e4124R281-R289

The solution proposed in this pull request consists of:
1. Passing `accessibilityState` to the `AndroidSwitchNativeComponent`
2. If the value of prop `accessibilityState.disabled` is different from the prop `disabled`, the prop `disabled` over-rides the `accessibilityState.disabled` value.

For example:
```jsx
<Switch disabled={true} accessibilityState={{disabled: false}} />
````
becomes:
````jsx
<Switch disabled={true} accessibilityState={{disabled: true}} />
````

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Switch Component doesn't disable click functionality when disabled

Pull Request resolved: #33070

Test Plan:
[1]. Switch has `disabled` and `accessibilityState={{disabled: false}}`
[2]. Switch has `disabled`
[3]. Switch has `accessibilityState={{disabled: true}}`
[4]. Switch has `accessibilityState={{disabled:false}}`
[5]. Switch has `disabled={false}`  and `accessibilityState={{disabled:true}}`
7. Test Cases on the main branch
[7.1]. Switch has `disabled` and `accessibilityState={{disabled: false}}`
[7.3] Switch has `accessibilityState={{disabled: true}}`
[7.5] Switch has `disabled={false}`  and `accessibilityState={{disabled:true}}`

[1]: fabOnReact/react-native-notes#5 (comment)
[2]: fabOnReact/react-native-notes#5 (comment)
[3]: fabOnReact/react-native-notes#5 (comment)
[4]: fabOnReact/react-native-notes#5 (comment)
[5]: fabOnReact/react-native-notes#5 (comment)
[7.1]: fabOnReact/react-native-notes#5 (comment)
[7.3]: fabOnReact/react-native-notes#5 (comment)
[7.5]: fabOnReact/react-native-notes#5 (comment)

Reviewed By: kacieb

Differential Revision: D34189484

Pulled By: blavalla

fbshipit-source-id: 8ea9221a5641d05c20d0309abdb3f0d02c569f2f
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this issue Jan 15, 2023
…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
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this issue Jan 15, 2023
Summary:
This issue fixes facebook#30944 fixes facebook#30840 ([Test Case 7.1][7.1], [Test Case 7.3][7.3], [Test Case 7.5][7.5]) which affects Platform Android. Previous PR facebook#31199.
The issue is caused by the missing prop `accessibilityState` in the Switch component.

The solution consists of passing the accessibilityState to the `AndroidSwitchNativeComponent` component as previously implemented in other components (for example, [Button][8]).

Relevant discussions facebook#30840 (comment) and https://github.com/facebook/react-native/pull/31001/files#r578827409.

[8]: https://github.com/facebook/react-native/pull/31001/files#diff-4f225d043edf4cf5b8288285b6a957e2187fc0242f240bde396e41c4c25e4124R281-R289

The solution proposed in this pull request consists of:
1. Passing `accessibilityState` to the `AndroidSwitchNativeComponent`
2. If the value of prop `accessibilityState.disabled` is different from the prop `disabled`, the prop `disabled` over-rides the `accessibilityState.disabled` value.

For example:
```jsx
<Switch disabled={true} accessibilityState={{disabled: false}} />
````
becomes:
````jsx
<Switch disabled={true} accessibilityState={{disabled: true}} />
````

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Switch Component doesn't disable click functionality when disabled

Pull Request resolved: facebook#33070

Test Plan:
[1]. Switch has `disabled` and `accessibilityState={{disabled: false}}`
[2]. Switch has `disabled`
[3]. Switch has `accessibilityState={{disabled: true}}`
[4]. Switch has `accessibilityState={{disabled:false}}`
[5]. Switch has `disabled={false}`  and `accessibilityState={{disabled:true}}`
7. Test Cases on the main branch
[7.1]. Switch has `disabled` and `accessibilityState={{disabled: false}}`
[7.3] Switch has `accessibilityState={{disabled: true}}`
[7.5] Switch has `disabled={false}`  and `accessibilityState={{disabled:true}}`

[1]: fabOnReact/react-native-notes#5 (comment)
[2]: fabOnReact/react-native-notes#5 (comment)
[3]: fabOnReact/react-native-notes#5 (comment)
[4]: fabOnReact/react-native-notes#5 (comment)
[5]: fabOnReact/react-native-notes#5 (comment)
[7.1]: fabOnReact/react-native-notes#5 (comment)
[7.3]: fabOnReact/react-native-notes#5 (comment)
[7.5]: fabOnReact/react-native-notes#5 (comment)

Reviewed By: kacieb

Differential Revision: D34189484

Pulled By: blavalla

fbshipit-source-id: 8ea9221a5641d05c20d0309abdb3f0d02c569f2f
@facebook facebook locked as resolved and limited conversation to collaborators Feb 15, 2023
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.