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

Infer accessibility label from children in RNGestureHandlerButton on iOS #3290

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

oblador
Copy link
Contributor

@oblador oblador commented Dec 16, 2024

Description

When using RectButton and friends without an explicit accessibilityLabel prop set, the accessibility label is empty on iOS, whereas on Android it's correctly inferred from its children like other react-native components work. I tracked this down to this component not inheriting from RCTView where this algorithm is defined. So to fix this, I simply vendored that function with a new prefix.

Test plan

Validated via Maestro Studio.

@latekvo
Copy link
Contributor

latekvo commented Dec 17, 2024

Hey, what repro did you use to test this feature?
The code looks great, but I'm failing to have the VoiceOver actually read the labels aloud.
Here's the test code I'm currently using:

Collapsed test code
import React from 'react';
import { StyleSheet, View, Text } from 'react-native';
import { RectButton } from 'react-native-gesture-handler';

export default function EmptyExample() {
  return (
    <View style={styles.container}>
      <Text importantForAccessibility="no">View inside RectButton</Text>
      <View style={styles.horizontal}>
        <RectButton accessibilityLabel="outer one">
          <Text importantForAccessibility="no">Outer + nested</Text>
          <View style={styles.box} accessibilityLabel="nested one" />
          <View style={styles.box} accessibilityLabel="nested two" />
        </RectButton>
        <RectButton>
          <Text importantForAccessibility="no">nested alone</Text>
          <View style={styles.box} accessibilityLabel="alone one" />
          <View style={styles.box} accessibilityLabel="alone two" />
        </RectButton>
        <RectButton accessibilityLabel="alone three">
          <Text importantForAccessibility="no">Outer</Text>
          <View style={styles.box} />
          <View style={styles.box} />
        </RectButton>
        <RectButton
          importantForAccessibility="no"
          accessibilityLabel="should be ignored">
          <Text>Ignored</Text>
          <View style={styles.box} />
          <View style={styles.box} />
        </RectButton>
      </View>
      <Text importantForAccessibility="no">RectButton inside View</Text>
      <View style={styles.horizontal}>
        <View accessibilityLabel="outer one">
          <Text importantForAccessibility="no">Outer + nested</Text>
          <RectButton style={styles.box} accessibilityLabel="nested one" />
          <RectButton style={styles.box} accessibilityLabel="nested two" />
        </View>
        <View>
          <Text importantForAccessibility="no">Outer + nested</Text>
          <RectButton style={styles.box} accessibilityLabel="alone one" />
          <RectButton style={styles.box} accessibilityLabel="alone two" />
        </View>
        <View accessibilityLabel="alone three">
          <Text importantForAccessibility="no">Outer + nested</Text>
          <RectButton style={styles.box} />
          <RectButton style={styles.box} />
        </View>
        <View>
          <Text>Ignored</Text>
          <RectButton
            style={styles.box}
            importantForAccessibility="no"
            accessibilityLabel="should be ignored one"
          />
          <RectButton
            style={styles.box}
            importantForAccessibility="no"
            accessibilityLabel="should be ignored two"
          />
        </View>
      </View>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
  },
  horizontal: {
    flex: 1,
    flexDirection: 'row',
    justifyContent: 'center',
    alignItems: 'center',
    gap: 10,
  },
  box: {
    padding: 40,
    backgroundColor: 'pink',
    margin: 1,
  },
  separator: {
    height: 1,
    width: 400,
    backgroundColor: 'black',
  },
});

@oblador
Copy link
Contributor Author

oblador commented Dec 17, 2024

I was using Maestro Studio, but you could do Accessibility Inspector that comes with Xcode too.

@oblador
Copy link
Contributor Author

oblador commented Dec 17, 2024

OK, so checked with the examples in this repo and it will only work if you pass the accessible prop. Should probably add this and other accessibility props to type definition.

Copy link
Contributor

@latekvo latekvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great fix, it looks like this is the default behaviour for React Native's interactive elements.

Please wait for a second approve before merging.

Tested on the following code:

Collapsed test code
import React from 'react';
import { StyleSheet, View, Text, Pressable } from 'react-native';
import { RectButton } from 'react-native-gesture-handler';

