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

Text Component does not announce "disabled" #30937 #1

Closed
fabOnReact opened this issue Oct 3, 2021 · 21 comments
Closed

Text Component does not announce "disabled" #30937 #1

fabOnReact opened this issue Oct 3, 2021 · 21 comments

Comments

@fabOnReact
Copy link
Owner

facebook/react-native#30937

@fabOnReact

This comment was marked as outdated.

@fabOnReact fabOnReact reopened this Feb 4, 2022
@fabOnReact fabOnReact pinned this issue Feb 4, 2022
@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 4, 2022

Test Cases

  1. ScreenReader does NOT announce disabled when moving focus to TouchableNativeFeedback
<TouchableNativeFeedback accessibilityState={{disabled: true}}>
    <Text>TouchableNativeFeedback</Text>
</TouchableNativeFeedback>

Expected Result:

  • ScreenReader announces disabled

Actual Result:

  • ScreenReader does NOT announce disabled

NOTES:

The problems does not reproduce with the View Component

https://github.com/fabriziobertoglio1987/react-native-notes/blob/9cd43340a7e2443564c2ff5e8e85d37f6e1e47ef/Libraries/Components/Touchable/TouchableNativeFeedback.js#L262-L264

<TouchableNativeFeedback accessibilityState={{disabled: true}}>
    <Text>TouchableNativeFeedback</Text>
</TouchableNativeFeedback>

renders

<Text>TouchableNativeFeedback</Text>
<TouchableNativeFeedback accessibilityState={{disabled: true}}>
    <View>
        <Text>TouchableNativeFeedback</Text>
    </View>
</TouchableNativeFeedback>

renders

<View><Text>TouchableNativeFeedback</Text></View>

CLICK TO OPEN TESTS RESULTS

2022-02-04.11-44-09.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 4, 2022

Test Cases

  1. ScreenReader does announce disabled when moving focus to TouchableNativeFeedback
<TouchableNativeFeedback accessibilityState={{disabled: true}}>
    <View>
       <Text>TouchableNativeFeedback</Text>
    </View>
</TouchableNativeFeedback>

Expected/Actual Result:

  • ScreenReader announces disabled

CLICK TO OPEN TESTS RESULTS

2022-02-04.11-50-25.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 4, 2022

Test Cases

  1. ScreenReader announces disabled when moving focus on <View focusable={true} />
<View
  focusable={true}
  style={{backgroundColor: 'red', height: 100, width: 100}}
  accessibilityState={{disabled: true}}>
  <Text style={{backgroundColor: 'green', color: 'white'}}>
    This is a View and it has an accessibilityState
  </Text>
</View>

Expected/Actual Result:

  • The ScreenReader announces disabled

NOTES:

ReactViewManager#focusable

Relevant discussion facebook/react-native#31033 (comment)

https://github.com/fabriziobertoglio1987/react-native-notes/blob/9cd43340a7e2443564c2ff5e8e85d37f6e1e47ef/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java#L236-L264

CLICK TO OPEN TESTS RESULTS

2022-02-04.12-35-09.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 4, 2022

Test Cases

  1. ScreenReader announces disabled when moving focus to Text
<Text accessibilityState={{disabled: true}}>This is a Text Example</Text>

Adding the prop accessible to ReactTextAnchorViewManager fixes the problem for this component.
The same solution from my previous pr facebook/react-native#30935 (comment).

https://github.com/fabriziobertoglio1987/react-native-notes/blob/9cd43340a7e2443564c2ff5e8e85d37f6e1e47ef/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextAnchorViewManager.java#L38-L39

       @ReactProp(name = "accessible")
       public void setAccessible(ReactTextView view, boolean accessible) {
         view.setFocusable(accessible);
       }

Expected/Actual Result:

  • ScreenReader announces disabled

CLICK TO OPEN TESTS RESULTS

2022-02-04.13-16-28.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 4, 2022

TO-DO:

  • Review diff
  • Write a list of edge cases
  • Test edge cases
  • Fix PR to handle edge case
  • Review fix
  • Test
  • Write test cases

Fix PR to handle edge case:

  • Remove Java logic (setAccessible) method in TextAnchorManager
  • pass prop disabled to NativeText
  • change prop as previously done with button
  • test

Edge Cases

Low Priority

@fabOnReact

