Fix: Conflicting states for accessibility and non-accessibility users by fabOnReact · Pull Request #354 · callstack/react-native-slider

fabOnReact · 2022-02-01T02:14:27Z

Summary:

This issue fixesreact/react-native#30939fixesreact/react-native#30840 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by the missing prop accessibilityState in the Slider component.

The solution consists of passing the accessibilityState to the RCTSliderNativeComponent component as previously implemented in other components (for example, Button).

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

The solution proposed in this pull request consists of:

removing the disabled default prop (related discussions fabOnReact@fc9f0c4 and fabOnReact/react-native@afb7fc2).
If the value of prop accessibilityState.disabled is different from the prop disabled, the prop disabled over-rides the accessibilityState.disabled value.

For example:

<Sliderdisabled={true}accessibilityState={{disabled: false}}/>

becomes:

<Sliderdisabled={true}accessibilityState={{disabled: true}}/>

Remove the existing Android java logic

An alternative Solution published in this branch would be:

not removing the default prop for disabled (false).
disabled default prop false over-rides accessibilityState.disabled

This alternative solution would not respect the requirement of not having conflicting states for accessibility and non-accessibility users and the approach adopted in react-native Components (for ex. Button).

For example:

<SlideraccessibilityState={{disabled: true}}/>

would become (as the deafult is disabled={false})

<SlideraccessibilityState={{disabled: false}}/>

ADDITIONAL NOTES

<Slider disabled={"true"} /> triggers a wrong type warning with typescript.
<Slider disabled={"true"} /> does not trigger a wrong type warning with flow.js.

Test Plan:

Android:

1. Slider has disabled and accessibilityState={{disabled: false}}
2. Slider has disabled
3. Slider has accessibilityState={{disabled: true}}
4. Slider has accessibilityState={{disabled:false}}
5. Slider has disabled={false} and accessibilityState={{disabled:true}}
7. Test Cases on the main branch
7.1. Slider has disabled and accessibilityState={{disabled: false}}
7.3 Slider has accessibilityState={{disabled: true}}
7.5 Slider has disabled={false} and accessibilityState={{disabled:true}}
7.6 Wrong prop type disabled="not a boolean value" with typescript
7.7 Wrong prop type disabled="not a boolean value" with flow on the master branch

Flow check on master seems to not work fabOnReact/react-native-notes#2 (comment) As an alternative I implemented an invariant check. https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245 https://github.com/zertosh/invariant

I will try to fix .flowconfig and enable flow type checking for Slider props

…essibility disabled state

fabOnReact · 2022-02-01T02:29:37Z

TO-DO

Test Android
Test iOS

BartoszKlonowski · 2022-02-04T10:47:13Z

Thank you, @fabriziobertoglio1987! 👍
Looking forward to have this improvement - can you let me know if you plan to continue (making it ready to be reviewed)?