export default function EmptyExample() {
  return (
    <View style={styles.container}>
      <Text importantForAccessibility="no">
        Accessibility label inferring - RectButton
      </Text>
      <View style={styles.horizontal}>
        <RectButton accessible style={[styles.horizontal, styles.wideButton]}>
          <View style={styles.box} accessible accessibilityLabel="nested one" />
          <View
            style={styles.box}
            accessible
            importantForAccessibility="no"
            accessibilityLabel="nested two"
          />
          <View
            style={styles.box}
            importantForAccessibility="no"
            accessibilityLabel="nested three"
          />
        </RectButton>
      </View>
      <Text importantForAccessibility="no">
        Accessibility label inferring - RN Pressable
      </Text>
      <View style={styles.horizontal}>
        <Pressable accessible style={[styles.horizontal, styles.wideButton]}>
          <View style={styles.box} accessible accessibilityLabel="nested one" />
          <View
            style={styles.box}
            accessible
            importantForAccessibility="no"
            accessibilityLabel="nested two"
          />
          <View
            style={styles.box}
            importantForAccessibility="no"
            accessibilityLabel="nested three"
          />
        </Pressable>
      </View>
      <Text importantForAccessibility="no">View inside RectButton</Text>
      <View style={styles.horizontal}>
        <RectButton accessible accessibilityLabel="outer one">
          <Text importantForAccessibility="no">In & Out</Text>
          <View style={styles.box} accessible accessibilityLabel="nested one" />
          <View style={styles.box} accessible accessibilityLabel="nested two" />
        </RectButton>
        <RectButton>
          <Text importantForAccessibility="no">In only</Text>
          <View style={styles.box} accessible accessibilityLabel="alone one" />
          <View style={styles.box} accessible accessibilityLabel="alone two" />
        </RectButton>
        <RectButton accessible accessibilityLabel="alone three">
          <Text importantForAccessibility="no">Out only</Text>
          <View style={styles.box} />
          <View style={styles.box} />
        </RectButton>
        <RectButton
          importantForAccessibility="no"
          accessibilityLabel="should be ignored">
          <Text>Ignored</Text>
          <View style={styles.box} />
          <View style={styles.box} />
        </RectButton>
      </View>
      <Text importantForAccessibility="no">View inside RN Pressable</Text>
      <View style={styles.horizontal}>
        <Pressable accessible accessibilityLabel="outer one">
          <Text importantForAccessibility="no">In & Out</Text>
          <View style={styles.box} accessible accessibilityLabel="nested one" />
          <View style={styles.box} accessible accessibilityLabel="nested two" />
        </Pressable>
        <Pressable>
          <Text importantForAccessibility="no">In only</Text>
          <View style={styles.box} accessible accessibilityLabel="alone one" />
          <View style={styles.box} accessible accessibilityLabel="alone two" />
        </Pressable>
        <Pressable accessible accessibilityLabel="alone three">
          <Text importantForAccessibility="no">Out only</Text>
          <View style={styles.box} />
          <View style={styles.box} />
        </Pressable>
        <Pressable
          importantForAccessibility="no"
          accessibilityLabel="should be ignored">
          <Text>Ignored</Text>
          <View style={styles.box} />
          <View style={styles.box} />
        </Pressable>
      </View>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
    alignItems: 'center',
  },
  horizontal: {
    flexDirection: 'row',
    justifyContent: 'center',
    alignItems: 'center',
    gap: 5,
    marginVertical: 5,
  },
  box: {
    padding: 35,
    backgroundColor: 'coral',
    margin: 1,
  },
  wideButton: {
    backgroundColor: 'orange',
    padding: 10,
  },
  separator: {
    height: 1,
    width: 400,
    backgroundColor: 'black',
  },
});

@latekvo latekvo requested a review from m-bert December 17, 2024 14:22
Copy link
Contributor

@latekvo latekvo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost forgot, please add AccessibilityProps to the RawButtonProps typescript interface:

-export interface RawButtonProps extends NativeViewGestureHandlerProps
+export interface RawButtonProps extends NativeViewGestureHandlerProps, AccessibilityProps

Otherwise none of the accessibility props are available to the components derived from RNGestureHandlerButton

@oblador
Copy link
Contributor Author

oblador commented Dec 17, 2024

Done!

@j-piasecki j-piasecki merged commit ba942f7 into software-mansion:main Dec 18, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

4 participants