This comment was marked as off-topic.

@fabOnReact fabOnReact unpinned this issue Feb 7, 2022
fabOnReact added a commit to fabOnReact/react-native that referenced this issue Feb 9, 2022
Adding the prop `accessible` to `ReactTextAnchorViewManager` fixes the problem for this component.
The same solution from my previous pr facebook#30935 (comment).

See test case at fabOnReact/react-native-notes#1 (comment)
@fabOnReact fabOnReact pinned this issue Feb 9, 2022
@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 9, 2022

Test Cases

7.6. Text has onPress callback and accessibilityState = {{ disabled: true }}

<Text
  onPress={() => console.warn('onPress')}
  accessibilityState={{disabled: true}}>
  Awesome App
</Text>

Expected Result:
The Text is disabled and should NOT trigger the onPress callback

Actual Result:
The Text announces disabled, but is not disabled and triggers the onPress callback

Related facebook/react-native#30947 Fixed with commits facebook/react-native@6ab7ab3 fabOnReact/react-native@17095c6

CLICK TO OPEN TESTS RESULTS - ON PR BRANCH BEFORE COMMIT 1c4f98dd

On PR Branch before commit 1c4f98dd the user is allowed to click on the text and trigger the onPress callback.

testcase15.mp4
CLICK TO OPEN TESTS RESULTS - MAIN BRANCH

testcase15onmain.mp4

fabOnReact added a commit to fabOnReact/react-native that referenced this issue Feb 9, 2022
Adding the prop `accessible` to `ReactTextAnchorViewManager` fixes the problem for this component.
The same solution from my previous pr facebook#30935 (comment).

See test case at fabOnReact/react-native-notes#1 (comment)
666647e
@fabOnReact

This comment was marked as outdated.

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 9, 2022

Test Cases

  1. Text has disabled and accessibilityState={{disabled: false}}
CLICK TO OPEN SOURCECODE

<Text
  style={styles.text}
  onPress={() => console.warn('onPress')}
  disabled
  accessibilityState={{disabled: false}}>
  This is a Text
</Text>

Expected/Actual Result:
The ScreenReader should announce disabled when the focus moves to the Text.
The onPress handler is disabled.

CLICK TO OPEN TESTS RESULTS

testcase1.mp4

@fabOnReact
Copy link
Owner Author

Test Cases

  1. Text has disabled
CLICK TO OPEN SOURCECODE

<Text
  style={styles.text}
  onPress={() => console.warn('onPress')}
  disabled>
  This is a Text
</Text>

Expected/Actual Result:

  • The Text is disabled
  • The ScreenReader announces disabled.
  • The onPress handler is disabled.
CLICK TO OPEN TESTS RESULTS

testcase2.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 9, 2022

Test Cases

  1. Text has accessibilityState={{disabled: true}}
CLICK TO OPEN SOURCECODE

<Text
  style={styles.text}
  onPress={() => console.warn('onPress')}
  accessibilityState={{disabled: true}}>
  This is a Text
</Text>

Expected/Actual Result:

  • The ScreenReader announces disabled when the focus moves to the Text.
  • The Text is disabled. Users are NOT allowed to interact with the text.
  • The onPress handler is disabled.
CLICK TO OPEN TESTS RESULTS

testcase3.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 9, 2022

Test Cases

  1. Text has accessibilityState={{disabled:false}}
CLICK TO OPEN SOURCECODE

<Text
  style={styles.text}
  onPress={() => console.warn('onPress')}
  accessibilityState={{disabled: false}}>
  This is a Text
</Text>

Expected/Actual Result:

  • The ScreenReader does not announce disabled.
  • The Text is not disabled. Users are allowed to interact with the text.
  • The onPress handler is NOT disabled.
CLICK TO OPEN TESTS RESULTS

testcase4.mp4

@fabOnReact
Copy link
Owner Author

Test Cases

  1. Text has disabled={false} accessibilityState={{disabled:true}}
CLICK TO OPEN SOURCECODE

<Text
  style={styles.text}
  onPress={() => console.warn('onPress')}
  disabled={false}
  accessibilityState={{disabled: true}}>
  This is a Text
</Text>

Expected/Actual Result:
The Text is not disabled.

CLICK TO OPEN TESTS RESULTS

testcase5.mp4

@fabOnReact
Copy link
Owner Author

