-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[NoQA] enforce accessibility rules in AI reviewer #85083
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| --- | ||
| ruleId: A11Y-1 | ||
| title: Interactive elements must have accessible labels | ||
| --- | ||
|
|
||
| ## [A11Y-1] Interactive elements must have accessible labels | ||
|
|
||
| ### Reasoning | ||
|
|
||
| Screen readers (VoiceOver/TalkBack) cannot convey the purpose of an interactive element without a text label. Icon-only buttons, image-only touchables, and components whose visible text is insufficient for context must provide `accessibilityLabel`. Without it, assistive technology announces the element as an unnamed control, making the app unusable for screen reader users. (WCAG 1.1.1, 4.1.2) | ||
|
|
||
| ### Incorrect | ||
|
|
||
| ```tsx | ||
| // Icon-only button with no label — screen reader says "button" | ||
| <Pressable onPress={onClose}> | ||
| <Icon src={Expensicons.Close} /> | ||
| </Pressable> | ||
|
|
||
| // Image-only touchable with no description | ||
| <TouchableOpacity onPress={openProfile}> | ||
| <Avatar source={avatarURL} /> | ||
| </TouchableOpacity> | ||
| ``` | ||
|
|
||
| ### Correct | ||
|
|
||
| ```tsx | ||
| <Pressable | ||
| onPress={onClose} | ||
| accessibilityLabel={translate('common.close')} | ||
| accessibilityRole="button" | ||
| > | ||
| <Icon src={Expensicons.Close} /> | ||
| </Pressable> | ||
|
|
||
| <TouchableOpacity | ||
| onPress={openProfile} | ||
| accessibilityLabel={translate('common.profile')} | ||
| accessibilityRole="imagebutton" | ||
| > | ||
| <Avatar source={avatarURL} /> | ||
| </TouchableOpacity> | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ### Review Metadata | ||
|
|
||
| Flag ONLY when ALL of these are true: | ||
|
|
||
| - Element is interactive (`Pressable`, `TouchableOpacity`, `TouchableWithoutFeedback`, `PressableWithFeedback`, `Button`, or has `onPress`/`onLongPress`) | ||
| - Element contains **no visible `<Text>` child** (icon-only, image-only, or SVG-only) | ||
| - Element has **no** `accessibilityLabel` prop | ||
|
|
||
| **DO NOT flag if:** | ||
|
|
||
| - Element has a `<Text>` child that clearly describes the action | ||
| - Element is explicitly hidden from accessibility (`accessible={false}`, `accessibilityElementsHidden={true}` on iOS, `importantForAccessibility="no"` on Android) | ||
| - Element is a list item wrapper where the child component handles its own accessibility | ||
|
|
||
| **Search Patterns** (hints for reviewers): | ||
| - `<Pressable` / `<TouchableOpacity` / `<TouchableWithoutFeedback` | ||
| - `<Icon` without sibling `<Text` | ||
| - `onPress` without `accessibilityLabel` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| --- | ||
| ruleId: A11Y-10 | ||
| title: Respect user text scaling preferences | ||
| --- | ||
|
|
||
| ## [A11Y-10] Respect user text scaling preferences | ||
|
|
||
| ### Reasoning | ||
|
|
||
| Users with low vision rely on system-level font size settings (iOS Dynamic Type / Android Font Size) to enlarge text. Setting `allowFontScaling={false}` or `maxFontSizeMultiplier={1}` disables this, making text unreadable for these users. Layouts must accommodate scaled text using flexible containers (`minHeight`, `flexWrap`) instead of fixed pixel heights. (WCAG 1.4.4) | ||
|
Comment on lines
+6
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we could restrict usage of |
||
|
|
||
| ### Incorrect | ||
|
|
||
| ```tsx | ||
| // Globally disabling font scaling | ||
| <Text allowFontScaling={false}>Account Balance</Text> | ||
|
|
||
| // Capping font scaling to prevent layout issues instead of fixing the layout | ||
| <Text maxFontSizeMultiplier={1}>$2,450.00</Text> | ||
|
|
||
| // Fixed height container that clips scaled text | ||
| <View style={{height: 40}}> | ||
| <Text style={{fontSize: 16}}>This text will be clipped at larger scales</Text> | ||
| </View> | ||
| ``` | ||
|
|
||
| ### Correct | ||
|
|
||
| ```tsx | ||
| // Text respects system scaling (default behavior — don't override it) | ||
| <Text>Account Balance</Text> | ||
|
|
||
| // Flexible container that accommodates scaled text | ||
| <View style={{minHeight: 40, paddingVertical: 8}}> | ||
| <Text style={{fontSize: 16}}> | ||
| This text grows with the container at larger scales | ||
| </Text> | ||
| </View> | ||
|
|
||
| // Acceptable: limiting scaling on tiny decorative text only (e.g., badge count) | ||
| <Text maxFontSizeMultiplier={1.5} style={styles.badgeCount}>3</Text> | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ### Review Metadata | ||
|
|
||
| Flag ONLY when ANY of these patterns is found: | ||
|
|
||
| - `allowFontScaling={false}` on `<Text>` or `<TextInput>` displaying user-facing content | ||
| - `maxFontSizeMultiplier={1}` effectively disabling scaling on readable text | ||
| - Fixed `height` on a container with text content and no `minHeight` or overflow accommodation | ||
|
|
||
| **DO NOT flag if:** | ||
|
|
||
| - `allowFontScaling={false}` on purely decorative text (icons rendered as text, single-character badges) | ||
| - `maxFontSizeMultiplier` set to a reasonable value (>= 1.5) to prevent extreme layout breakage | ||
| - Container uses `minHeight` instead of `height` | ||
| - Text is inside a component that manages scaling internally | ||
|
|
||
| **Search Patterns** (hints for reviewers): | ||
| - `allowFontScaling={false}` | ||
| - `maxFontSizeMultiplier={1}` | ||
| - `maxFontSizeMultiplier={1.0}` | ||
| - Fixed `height:` on text containers | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| --- | ||
| ruleId: A11Y-11 | ||
| title: Forms must have accessible labels, errors, and instructions | ||
| --- | ||
|
|
||
| ## [A11Y-11] Forms must have accessible labels, errors, and instructions | ||
|
|
||
| ### Reasoning | ||
|
|
||
| Screen reader users navigate forms field by field. Each input must be associated with a descriptive label so users know what to enter. While React Native uses `placeholder` as a fallback accessible name, it disappears once the user starts typing, leaving the field unlabeled. An explicit `accessibilityLabel` (or `accessibilityLabelledBy` on Android) persists regardless of input state. Error messages must be announced when they appear using `accessibilityLiveRegion` (Android) and `AccessibilityInfo.announceForAccessibility()` (iOS). (WCAG 1.3.1, 3.3.1, 3.3.2) | ||
|
Comment on lines
+6
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In ideal scenario, we'd enforce those kind of a11y requirements via component design system which serves as build blocks for most of UI like forms. This would be better in terms of other angles, not just pure a11y. What do you think is needed to be achieved in our app? |
||
|
|
||
| ### Incorrect | ||
|
|
||
| ```tsx | ||
| // Input with no accessible label — screen reader says "edit text" | ||
| <TextInput | ||
| value={email} | ||
| onChangeText={setEmail} | ||
| placeholder="Email" | ||
| /> | ||
|
|
||
| // Error not announced, not associated with input | ||
| <TextInput value={email} onChangeText={setEmail} /> | ||
| {emailError && <Text style={styles.error}>{emailError}</Text>} | ||
| ``` | ||
|
|
||
| ### Correct | ||
|
|
||
| ```tsx | ||
| // Input with accessible label | ||
| <TextInput | ||
| value={email} | ||
| onChangeText={setEmail} | ||
| placeholder="Email" | ||
| accessibilityLabel={translate('common.email')} | ||
| accessibilityHint={translate('common.enterYourEmail')} | ||
| /> | ||
|
|
||
| // Android: label linked to input via nativeID | ||
| <Text nativeID="emailLabel">{translate('common.email')}</Text> | ||
| <TextInput | ||
| value={email} | ||
| onChangeText={setEmail} | ||
| accessibilityLabelledBy="emailLabel" | ||
| /> | ||
|
|
||
| // Error announced via live region (Android) + announceForAccessibility (iOS) | ||
| <TextInput | ||
| value={email} | ||
| onChangeText={setEmail} | ||
| accessibilityLabel={translate('common.email')} | ||
| /> | ||
| {emailError && ( | ||
| <Text | ||
| style={styles.error} | ||
| accessibilityLiveRegion="assertive" | ||
| accessibilityRole="alert" | ||
| > | ||
| {emailError} | ||
| </Text> | ||
| )} | ||
|
|
||
| // iOS: announce error to VoiceOver | ||
| useEffect(() => { | ||
| if (emailError) { | ||
| AccessibilityInfo.announceForAccessibility(emailError); | ||
| } | ||
| }, [emailError]); | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ### Review Metadata | ||
|
|
||
| Flag ONLY when ANY of these patterns is found: | ||
|
|
||
| - `<TextInput>` with **no** `accessibilityLabel` and **no** `accessibilityLabelledBy` | ||
| - `<TextInput>` relying **only** on `placeholder` for labeling (placeholder disappears once user types, leaving the field unlabeled for screen readers) | ||
| - Form validation error text rendered without `accessibilityLiveRegion` or `accessibilityRole="alert"` | ||
|
|
||
| **DO NOT flag if:** | ||
|
|
||
| - Using a form component library that wraps inputs with labels internally | ||
| - `accessibilityLabel` is set on the input or a parent `accessible` container | ||
| - `accessibilityLabelledBy` links to a visible label via `nativeID` (Android-only — ensure `accessibilityLabel` is also set as iOS fallback) | ||
|
|
||
| **Search Patterns** (hints for reviewers): | ||
| - `<TextInput` without `accessibilityLabel` | ||
| - `placeholder=` as sole labeling mechanism | ||
| - Error text rendering without `accessibilityLiveRegion` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| --- | ||
| ruleId: A11Y-12 | ||
| title: Maintain logical focus order matching visual layout | ||
| --- | ||
|
|
||
| ## [A11Y-12] Maintain logical focus order matching visual layout | ||
|
|
||
| ### Reasoning | ||
|
|
||
| Screen readers traverse elements in JSX/DOM order, not visual order. When absolute positioning, `flexDirection: 'row-reverse'`, or `zIndex` rearranges elements visually, the screen reader focus order diverges from what sighted users see. This creates a confusing navigation experience where focus jumps unpredictably. JSX order must match the intended reading/interaction order. (WCAG 2.4.3, 1.3.2) | ||
|
|
||
| ### Incorrect | ||
|
|
||
| ```tsx | ||
| // Visual order: [Cancel] [Submit] but JSX order is reversed | ||
| <View style={{flexDirection: 'row-reverse'}}> | ||
| <Pressable onPress={onSubmit}><Text>Submit</Text></Pressable> | ||
| <Pressable onPress={onCancel}><Text>Cancel</Text></Pressable> | ||
| </View> | ||
|
|
||
| // Header visually on top via absolute positioning, but last in JSX | ||
| <View> | ||
| <View style={styles.content}><Text>Body content</Text></View> | ||
| <View style={[styles.header, {position: 'absolute', top: 0}]}> | ||
| <Text accessibilityRole="header">Title</Text> | ||
| </View> | ||
| </View> | ||
| ``` | ||
|
|
||
| ### Correct | ||
|
|
||
| ```tsx | ||
| // JSX order matches visual reading order | ||
| <View style={{flexDirection: 'row'}}> | ||
| <Pressable onPress={onCancel}><Text>Cancel</Text></Pressable> | ||
| <Pressable onPress={onSubmit}><Text>Submit</Text></Pressable> | ||
| </View> | ||
|
|
||
| // Header first in JSX, matching visual order | ||
| <View> | ||
| <View style={styles.header}> | ||
| <Text accessibilityRole="header">Title</Text> | ||
| </View> | ||
| <View style={styles.content}><Text>Body content</Text></View> | ||
| </View> | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ### Review Metadata | ||
|
|
||
| Flag ONLY when ANY of these patterns is found: | ||
|
|
||
| - `flexDirection: 'row-reverse'` or `'column-reverse'` on a container with multiple interactive children | ||
| - Absolute positioning causing an element to appear visually before its JSX siblings | ||
| - `zIndex` layering that places interactive elements in a different visual order than JSX order | ||
|
|
||
| **DO NOT flag if:** | ||
|
|
||
| - `row-reverse` is used on a container with a single child or non-interactive children | ||
| - Visual reordering is purely decorative (no interactive elements affected) | ||
| - The component uses `experimental_accessibilityOrder` to explicitly control focus order | ||
|
|
||
| **Search Patterns** (hints for reviewers): | ||
| - `flexDirection: 'row-reverse'` / `'column-reverse'` | ||
| - `position: 'absolute'` on interactive elements | ||
| - `zIndex` on containers with multiple interactive children |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| --- | ||
| ruleId: A11Y-2 | ||
| title: Use semantic accessibilityRole for interactive elements | ||
| --- | ||
|
|
||
| ## [A11Y-2] Use semantic accessibilityRole for interactive elements | ||
|
|
||
| ### Reasoning | ||
|
|
||
| React Native components have no implicit semantic meaning — assistive technology treats them as generic containers. Interactive elements must declare their role via `accessibilityRole` or `role` so screen readers can convey what the element does. Note: `role` is a cross-platform alias that takes precedence over `accessibilityRole` when both are set. (WCAG 4.1.2) | ||
|
|
||
| ### Incorrect | ||
|
|
||
| ```tsx | ||
| // Pressable with no role — screen reader doesn't convey it's a button | ||
| <Pressable onPress={handleSubmit}> | ||
| <Text>Submit</Text> | ||
| </Pressable> | ||
|
|
||
| // Pressable acting as link with no role | ||
| <Pressable onPress={() => openURL(href)}> | ||
| <Text style={styles.link}>Learn more</Text> | ||
| </Pressable> | ||
|
|
||
| // Section heading with no role | ||
| <Text style={styles.heading}>Account Settings</Text> | ||
| ``` | ||
|
|
||
| ### Correct | ||
|
|
||
| ```tsx | ||
| <Pressable | ||
| accessibilityRole="button" | ||
| onPress={handleSubmit} | ||
| > | ||
|
Comment on lines
+32
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Examples use Btw, how this one overlaps with |
||
| <Text>Submit</Text> | ||
| </Pressable> | ||
|
|
||
| <Pressable | ||
| accessibilityRole="link" | ||
| onPress={() => openURL(href)} | ||
| > | ||
| <Text style={styles.link}>Learn more</Text> | ||
| </Pressable> | ||
|
|
||
| <Text | ||
| accessibilityRole="header" | ||
| style={styles.heading} | ||
| > | ||
| Account Settings | ||
| </Text> | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ### Review Metadata | ||
|
|
||
| Flag ONLY when ANY of these patterns is found: | ||
|
|
||
| - `<Pressable>` or interactive component with `onPress`/`onLongPress` handler but **no** `accessibilityRole` or `role` prop | ||
| - `<Pressable>` or `<TouchableOpacity>` navigating to a URL without `accessibilityRole="link"` | ||
| - Text styled as a heading (large/bold, section title) without `accessibilityRole="header"` | ||
| - Toggle/switch UI without `accessibilityRole="switch"` or `"checkbox"` | ||
| - Tab UI without `accessibilityRole="tab"` | ||
|
|
||
| **DO NOT flag if:** | ||
|
|
||
| - Component already has `accessibilityRole` or `role` set | ||
| - Using a design system component that sets the role internally (e.g., `<Button>`, `<Switch>`, `<Checkbox>`) | ||
| - Element is not interactive and not a semantic landmark | ||
|
|
||
| **Search Patterns** (hints for reviewers): | ||
| - `onPress` without `accessibilityRole` or `role` | ||
| - `<Pressable` without `accessibilityRole` | ||
| - Heading-styled `<Text` without `accessibilityRole="header"` | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the general priority of "static linting first, ai reviewer last" - looks like this rule might overlap with
react-native-a11y/has-valid-accessibility-descriptorswe have enabled.Maybe we could narrow it down, so there are no redundant violations coming both from eslint and reviewer. Thanks!