Also: Let me know if you need help with testing it on iOS (of course it's all Java implementation, but we will need to check if changing the JS layer affected other platforms).

fabOnReact · 2022-02-04T22:23:37Z

Thanks a lot @BartoszKlonowski
I plan to continue. I will test iOS and then move this pull request to Ready to Review.

Thanks a lot
I wish you a good weekend 🙏

BartoszKlonowski

Good work in general, thank you! 👍
The only remark I have is to create separate examples and it should be ready to deliver.

There's already a test, where disabled={true} is checked against accessibilityState={{disabled: false}}, but to double check all the combinations this commit adds new test that acts as the inverted scenario to make sure that conflicts are avoided both directions.

…ality when disabled (#33076) Summary: This issue fixes#30937fixes#30947fixes#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

It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski · 2022-02-28T17:17:33Z

Thank you, @fabriziobertoglio1987!

…ality when disabled (react#33076) Summary: This issue fixesreact#30937fixesreact#30947fixesreact#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 react#31252) Related PR react#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: react#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]: react@17095c6 [11]: react@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

fabOnReact · 2022-02-01T02:14:27Z

Summary:

This issue fixesreact/react-native#30939fixesreact/react-native#30840 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by the missing prop accessibilityState in the Slider component.

The solution consists of passing the accessibilityState to the RCTSliderNativeComponent component as previously implemented in other components (for example, Button).

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

The solution proposed in this pull request consists of:

removing the disabled default prop (related discussions fabOnReact@fc9f0c4 and fabOnReact/react-native@afb7fc2).
If the value of prop accessibilityState.disabled is different from the prop disabled, the prop disabled over-rides the accessibilityState.disabled value.

For example:

<Sliderdisabled={true}accessibilityState={{disabled: false}}/>

becomes:

<Sliderdisabled={true}accessibilityState={{disabled: true}}/>

Remove the existing Android java logic

An alternative Solution published in this branch would be:

not removing the default prop for disabled (false).
disabled default prop false over-rides accessibilityState.disabled

This alternative solution would not respect the requirement of not having conflicting states for accessibility and non-accessibility users and the approach adopted in react-native Components (for ex. Button).

For example:

<SlideraccessibilityState={{disabled: true}}/>

would become (as the deafult is disabled={false})

<SlideraccessibilityState={{disabled: false}}/>

ADDITIONAL NOTES

<Slider disabled={"true"} /> triggers a wrong type warning with typescript.
<Slider disabled={"true"} /> does not trigger a wrong type warning with flow.js.

Test Plan:

Android:

1. Slider has disabled and accessibilityState={{disabled: false}}
2. Slider has disabled
3. Slider has accessibilityState={{disabled: true}}
4. Slider has accessibilityState={{disabled:false}}
5. Slider has disabled={false} and accessibilityState={{disabled:true}}
7. Test Cases on the main branch
7.1. Slider has disabled and accessibilityState={{disabled: false}}
7.3 Slider has accessibilityState={{disabled: true}}
7.5 Slider has disabled={false} and accessibilityState={{disabled:true}}
7.6 Wrong prop type disabled="not a boolean value" with typescript
7.7 Wrong prop type disabled="not a boolean value" with flow on the master branch

Flow check on master seems to not work fabOnReact/react-native-notes#2 (comment) As an alternative I implemented an invariant check. https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245 https://github.com/zertosh/invariant

I will try to fix .flowconfig and enable flow type checking for Slider props

…essibility disabled state

fabOnReact · 2022-02-01T02:29:37Z

TO-DO

Test Android
Test iOS

BartoszKlonowski · 2022-02-04T10:47:13Z

Thank you, @fabriziobertoglio1987! 👍
Looking forward to have this improvement - can you let me know if you plan to continue (making it ready to be reviewed)?

Also: Let me know if you need help with testing it on iOS (of course it's all Java implementation, but we will need to check if changing the JS layer affected other platforms).

fabOnReact · 2022-02-04T22:23:37Z

Thanks a lot @BartoszKlonowski
I plan to continue. I will test iOS and then move this pull request to Ready to Review.

Thanks a lot
I wish you a good weekend 🙏

BartoszKlonowski

Good work in general, thank you! 👍
The only remark I have is to create separate examples and it should be ready to deliver.

There's already a test, where disabled={true} is checked against accessibilityState={{disabled: false}}, but to double check all the combinations this commit adds new test that acts as the inverted scenario to make sure that conflicts are avoided both directions.

…ality when disabled (#33076) Summary: This issue fixes#30937fixes#30947fixes#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

It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski · 2022-02-28T17:17:33Z

Thank you, @fabriziobertoglio1987!

…ality when disabled (react#33076) Summary: This issue fixesreact#30937fixesreact#30947fixesreact#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 react#31252) Related PR react#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: react#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]: react@17095c6 [11]: react@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

fabOnReact · 2022-02-01T02:14:27Z

Summary:

This issue fixesreact/react-native#30939fixesreact/react-native#30840 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by the missing prop accessibilityState in the Slider component.

The solution consists of passing the accessibilityState to the RCTSliderNativeComponent component as previously implemented in other components (for example, Button).

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

The solution proposed in this pull request consists of:

removing the disabled default prop (related discussions fabOnReact@fc9f0c4 and fabOnReact/react-native@afb7fc2).
If the value of prop accessibilityState.disabled is different from the prop disabled, the prop disabled over-rides the accessibilityState.disabled value.

For example:

<Sliderdisabled={true}accessibilityState={{disabled: false}}/>

becomes:

<Sliderdisabled={true}accessibilityState={{disabled: true}}/>

Remove the existing Android java logic

An alternative Solution published in this branch would be:

not removing the default prop for disabled (false).
disabled default prop false over-rides accessibilityState.disabled

This alternative solution would not respect the requirement of not having conflicting states for accessibility and non-accessibility users and the approach adopted in react-native Components (for ex. Button).

For example:

<SlideraccessibilityState={{disabled: true}}/>

would become (as the deafult is disabled={false})

<SlideraccessibilityState={{disabled: false}}/>

ADDITIONAL NOTES

<Slider disabled={"true"} /> triggers a wrong type warning with typescript.
<Slider disabled={"true"} /> does not trigger a wrong type warning with flow.js.

Test Plan:

Android:

1. Slider has disabled and accessibilityState={{disabled: false}}
2. Slider has disabled
3. Slider has accessibilityState={{disabled: true}}
4. Slider has accessibilityState={{disabled:false}}
5. Slider has disabled={false} and accessibilityState={{disabled:true}}
7. Test Cases on the main branch
7.1. Slider has disabled and accessibilityState={{disabled: false}}
7.3 Slider has accessibilityState={{disabled: true}}
7.5 Slider has disabled={false} and accessibilityState={{disabled:true}}
7.6 Wrong prop type disabled="not a boolean value" with typescript
7.7 Wrong prop type disabled="not a boolean value" with flow on the master branch

Flow check on master seems to not work fabOnReact/react-native-notes#2 (comment) As an alternative I implemented an invariant check. https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245 https://github.com/zertosh/invariant

I will try to fix .flowconfig and enable flow type checking for Slider props

…essibility disabled state

fabOnReact · 2022-02-01T02:29:37Z

TO-DO

Test Android
Test iOS

BartoszKlonowski · 2022-02-04T10:47:13Z

Thank you, @fabriziobertoglio1987! 👍
Looking forward to have this improvement - can you let me know if you plan to continue (making it ready to be reviewed)?

Also: Let me know if you need help with testing it on iOS (of course it's all Java implementation, but we will need to check if changing the JS layer affected other platforms).

fabOnReact · 2022-02-04T22:23:37Z

Thanks a lot @BartoszKlonowski
I plan to continue. I will test iOS and then move this pull request to Ready to Review.

Thanks a lot
I wish you a good weekend 🙏

BartoszKlonowski

Good work in general, thank you! 👍
The only remark I have is to create separate examples and it should be ready to deliver.

There's already a test, where disabled={true} is checked against accessibilityState={{disabled: false}}, but to double check all the combinations this commit adds new test that acts as the inverted scenario to make sure that conflicts are avoided both directions.

…ality when disabled (#33076) Summary: This issue fixes#30937fixes#30947fixes#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

It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski · 2022-02-28T17:17:33Z

Thank you, @fabriziobertoglio1987!

…ality when disabled (react#33076) Summary: This issue fixesreact#30937fixesreact#30947fixesreact#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 react#31252) Related PR react#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: react#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]: react@17095c6 [11]: react@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

fabOnReact · 2022-02-01T02:14:27Z

Summary:

This issue fixesreact/react-native#30939fixesreact/react-native#30840 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by the missing prop accessibilityState in the Slider component.

The solution consists of passing the accessibilityState to the RCTSliderNativeComponent component as previously implemented in other components (for example, Button).

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

The solution proposed in this pull request consists of:

removing the disabled default prop (related discussions fabOnReact@fc9f0c4 and fabOnReact/react-native@afb7fc2).
If the value of prop accessibilityState.disabled is different from the prop disabled, the prop disabled over-rides the accessibilityState.disabled value.

For example:

<Sliderdisabled={true}accessibilityState={{disabled: false}}/>

becomes:

<Sliderdisabled={true}accessibilityState={{disabled: true}}/>

Remove the existing Android java logic

An alternative Solution published in this branch would be:

not removing the default prop for disabled (false).
disabled default prop false over-rides accessibilityState.disabled

This alternative solution would not respect the requirement of not having conflicting states for accessibility and non-accessibility users and the approach adopted in react-native Components (for ex. Button).

For example:

<SlideraccessibilityState={{disabled: true}}/>

would become (as the deafult is disabled={false})

<SlideraccessibilityState={{disabled: false}}/>

ADDITIONAL NOTES

<Slider disabled={"true"} /> triggers a wrong type warning with typescript.
<Slider disabled={"true"} /> does not trigger a wrong type warning with flow.js.

Test Plan:

Android:

1. Slider has disabled and accessibilityState={{disabled: false}}
2. Slider has disabled
3. Slider has accessibilityState={{disabled: true}}
4. Slider has accessibilityState={{disabled:false}}
5. Slider has disabled={false} and accessibilityState={{disabled:true}}
7. Test Cases on the main branch
7.1. Slider has disabled and accessibilityState={{disabled: false}}
7.3 Slider has accessibilityState={{disabled: true}}
7.5 Slider has disabled={false} and accessibilityState={{disabled:true}}
7.6 Wrong prop type disabled="not a boolean value" with typescript
7.7 Wrong prop type disabled="not a boolean value" with flow on the master branch

Flow check on master seems to not work fabOnReact/react-native-notes#2 (comment) As an alternative I implemented an invariant check. https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245 https://github.com/zertosh/invariant

I will try to fix .flowconfig and enable flow type checking for Slider props

…essibility disabled state

fabOnReact · 2022-02-01T02:29:37Z

TO-DO

Test Android
Test iOS

BartoszKlonowski · 2022-02-04T10:47:13Z

Thank you, @fabriziobertoglio1987! 👍
Looking forward to have this improvement - can you let me know if you plan to continue (making it ready to be reviewed)?

Also: Let me know if you need help with testing it on iOS (of course it's all Java implementation, but we will need to check if changing the JS layer affected other platforms).

fabOnReact · 2022-02-04T22:23:37Z

Thanks a lot @BartoszKlonowski
I plan to continue. I will test iOS and then move this pull request to Ready to Review.

Thanks a lot
I wish you a good weekend 🙏

BartoszKlonowski

Good work in general, thank you! 👍
The only remark I have is to create separate examples and it should be ready to deliver.

There's already a test, where disabled={true} is checked against accessibilityState={{disabled: false}}, but to double check all the combinations this commit adds new test that acts as the inverted scenario to make sure that conflicts are avoided both directions.

…ality when disabled (#33076) Summary: This issue fixes#30937fixes#30947fixes#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

It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski · 2022-02-28T17:17:33Z

Thank you, @fabriziobertoglio1987!

…ality when disabled (react#33076) Summary: This issue fixesreact#30937fixesreact#30947fixesreact#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 react#31252) Related PR react#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: react#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]: react@17095c6 [11]: react@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

fabOnReact · 2022-02-01T02:14:27Z

Summary:

This issue fixesreact/react-native#30939fixesreact/react-native#30840 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by the missing prop accessibilityState in the Slider component.

The solution consists of passing the accessibilityState to the RCTSliderNativeComponent component as previously implemented in other components (for example, Button).

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

The solution proposed in this pull request consists of:

removing the disabled default prop (related discussions fabOnReact@fc9f0c4 and fabOnReact/react-native@afb7fc2).
If the value of prop accessibilityState.disabled is different from the prop disabled, the prop disabled over-rides the accessibilityState.disabled value.

For example:

<Sliderdisabled={true}accessibilityState={{disabled: false}}/>

becomes:

<Sliderdisabled={true}accessibilityState={{disabled: true}}/>

Remove the existing Android java logic

An alternative Solution published in this branch would be:

not removing the default prop for disabled (false).
disabled default prop false over-rides accessibilityState.disabled

This alternative solution would not respect the requirement of not having conflicting states for accessibility and non-accessibility users and the approach adopted in react-native Components (for ex. Button).

For example:

<SlideraccessibilityState={{disabled: true}}/>

would become (as the deafult is disabled={false})

<SlideraccessibilityState={{disabled: false}}/>

ADDITIONAL NOTES

<Slider disabled={"true"} /> triggers a wrong type warning with typescript.
<Slider disabled={"true"} /> does not trigger a wrong type warning with flow.js.

Test Plan:

Android:

1. Slider has disabled and accessibilityState={{disabled: false}}
2. Slider has disabled
3. Slider has accessibilityState={{disabled: true}}
4. Slider has accessibilityState={{disabled:false}}
5. Slider has disabled={false} and accessibilityState={{disabled:true}}
7. Test Cases on the main branch
7.1. Slider has disabled and accessibilityState={{disabled: false}}
7.3 Slider has accessibilityState={{disabled: true}}
7.5 Slider has disabled={false} and accessibilityState={{disabled:true}}
7.6 Wrong prop type disabled="not a boolean value" with typescript
7.7 Wrong prop type disabled="not a boolean value" with flow on the master branch

Flow check on master seems to not work fabOnReact/react-native-notes#2 (comment) As an alternative I implemented an invariant check. https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245 https://github.com/zertosh/invariant

I will try to fix .flowconfig and enable flow type checking for Slider props

…essibility disabled state

fabOnReact · 2022-02-01T02:29:37Z

TO-DO

Test Android
Test iOS

BartoszKlonowski · 2022-02-04T10:47:13Z

Thank you, @fabriziobertoglio1987! 👍
Looking forward to have this improvement - can you let me know if you plan to continue (making it ready to be reviewed)?

Also: Let me know if you need help with testing it on iOS (of course it's all Java implementation, but we will need to check if changing the JS layer affected other platforms).

fabOnReact · 2022-02-04T22:23:37Z

Thanks a lot @BartoszKlonowski
I plan to continue. I will test iOS and then move this pull request to Ready to Review.

Thanks a lot
I wish you a good weekend 🙏

BartoszKlonowski

Good work in general, thank you! 👍
The only remark I have is to create separate examples and it should be ready to deliver.

There's already a test, where disabled={true} is checked against accessibilityState={{disabled: false}}, but to double check all the combinations this commit adds new test that acts as the inverted scenario to make sure that conflicts are avoided both directions.

…ality when disabled (#33076) Summary: This issue fixes#30937fixes#30947fixes#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

It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski · 2022-02-28T17:17:33Z

Thank you, @fabriziobertoglio1987!

…ality when disabled (react#33076) Summary: This issue fixesreact#30937fixesreact#30947fixesreact#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 react#31252) Related PR react#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: react#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]: react@17095c6 [11]: react@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

fabOnReact · 2022-02-01T02:14:27Z

Summary:

This issue fixesreact/react-native#30939fixesreact/react-native#30840 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by the missing prop accessibilityState in the Slider component.

The solution consists of passing the accessibilityState to the RCTSliderNativeComponent component as previously implemented in other components (for example, Button).

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

The solution proposed in this pull request consists of:

removing the disabled default prop (related discussions fabOnReact@fc9f0c4 and fabOnReact/react-native@afb7fc2).
If the value of prop accessibilityState.disabled is different from the prop disabled, the prop disabled over-rides the accessibilityState.disabled value.

For example:

<Sliderdisabled={true}accessibilityState={{disabled: false}}/>

becomes:

<Sliderdisabled={true}accessibilityState={{disabled: true}}/>

Remove the existing Android java logic

An alternative Solution published in this branch would be:

not removing the default prop for disabled (false).
disabled default prop false over-rides accessibilityState.disabled

This alternative solution would not respect the requirement of not having conflicting states for accessibility and non-accessibility users and the approach adopted in react-native Components (for ex. Button).

For example:

<SlideraccessibilityState={{disabled: true}}/>

would become (as the deafult is disabled={false})

<SlideraccessibilityState={{disabled: false}}/>

ADDITIONAL NOTES

<Slider disabled={"true"} /> triggers a wrong type warning with typescript.
<Slider disabled={"true"} /> does not trigger a wrong type warning with flow.js.

Test Plan:

Android:

1. Slider has disabled and accessibilityState={{disabled: false}}
2. Slider has disabled
3. Slider has accessibilityState={{disabled: true}}
4. Slider has accessibilityState={{disabled:false}}
5. Slider has disabled={false} and accessibilityState={{disabled:true}}
7. Test Cases on the main branch
7.1. Slider has disabled and accessibilityState={{disabled: false}}
7.3 Slider has accessibilityState={{disabled: true}}
7.5 Slider has disabled={false} and accessibilityState={{disabled:true}}
7.6 Wrong prop type disabled="not a boolean value" with typescript
7.7 Wrong prop type disabled="not a boolean value" with flow on the master branch

Flow check on master seems to not work fabOnReact/react-native-notes#2 (comment) As an alternative I implemented an invariant check. https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245 https://github.com/zertosh/invariant

I will try to fix .flowconfig and enable flow type checking for Slider props

…essibility disabled state

fabOnReact · 2022-02-01T02:29:37Z

TO-DO

Test Android
Test iOS

BartoszKlonowski · 2022-02-04T10:47:13Z

Thank you, @fabriziobertoglio1987! 👍
Looking forward to have this improvement - can you let me know if you plan to continue (making it ready to be reviewed)?

Also: Let me know if you need help with testing it on iOS (of course it's all Java implementation, but we will need to check if changing the JS layer affected other platforms).

fabOnReact · 2022-02-04T22:23:37Z

Thanks a lot @BartoszKlonowski
I plan to continue. I will test iOS and then move this pull request to Ready to Review.

Thanks a lot
I wish you a good weekend 🙏

BartoszKlonowski

Good work in general, thank you! 👍
The only remark I have is to create separate examples and it should be ready to deliver.

There's already a test, where disabled={true} is checked against accessibilityState={{disabled: false}}, but to double check all the combinations this commit adds new test that acts as the inverted scenario to make sure that conflicts are avoided both directions.

…ality when disabled (#33076) Summary: This issue fixes#30937fixes#30947fixes#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

It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski · 2022-02-28T17:17:33Z

Thank you, @fabriziobertoglio1987!

…ality when disabled (react#33076) Summary: This issue fixesreact#30937fixesreact#30947fixesreact#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 react#31252) Related PR react#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: react#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]: react@17095c6 [11]: react@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

fabOnReact · 2022-02-01T02:14:27Z

Summary:

This issue fixesreact/react-native#30939fixesreact/react-native#30840 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by the missing prop accessibilityState in the Slider component.

The solution consists of passing the accessibilityState to the RCTSliderNativeComponent component as previously implemented in other components (for example, Button).

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

The solution proposed in this pull request consists of:

removing the disabled default prop (related discussions fabOnReact@fc9f0c4 and fabOnReact/react-native@afb7fc2).
If the value of prop accessibilityState.disabled is different from the prop disabled, the prop disabled over-rides the accessibilityState.disabled value.

For example:

<Sliderdisabled={true}accessibilityState={{disabled: false}}/>

becomes:

<Sliderdisabled={true}accessibilityState={{disabled: true}}/>

Remove the existing Android java logic

An alternative Solution published in this branch would be:

not removing the default prop for disabled (false).
disabled default prop false over-rides accessibilityState.disabled

This alternative solution would not respect the requirement of not having conflicting states for accessibility and non-accessibility users and the approach adopted in react-native Components (for ex. Button).

For example:

<SlideraccessibilityState={{disabled: true}}/>

would become (as the deafult is disabled={false})

<SlideraccessibilityState={{disabled: false}}/>

ADDITIONAL NOTES

<Slider disabled={"true"} /> triggers a wrong type warning with typescript.
<Slider disabled={"true"} /> does not trigger a wrong type warning with flow.js.

Test Plan:

Android:

1. Slider has disabled and accessibilityState={{disabled: false}}
2. Slider has disabled
3. Slider has accessibilityState={{disabled: true}}
4. Slider has accessibilityState={{disabled:false}}
5. Slider has disabled={false} and accessibilityState={{disabled:true}}
7. Test Cases on the main branch
7.1. Slider has disabled and accessibilityState={{disabled: false}}
7.3 Slider has accessibilityState={{disabled: true}}
7.5 Slider has disabled={false} and accessibilityState={{disabled:true}}
7.6 Wrong prop type disabled="not a boolean value" with typescript
7.7 Wrong prop type disabled="not a boolean value" with flow on the master branch

Flow check on master seems to not work fabOnReact/react-native-notes#2 (comment) As an alternative I implemented an invariant check. https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245 https://github.com/zertosh/invariant

I will try to fix .flowconfig and enable flow type checking for Slider props

…essibility disabled state

fabOnReact · 2022-02-01T02:29:37Z

TO-DO

Test Android
Test iOS

BartoszKlonowski · 2022-02-04T10:47:13Z

Thank you, @fabriziobertoglio1987! 👍
Looking forward to have this improvement - can you let me know if you plan to continue (making it ready to be reviewed)?

Also: Let me know if you need help with testing it on iOS (of course it's all Java implementation, but we will need to check if changing the JS layer affected other platforms).

fabOnReact · 2022-02-04T22:23:37Z

Thanks a lot @BartoszKlonowski
I plan to continue. I will test iOS and then move this pull request to Ready to Review.

Thanks a lot
I wish you a good weekend 🙏

BartoszKlonowski

Good work in general, thank you! 👍
The only remark I have is to create separate examples and it should be ready to deliver.

There's already a test, where disabled={true} is checked against accessibilityState={{disabled: false}}, but to double check all the combinations this commit adds new test that acts as the inverted scenario to make sure that conflicts are avoided both directions.

…ality when disabled (#33076) Summary: This issue fixes#30937fixes#30947fixes#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

It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski · 2022-02-28T17:17:33Z

Thank you, @fabriziobertoglio1987!

…ality when disabled (react#33076) Summary: This issue fixesreact#30937fixesreact#30947fixesreact#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 react#31252) Related PR react#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: react#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]: react@17095c6 [11]: react@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

fabOnReact · 2022-02-01T02:14:27Z

Summary:

This issue fixesreact/react-native#30939fixesreact/react-native#30840 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by the missing prop accessibilityState in the Slider component.

The solution consists of passing the accessibilityState to the RCTSliderNativeComponent component as previously implemented in other components (for example, Button).

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

The solution proposed in this pull request consists of:

removing the disabled default prop (related discussions fabOnReact@fc9f0c4 and fabOnReact/react-native@afb7fc2).
If the value of prop accessibilityState.disabled is different from the prop disabled, the prop disabled over-rides the accessibilityState.disabled value.

For example:

<Sliderdisabled={true}accessibilityState={{disabled: false}}/>

becomes:

<Sliderdisabled={true}accessibilityState={{disabled: true}}/>

Remove the existing Android java logic

An alternative Solution published in this branch would be:

not removing the default prop for disabled (false).
disabled default prop false over-rides accessibilityState.disabled

This alternative solution would not respect the requirement of not having conflicting states for accessibility and non-accessibility users and the approach adopted in react-native Components (for ex. Button).

For example:

<SlideraccessibilityState={{disabled: true}}/>

would become (as the deafult is disabled={false})

<SlideraccessibilityState={{disabled: false}}/>

ADDITIONAL NOTES

<Slider disabled={"true"} /> triggers a wrong type warning with typescript.
<Slider disabled={"true"} /> does not trigger a wrong type warning with flow.js.

Test Plan:

Android:

1. Slider has disabled and accessibilityState={{disabled: false}}
2. Slider has disabled
3. Slider has accessibilityState={{disabled: true}}
4. Slider has accessibilityState={{disabled:false}}
5. Slider has disabled={false} and accessibilityState={{disabled:true}}
7. Test Cases on the main branch
7.1. Slider has disabled and accessibilityState={{disabled: false}}
7.3 Slider has accessibilityState={{disabled: true}}
7.5 Slider has disabled={false} and accessibilityState={{disabled:true}}
7.6 Wrong prop type disabled="not a boolean value" with typescript
7.7 Wrong prop type disabled="not a boolean value" with flow on the master branch

Flow check on master seems to not work fabOnReact/react-native-notes#2 (comment) As an alternative I implemented an invariant check. https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245 https://github.com/zertosh/invariant

I will try to fix .flowconfig and enable flow type checking for Slider props

…essibility disabled state

fabOnReact · 2022-02-01T02:29:37Z

TO-DO

Test Android
Test iOS

BartoszKlonowski · 2022-02-04T10:47:13Z

Thank you, @fabriziobertoglio1987! 👍
Looking forward to have this improvement - can you let me know if you plan to continue (making it ready to be reviewed)?

Also: Let me know if you need help with testing it on iOS (of course it's all Java implementation, but we will need to check if changing the JS layer affected other platforms).

fabOnReact · 2022-02-04T22:23:37Z

Thanks a lot @BartoszKlonowski
I plan to continue. I will test iOS and then move this pull request to Ready to Review.

Thanks a lot
I wish you a good weekend 🙏

BartoszKlonowski

Good work in general, thank you! 👍
The only remark I have is to create separate examples and it should be ready to deliver.

There's already a test, where disabled={true} is checked against accessibilityState={{disabled: false}}, but to double check all the combinations this commit adds new test that acts as the inverted scenario to make sure that conflicts are avoided both directions.

…ality when disabled (#33076) Summary: This issue fixes#30937fixes#30947fixes#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

It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski · 2022-02-28T17:17:33Z

Thank you, @fabriziobertoglio1987!

…ality when disabled (react#33076) Summary: This issue fixesreact#30937fixesreact#30947fixesreact#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 react#31252) Related PR react#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: react#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]: react@17095c6 [11]: react@6ab7ab3 Reviewed By: blavalla Differential Revision: D34211793 Pulled By: ShikaSD fbshipit-source-id: e153fb48c194f5884e30beb9172e66aca7ce1a41

fabOnReact added 13 commits January 26, 2022 16:31

adding accessibilityState.disabled prop to Slider.js
aaa1685

removing java logic to trigger accessibility disabled announcement
9d2e110

jest tests accessiblityState.disabled and disabled prop
8a2bd07

update jest snapshots
88d063e

adding Slider disabled example
80d7056

trigger invariant error when props.disabled is null
bc3f450

remove check on prop type commit16d321b
db11dd3
I will try to fix .flowconfig and enable flow type checking for Slider props

improve test cases descriptions
1af1de1

add LogBox (console.warn) message to notify developer conflicting acc…
584098b
…essibility disabled state

update LogBox warning text
d15d881

remove LogBox warning
6e99b65

jest fix obsolete snapshots
3bf5b71

BartoszKlonowski self-requested a review February 1, 2022 11:20

fabOnReact marked this pull request as ready for review February 11, 2022 06:30

BartoszKlonowski approved these changes Feb 15, 2022
View reviewed changes

Comment threadsrc/js/Slider.js
Comment threadexample/SliderExample.js Outdated

avoid conflicting disabled states
8a8436e
It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski merged commit 006dcb9 into callstack:mainFeb 28, 2022

fabOnReact mentioned this pull request Apr 8, 2022
Android: Role description is announced before the components text, rather than after #31042 fabOnReact/react-native-notes#14
Closed

fabOnReact added 13 commits January 26, 2022 16:31

adding accessibilityState.disabled prop to Slider.js
aaa1685

removing java logic to trigger accessibility disabled announcement
9d2e110

jest tests accessiblityState.disabled and disabled prop
8a2bd07

update jest snapshots
88d063e

adding Slider disabled example
80d7056

trigger invariant error when props.disabled is null
bc3f450

remove check on prop type commit16d321b
db11dd3
I will try to fix .flowconfig and enable flow type checking for Slider props

improve test cases descriptions
1af1de1

add LogBox (console.warn) message to notify developer conflicting acc…
584098b
…essibility disabled state

update LogBox warning text
d15d881

remove LogBox warning
6e99b65

jest fix obsolete snapshots
3bf5b71

BartoszKlonowski self-requested a review February 1, 2022 11:20

fabOnReact marked this pull request as ready for review February 11, 2022 06:30

BartoszKlonowski approved these changes Feb 15, 2022
View reviewed changes

Comment threadsrc/js/Slider.js
Comment threadexample/SliderExample.js Outdated

avoid conflicting disabled states
8a8436e
It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski merged commit 006dcb9 into callstack:mainFeb 28, 2022

fabOnReact mentioned this pull request Apr 8, 2022
Android: Role description is announced before the components text, rather than after #31042 fabOnReact/react-native-notes#14
Closed

fabOnReact added 13 commits January 26, 2022 16:31

adding accessibilityState.disabled prop to Slider.js
aaa1685

removing java logic to trigger accessibility disabled announcement
9d2e110

jest tests accessiblityState.disabled and disabled prop
8a2bd07

update jest snapshots
88d063e

adding Slider disabled example
80d7056

trigger invariant error when props.disabled is null
bc3f450

remove check on prop type commit16d321b
db11dd3
I will try to fix .flowconfig and enable flow type checking for Slider props

improve test cases descriptions
1af1de1

add LogBox (console.warn) message to notify developer conflicting acc…
584098b
…essibility disabled state

update LogBox warning text
d15d881

remove LogBox warning
6e99b65

jest fix obsolete snapshots
3bf5b71

BartoszKlonowski self-requested a review February 1, 2022 11:20

fabOnReact marked this pull request as ready for review February 11, 2022 06:30

BartoszKlonowski approved these changes Feb 15, 2022
View reviewed changes

Comment threadsrc/js/Slider.js
Comment threadexample/SliderExample.js Outdated

avoid conflicting disabled states
8a8436e
It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski merged commit 006dcb9 into callstack:mainFeb 28, 2022

fabOnReact mentioned this pull request Apr 8, 2022
Android: Role description is announced before the components text, rather than after #31042 fabOnReact/react-native-notes#14
Closed

fabOnReact added 13 commits January 26, 2022 16:31

adding accessibilityState.disabled prop to Slider.js
aaa1685

removing java logic to trigger accessibility disabled announcement
9d2e110

jest tests accessiblityState.disabled and disabled prop
8a2bd07

update jest snapshots
88d063e

adding Slider disabled example
80d7056

trigger invariant error when props.disabled is null
bc3f450

remove check on prop type commit16d321b
db11dd3
I will try to fix .flowconfig and enable flow type checking for Slider props

improve test cases descriptions
1af1de1

add LogBox (console.warn) message to notify developer conflicting acc…
584098b
…essibility disabled state

update LogBox warning text
d15d881

remove LogBox warning
6e99b65

jest fix obsolete snapshots
3bf5b71

BartoszKlonowski self-requested a review February 1, 2022 11:20

fabOnReact marked this pull request as ready for review February 11, 2022 06:30

BartoszKlonowski approved these changes Feb 15, 2022
View reviewed changes

Comment threadsrc/js/Slider.js
Comment threadexample/SliderExample.js Outdated

avoid conflicting disabled states
8a8436e
It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski merged commit 006dcb9 into callstack:mainFeb 28, 2022

fabOnReact mentioned this pull request Apr 8, 2022
Android: Role description is announced before the components text, rather than after #31042 fabOnReact/react-native-notes#14
Closed

fabOnReact added 13 commits January 26, 2022 16:31

adding accessibilityState.disabled prop to Slider.js
aaa1685

removing java logic to trigger accessibility disabled announcement
9d2e110

jest tests accessiblityState.disabled and disabled prop
8a2bd07

update jest snapshots
88d063e

adding Slider disabled example
80d7056

trigger invariant error when props.disabled is null
bc3f450

remove check on prop type commit16d321b
db11dd3
I will try to fix .flowconfig and enable flow type checking for Slider props

improve test cases descriptions
1af1de1

add LogBox (console.warn) message to notify developer conflicting acc…
584098b
…essibility disabled state

update LogBox warning text
d15d881

remove LogBox warning
6e99b65

jest fix obsolete snapshots
3bf5b71

BartoszKlonowski self-requested a review February 1, 2022 11:20

fabOnReact marked this pull request as ready for review February 11, 2022 06:30

BartoszKlonowski approved these changes Feb 15, 2022
View reviewed changes

Comment threadsrc/js/Slider.js
Comment threadexample/SliderExample.js Outdated

avoid conflicting disabled states
8a8436e
It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski merged commit 006dcb9 into callstack:mainFeb 28, 2022

fabOnReact mentioned this pull request Apr 8, 2022
Android: Role description is announced before the components text, rather than after #31042 fabOnReact/react-native-notes#14
Closed

fabOnReact added 13 commits January 26, 2022 16:31

adding accessibilityState.disabled prop to Slider.js
aaa1685

removing java logic to trigger accessibility disabled announcement
9d2e110

jest tests accessiblityState.disabled and disabled prop
8a2bd07

update jest snapshots
88d063e

adding Slider disabled example
80d7056

trigger invariant error when props.disabled is null
bc3f450

remove check on prop type commit16d321b
db11dd3
I will try to fix .flowconfig and enable flow type checking for Slider props

improve test cases descriptions
1af1de1

add LogBox (console.warn) message to notify developer conflicting acc…
584098b
…essibility disabled state

update LogBox warning text
d15d881

remove LogBox warning
6e99b65

jest fix obsolete snapshots
3bf5b71

BartoszKlonowski self-requested a review February 1, 2022 11:20

fabOnReact marked this pull request as ready for review February 11, 2022 06:30

BartoszKlonowski approved these changes Feb 15, 2022
View reviewed changes

Comment threadsrc/js/Slider.js
Comment threadexample/SliderExample.js Outdated

avoid conflicting disabled states
8a8436e
It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski merged commit 006dcb9 into callstack:mainFeb 28, 2022

fabOnReact mentioned this pull request Apr 8, 2022
Android: Role description is announced before the components text, rather than after #31042 fabOnReact/react-native-notes#14
Closed

fabOnReact added 13 commits January 26, 2022 16:31

adding accessibilityState.disabled prop to Slider.js
aaa1685

removing java logic to trigger accessibility disabled announcement
9d2e110

jest tests accessiblityState.disabled and disabled prop
8a2bd07

update jest snapshots
88d063e

adding Slider disabled example
80d7056

trigger invariant error when props.disabled is null
bc3f450

remove check on prop type commit16d321b
db11dd3
I will try to fix .flowconfig and enable flow type checking for Slider props

improve test cases descriptions
1af1de1

add LogBox (console.warn) message to notify developer conflicting acc…
584098b
…essibility disabled state

update LogBox warning text
d15d881

remove LogBox warning
6e99b65

jest fix obsolete snapshots
3bf5b71

BartoszKlonowski self-requested a review February 1, 2022 11:20

fabOnReact marked this pull request as ready for review February 11, 2022 06:30

BartoszKlonowski approved these changes Feb 15, 2022
View reviewed changes

Comment threadsrc/js/Slider.js
Comment threadexample/SliderExample.js Outdated

avoid conflicting disabled states
8a8436e
It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski merged commit 006dcb9 into callstack:mainFeb 28, 2022

fabOnReact mentioned this pull request Apr 8, 2022
Android: Role description is announced before the components text, rather than after #31042 fabOnReact/react-native-notes#14
Closed

fabOnReact added 13 commits January 26, 2022 16:31

adding accessibilityState.disabled prop to Slider.js
aaa1685

removing java logic to trigger accessibility disabled announcement
9d2e110

jest tests accessiblityState.disabled and disabled prop
8a2bd07

update jest snapshots
88d063e

adding Slider disabled example
80d7056

trigger invariant error when props.disabled is null
bc3f450

remove check on prop type commit16d321b
db11dd3
I will try to fix .flowconfig and enable flow type checking for Slider props

improve test cases descriptions
1af1de1

add LogBox (console.warn) message to notify developer conflicting acc…
584098b
…essibility disabled state

update LogBox warning text
d15d881

remove LogBox warning
6e99b65

jest fix obsolete snapshots
3bf5b71

BartoszKlonowski self-requested a review February 1, 2022 11:20

fabOnReact marked this pull request as ready for review February 11, 2022 06:30

BartoszKlonowski approved these changes Feb 15, 2022
View reviewed changes

Comment threadsrc/js/Slider.js
Comment threadexample/SliderExample.js Outdated

avoid conflicting disabled states
8a8436e
It would be better to show that both these properties can be used separately without fear of another. callstack#354 (comment)

BartoszKlonowski merged commit 006dcb9 into callstack:mainFeb 28, 2022

fabOnReact mentioned this pull request Apr 8, 2022
Android: Role description is announced before the components text, rather than after #31042 fabOnReact/react-native-notes#14
Closed

Uh oh!

Conversation

fabOnReact commented Feb 1, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Test Plan:

Uh oh!

fabOnReact commented Feb 1, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TO-DO

Uh oh!

BartoszKlonowski commented Feb 4, 2022

Uh oh!

fabOnReact commented Feb 4, 2022

Uh oh!

BartoszKlonowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BartoszKlonowski commented Feb 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

fabOnReact commented Feb 1, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Test Plan:

Uh oh!

fabOnReact commented Feb 1, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TO-DO

Uh oh!

BartoszKlonowski commented Feb 4, 2022

Uh oh!

fabOnReact commented Feb 4, 2022

Uh oh!

BartoszKlonowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BartoszKlonowski commented Feb 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

fabOnReact commented Feb 1, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Test Plan:

Uh oh!

fabOnReact commented Feb 1, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TO-DO

Uh oh!

BartoszKlonowski commented Feb 4, 2022

Uh oh!

fabOnReact commented Feb 4, 2022

Uh oh!

BartoszKlonowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fabOnReact commented Feb 1, 2022•
edited
Loading

fabOnReact commented Feb 1, 2022•
edited
Loading

fabOnReact commented Feb 1, 2022•
edited
Loading

fabOnReact commented Feb 1, 2022•
edited
Loading

fabOnReact commented Feb 1, 2022•
edited
Loading

fabOnReact commented Feb 1, 2022•
edited
Loading

fabOnReact commented Feb 1, 2022•
edited
Loading

fabOnReact commented Feb 1, 2022•
edited
Loading

fabOnReact commented Feb 1, 2022•
edited
Loading