Test Cases

7.1 Text has disabled set to true and accessibilityState={{disabled: false}} (main branch)

CLICK TO OPEN SOURCECODE

<Text
  style={styles.text}
  onPress={() => console.warn('onPress')}
  disabled
  accessibilityState={{disabled: false}}>
  This is a Text
</Text>

Expected Result:
The ScreenReader should announce disabled when the focus moves to the Text.

Actual Result:
The ScreenReader does not announce disabled when the focus moves to the Text.

CLICK TO OPEN TESTS RESULTS

testcase7-1.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 9, 2022

Test Cases

7.3. Text has accessibilityState={{disabled: true}} (tested on main branch).

CLICK TO OPEN SOURCECODE

<Text
  style={styles.text}
  onPress={() => console.warn('onPress')}
  accessibilityState={{disabled: true}}>
  This is a Text
</Text>

Expected Result:

  • The ScreenReader announces disabled when the focus moves to the Text.
  • The Text is disabled. Users are NOT allowed to interact with the Text (click and trigger onPress callback).

Actual Result:

  • The Text is NOT disabled.
  • Users are allowed to interact with the Text.
CLICK TO OPEN TESTS RESULTS

testcase7-3.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 9, 2022

Test Cases

7.5 Text has disabled={false} and accessibilityState={{disabled:true}} (on main branch)

CLICK TO OPEN SOURCECODE

<Text
  style={styles.text}
  onPress={() => console.warn('onPress')}
  disabled={false}
  accessibilityState={{disabled: true}}>
  This is a Text
</Text>

Expected Result:
The Text is disabled. ScreenReader should announce disabled when the focus moves to the Text.

Actual Result:

  • The ScreenReader does not announce disabled.
  • Users are allowed to interact with the Text.
CLICK TO OPEN TESTS RESULTS

testcase7-5.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 9, 2022

Summary

This issue fixes facebook/react-native#30937 facebook/react-native#30947 (Test Case 7.1, Test Case 7.3, Test Case 7.5) .
The issue is caused by:

  1. the missing prop accessibilityState in the Text component fabOnReact/react-native@6ab7ab3
  2. missing setter for prop accessible in ReactTextAnchorViewManager fabOnReact/react-native@17095c6

Related PRs facebook/react-native#33070 callstack/react-native-slider#354

Changelog

[General] [Fixed] - Text Component does not announce and disable click functionality when disabled

Test Plan

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

@fabOnReact
Copy link
Owner Author

PR facebook/react-native#33076

@fabOnReact fabOnReact unpinned this issue Feb 9, 2022
@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 13, 2022

Test Cases

7.7 Text has accessibilityState={{disabled:true}} and no method setAccessible in ReactTextAnchorViewManager (tested on commit facebook/react-native@6ab7ab3)

Without the below logic in ReactTextAnchorViewManager (added in commit facebook/react-native@17095c6).

  @ReactProp(name = "accessible")
  public void setAccessible(ReactTextView view, boolean accessible) {
    view.setFocusable(accessible);
  }
CLICK TO OPEN SOURCECODE

<Text accessibilityState={{disabled: true}}>
  This is a Text
</Text>

Expected Result:
The ScreenReader announces disabled when the focus moves to the Text.

Actual Result:

  • The ScreenReader does not announce disabled.
CLICK TO OPEN TESTS RESULTS

notAnnounceDisabled.mp4

@fabOnReact
Copy link
Owner Author

fabOnReact commented Feb 13, 2022

Test Cases

  1. Text has accessibilityState={{disabled:true}} and method setAccessible in ReactTextAnchorViewManager (tested on commit facebook/react-native@17095c6)

With the below logic in ReactTextAnchorViewManager.

  @ReactProp(name = "accessible")
  public void setAccessible(ReactTextView view, boolean accessible) {
    view.setFocusable(accessible);
  }
CLICK TO OPEN SOURCECODE

<Text accessibilityState={{disabled: true}}>
  This is a Text
</Text>

Expected/Actual Result:
The ScreenReader announces disabled when the focus moves to the Text.

CLICK TO OPEN TESTS RESULTS

announcesDisabled.mp4

facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Feb 15, 2022
…ality when disabled (#33076)

Summary:
This issue fixes #30937 fixes #30947 fixes #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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant