fabOnReact / react-native-notes

MIT License
0 stars 0 forks source link

Android: Role description is announced before the components text, rather than after #31042 #14

Closed fabOnReact closed 2 years ago

fabOnReact commented 2 years ago

Issue https://github.com/facebook/react-native/issues/31042 PR https://github.com/facebook/react-native/pull/33690

fabOnReact commented 2 years ago

https://github.com/fabriziobertoglio1987/react-native-notes/blob/472d531ae867ac23a866851cb51759053b267b1c/Libraries/Components/View/ViewNativeComponent.js#L22-L25

https://github.com/fabriziobertoglio1987/react-native-notes/blob/472d531ae867ac23a866851cb51759053b267b1c/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricComponents.java#L26-L43

https://github.com/fabriziobertoglio1987/react-native-notes/blob/472d531ae867ac23a866851cb51759053b267b1c/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java#L19

https://github.com/fabriziobertoglio1987/react-native-notes/blob/472d531ae867ac23a866851cb51759053b267b1c/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewManager.java#L38-L40

fabOnReact commented 2 years ago
fabOnReact commented 2 years ago

https://github.com/facebook/react-native/pull/31296#discussion_r616184584 The issues seems to be caused when passing accessibilityRole="button" to a TouchableNativeFeedback https://github.com/fabriziobertoglio1987/react-native-notes/issues/1#issuecomment-1029609228

fabOnReact commented 2 years ago

The View does not announce the child component Text when accessibilityLabel is missing (automatic content grouping)

The View has accessibilityRole="button" and child component with:

Expected Result: The View announces This is a child Text, Button.

Actual Result: The View announces Button.

click to open code used in the test

```javascript {childText} ```

click to open recording of test case

fabOnReact commented 2 years ago

The View announces the child component Text when accessibilityLabel is missing (automatic content grouping)

The View has accessibilityRole="button" and child component with:

Expected/Actual Result: The View announces This is a child Text, Button.

Notes

click to open code used in the test

```javascript {childText} ```

click to open recording of test case

fabOnReact commented 2 years ago

Scenarios - TouchableNativeFeedback

Touchable with one child

``` Text number 1 ```

Touchable with multiple children

``` Text number 1 Text number 2 ```

Touchable with children that have children

``` Text number 1 Text number 2 Text number 3 ```

Touchable with children's that are not Text components

``` Text number 1 Text number 2 ```

Touchable with one Text link

``` Text number 1Link ```

fabOnReact commented 2 years ago
fabOnReact commented 2 years ago

Low Priority:

completed

- [x] test edge cases - [x] evaluate if we need [isTopLevelScrollItem][1] - [x] implement logic to only handle the basic scenario implement logic for isTopLevelScrollItem - [x] compare current implementation with Flipper - [x] prepare draft implementation - [x] evaluate adding/removing code after comparing with Flipper - [x] test EditText and verify behaviour is correct - [x] prepare pr summary

low priority

- [x] getRole to avoid triggering runtime error - [x] scenario - scroll view or other components - [ ] adding space when several text components next to each other - [x] consider adding a check on the accessibilityRole=button do we need this behaviour only for the items with accessibilityRole button? if yes, then we add a check on the accessibilityRole

fabOnReact commented 2 years ago

Screenreader focuses on ScrollView Items

Expected/Actual Result: This has to be verified again before moving the pr to Ready for Review.

click to open code used in the test

```javascript function TouchableExample(props) { return ( Text number 1 Text number 2Text number 3 ); } ```

click to open recording of test case - without the implementation

click to open recording of test case - with the implementation

fabOnReact commented 2 years ago

Screenreader announces Pressability items

Expected/Actual Result: Screenreader announces Text number 1Text number 2 Text number 3, Button.

click to open code used in the test

```javascript function TouchableExample(props) { return ( console.warn('pressed')} accessibilityRole="button" style={{backgroundColor: 'red'}}> Text number 1 Text number 2Text number 3 ); } ```

click to open recording of test case

fabOnReact commented 2 years ago

Screenreader correctly announcing accessible/non-accessible items

Expected/Actual Result: Talkback announces Text number 1Text number 2Text number3, Button. Talkback announces Text clickable.

click to open code used in the test

```javascript function TouchableExample(props) { return ( console.warn('pressed')} accessibilityRole="button" style={{backgroundColor: 'red'}}> console.warn('pressed')}>Text clickable Text number 1 Text number 2Text number 3 ); } ```

click to open recording of test case

fabOnReact commented 2 years ago

Conditions that should trigger the custom contentDescription

https://github.com/facebook/react-native/pull/33690#issuecomment-1106779940

The custom contentDescription should be triggered if:

fabOnReact commented 2 years ago

Runtime error when calling createNodeInfoFromView

  @Nullable
  public static AccessibilityNodeInfoCompat createNodeInfoFromView(View view) {
    if (view == null) {
      return null;
    }

    final AccessibilityNodeInfoCompat nodeInfo = AccessibilityNodeInfoCompat.obtain();

    // For some unknown reason, Android seems to occasionally throw a NPE from
    // onInitializeAccessibilityNodeInfo.
    try {
      // ==> Runtime when called from onInitializeAccessbilityNodeInfo
      ViewCompat.onInitializeAccessibilityNodeInfo(view, nodeInfo);
    } // ...
}

create an infinite loop, onInitializeAccessibilityNodeInfo calls method that calls itself without stopping.

fabOnReact commented 2 years ago

Button role is announced after child contentDescription with TouchableNativeFeedback

Expected/Actual Result: Talkback announces Text number 1, Text number 1, Button

click to open code used in the test

```javascript Text number 1 Text number 1 ```

without fix - Button role is announced before child text

without fix - Button role is announced before child text (second example)

with fix - Button role is announced after child text

with fix - Button role is announced after child text (second example)

fabOnReact commented 2 years ago

Testing for regressions in Accessibility Actions

Expected/Actual Result: Same behavior with/out implementation

click to open code used in the test

```javascript { switch (event.nativeEvent.actionName) { case 'activate': Alert.alert('Alert', 'Activate accessiblity action'); break; case 'copy': Alert.alert('Alert', 'copy action success'); break; } }}> Text number 1 Text number 2Text number 3 ```

click to open recording of test case - without implementation

click to open recording of test case - with implementation

fabOnReact commented 2 years ago
fabOnReact commented 2 years ago

Recordings of complete test cases in rn-tester app main and pr branch

click to open recording of test case - main branch

click to open recording of test case - pr branch

fabOnReact commented 2 years ago
fabOnReact commented 2 years ago
TouchableOpacity with child EditText annouces Button role before the child contentDescription

test case added with commit https://github.com/fabriziobertoglio1987/react-native/commit/9d72a70c3b27d8fb198dd32a1cd8c031e458d089 The issue is caused by this [line](https://github.com/fabriziobertoglio1987/react-native/blob/9d72a70c3b27d8fb198dd32a1cd8c031e458d089/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java#L308). `info.getText()` retrieves the text of the child TextInput (the placeholder), but the current element (in js the TouchableNativeFeedback), does not have contentDescription or Text and the role (button) of the TouchableNativeFeedback is announced before the contentDescription of the child component. TouchableNativeFeedback does not exist in ReactAndroid, and it is just a wrapper to return the child component with an onPress event handler, for this reason `info.getText()` returns the child text. The functionalities are developed with https://github.com/fabriziobertoglio1987/react-native/commit/9d1bf23028ef056ae0aba4283e727b2b4e239f52

test done on the 27.05

in this case the first text is not announced, because it's a secure entry.

test done on the 30.05

on commit https://github.com/fabriziobertoglio1987/react-native/commit/abed89ec86affa80f41a03eaf14e0cf41f1f1dbf

fabOnReact commented 2 years ago
TouchableOpacity with child EditText annouces placeholder text before and after the role

The issue reproduces on the main/pr branch. The placeholder text is announced before and after the role. **Expected Result** this is the placeholder, Button **Actual Result** this is the placeholder, Button, this is the placeholder [Explanation](https://user-images.githubusercontent.com/24992535/170980322-99df0839-c45a-48e4-a3c4-799ee42c9ad7.mp4) of the result: - The first announcement corresponds to the value returned from `nodeInfo.getText()` - The second announcement corresponds to the role - The last announcement corresponds to the hint text Flipper includes implementation of [getTalkbackHint](https://github.com/facebook/flipper/blob/e44cad5e9912b359378a43ec47c5a49bbca85fc3/android/src/main/java/com/facebook/flipper/plugins/inspector/descriptors/utils/AccessibilityUtil.java#L376) and ([getTalkbackData](https://github.com/facebook/flipper/blob/e44cad5e9912b359378a43ec47c5a49bbca85fc3/android/src/main/java/com/facebook/flipper/plugins/inspector/descriptors/utils/AccessibilityUtil.java#L715-L719)).

Source Code of Test Case

```javascript ```

Test Case of Main Branch

Test Case of PR Branch

fabOnReact commented 2 years ago
TouchableOpacity with TextInput child announces contentDescription before the Role

fabOnReact commented 2 years ago
EditText is allways accessible to TalkBack screenreader

I could not find a test scenarios for this [logic](https://github.com/fabriziobertoglio1987/react-native/blob/388911e4e7b27a627def6e276562018e7e400c4f/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java#L1053-L1075) implemented with commit https://github.com/fabriziobertoglio1987/react-native/commit/388911e4e7b27a627def6e276562018e7e400c4f The EditText is always accessible, so it makes no sense to over-ride the TalkBack logic. The logic could be deleted.

Sourcecode

The [code](https://github.com/fabriziobertoglio1987/react-native/blob/388911e4e7b27a627def6e276562018e7e400c4f/packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js#L283-L299) of the example ```javascript Text number 1 ```

https://user-images.githubusercontent.com/24992535/171169545-339dc638-3938-4166-928a-bba47b280a75.mp4

fabOnReact commented 2 years ago

The default implementation of AccNodeInfo.getRoleDescription does not work with all roles:

1) image => “Text number 1, Image, Text number 2, Image” With a parent role button => “Text number 1, Text number 2, Button” 2) text => “Text number 1, Text number 2” 3) button => the component with role button becomes accessible 4) search => “Text number 1, EditBox, Text number 2”

fabOnReact commented 2 years ago
fabOnReact commented 2 years ago
One of the child has accessibilityState (hasStateDescription triggers the announcement)

https://github.com/fabriziobertoglio1987/react-native/commit/c17f00ea321221d379a4b9e4fefbca991f50afc1 https://github.com/facebook/react-native/pull/33690#discussion_r890259702

sourcecode from test case

```javascript Text number 1 ```

fabOnReact commented 2 years ago
One of the child has accessibilityHint (hasText triggers the announcement)

https://github.com/fabriziobertoglio1987/react-native/commit/c17f00ea321221d379a4b9e4fefbca991f50afc1 https://github.com/facebook/react-native/pull/33690#discussion_r890259702

sourcecode from test case

```javascript ```