aryella-lacerda / react-native-accessibility-engine

Make accessibility-related assertions on React Native code using React Test Renderer
MIT License
163 stars 13 forks source link

Accessibility Props flagged incorrectly for tabs and carousels #307

Open heathercodes opened 1 year ago

heathercodes commented 1 year ago

Describe the bug I'm building a TabBar component using a Carousel library, and noticed that the toBeAccessible RTL helper is calling out the wrong things:

  1. "tab" role is flagged as incorrect my first attempt at building a TabBar component used react-native-tab-view with a Custom header and tabs. While testing the component, I noticed, that while adding accessibilityRole of tablist to the header component was flagged as correct, the TabBar items, the tabs themselves with an accessibilityRole of tab was incorrect. This library wanted me to change it to button.

  2. "accessibilityState" is required for carousels I moved onto another library (for unrelated reasons to the above) and while using a carousel library react-native-reanimated-carousel. While this library has other accessibility issues I won't get into, this library called out that an accessibilityState with disabled boolean prop was required to be passed all the way to the base View component. While doing some research, however, I noted that other accessible carousel libraries do not do this, but instead have other accessibility props:

Component under test 1. use the basic set up of: https://github.com/satya164/react-native-tab-view#quick-start

      const FirstRoute = () => (
        <View style={{ flex: 1, backgroundColor: '#ff4081' }} />
      );

      const SecondRoute = () => (
        <View style={{ flex: 1, backgroundColor: '#673ab7' }} />
      );

      const renderScene = SceneMap({
        first: FirstRoute,
        second: SecondRoute,
      });

      function TabViewExample() {
        const layout = useWindowDimensions();

        const [index, setIndex] = React.useState(0);
        const [routes] = React.useState([
          { key: 'first', title: 'First' },
          { key: 'second', title: 'Second' },
        ]);

        return (
          <TabView
            navigationState={{ index, routes }}
            renderScene={renderScene}
            onIndexChange={setIndex}
            initialLayout={{ width: layout.width }}
          />
        );
      }

      render(<TabViewExample />);

      expect(screen.container).toBeAccessible();

and this will flag:

MobileProviderStack > ProviderStack > ThemeContainer > DarkModeProvider > ThemeProvider > IntlContainer > IntlProvider > InjectIntl(Component) > TabViewExample > TabViewExample > TabView > View > PagerViewAdapter > TabBar > AnimatedComponent > View > View > AnimatedComponent > FlatList > VirtualizedList > VirtualizedListContextProvider > ScrollView > RCTScrollView > View > CellRenderer > VirtualizedListCellContextProvider > View > TabBarItem > TabBarItemInternal > PlatformPressable > Pressable
 |
 |     Problem:  "This component is pressable but the user hasn't been informed that it behaves like a button/link/radio"
 |     Solution: "Set the 'accessibilityRole' prop to button or link or imagebutton or radio"

2. use the basic set up of: https://github.com/dohooo/react-native-reanimated-carousel#usage

function MyCarousel() {
      const width = Dimensions.get('window').width;
      return (
        <View style={{ flex: 1 }}>
          <Carousel
            loop
            width={width}
            height={width / 2}
            autoPlay={true}
            data={[...new Array(6).keys()]}
            scrollAnimationDuration={1000}
            onSnapToItem={(index) => console.log('current index:', index)}
            renderItem={({ index }) => (
              <View
                style={{
                  flex: 1,
                  borderWidth: 1,
                  justifyContent: 'center',
                }}
              >
                <Text style={{ textAlign: 'center', fontSize: 30 }}>
                  {index}
                </Text>
              </View>
            )}
          />
        </View>
      );
    }

    render(<MyCarousel />);

    expect(screen.container).toBeAccessible();

and this will flag:

 MobileProviderStack > ProviderStack > ThemeContainer > DarkModeProvider > ThemeProvider > IntlContainer > IntlProvider > InjectIntl(Component) > MyCarousel > MyCarousel > View > IScrollViewGesture > PanGestureHandler
 |
 |     Problem:  "This component has a disabled state but it isn't exposed to the user"
 |     Solution: "Set the 'accessibilityState' prop to an object containing a boolean 'disabled' key"

Expected behavior

  1. when using tablist, I would expect "tab" roles of its children not to be flagged as incorrect by this library.
  2. when using a carousel library, accessibilityState should not be the only prop flagged if other props (accessible, accessibilityElementsHidden, etc.) already exist.

Context

aryella-lacerda commented 1 year ago

Hello, @heathercodes! Thanks for reaching out about these issues.

  1. The rule that's being triggered is pressable-role-required. It's a sanity check, but we haven't added tab to the list of reasonable pressable roles. This seems like a simple fix, if you'd like to open a PR. Otherwise, I can get to it this weekend.
  2. This one requires a little research. 🤔 I hadn't taken carousels into consideration when I wrote the existing rules. There are a few guidelines that I'll link here for reference, but I'll get back to you when I know more!
heathercodes commented 1 year ago

thank you! I'll make a PR for issue 1 shortly

heathercodes commented 1 year ago

Ready for your review! https://github.com/aryella-lacerda/react-native-accessibility-engine/pull/308

heathercodes commented 1 year ago

hey @aryella-lacerda wondered if you had looked into the accessibility for carousels at all?

on my end, while implementing the carousel for my app, in the component that wraps each item in the carousel, I tried making it accessible by using accessibilityElementsHidden and importantForAccessibility. Wondered if this made sense to you?

<View
      key={key || `${index}-content`}
      accessibilityElementsHidden={index !== selectedIndex}
      importantForAccessibility={
        index === selectedIndex ? 'auto' : 'no-hide-descendants'
      }
    >
      {children} // carousel item here
</View>

If you hadn't had a chance to look into it, that's also ok! we can likely close up this issue in any event.

Thanks!