[Mobile] Complete Accessibility Audit #269
Conversation
Co-authored-by: Devasy <110348311+Devasy@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
✅ Deploy Preview for split-but-wiser canceled.
|
WalkthroughThis PR adds accessibility properties (accessibilityLabel, accessibilityRole, accessibilityHint, accessibilityState) across mobile screens and updates .Jules documentation to record a completed mobile accessibility audit and React Native accessibility patterns. Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mobile/screens/EditProfileScreen.js (1)
101-111:⚠️ Potential issue | 🟡 MinorAccessibility label doesn't match dynamic button text.
The
accessibilityLabelis static ("Change profile picture") but the visible button text changes dynamically between "Change Photo" and "Add Photo" based on whether an image is picked. Screen reader users may hear a different label than what's visually displayed, causing confusion.🔧 Proposed fix
<Button mode="outlined" onPress={pickImage} icon="camera" style={styles.imageButton} - accessibilityLabel="Change profile picture" + accessibilityLabel={pickedImage ? "Change profile picture" : "Add profile picture"} accessibilityRole="button" accessibilityHint="Opens your media library to select a new photo" > {pickedImage ? "Change Photo" : "Add Photo"} </Button>mobile/screens/GroupDetailsScreen.js (1)
106-122:⚠️ Potential issue | 🟡 MinorIncorrect
accessibilityRole="button"on non-interactive Card.The Card component has
accessibilityRole="button"but noonPresshandler, meaning it's not interactive. Screen readers will announce this as a button, misleading users into thinking they can interact with it when they cannot.🔧 Proposed fix - remove button role or add interactivity
If the card is purely informational, remove the button role:
<Card style={styles.card} - accessibilityRole="button" accessibilityLabel={`Expense: ${item.description}, Amount: ${formatCurrency( item.amount )}. Paid by ${getMemberName(item.paidBy || item.createdBy)}. ${balanceText}`} >Alternatively, if you intend to make the card tappable (e.g., to view expense details), add an
onPresshandler.
🤖 Fix all issues with AI agents
In `@mobile/screens/AccountScreen.js`:
- Around line 70-76: The Logout List.Item lacks an accessibilityHint to warn
users about the destructive action; update the logout component (the List.Item
with title "Logout" and onPress={handleLogout}) to include an accessibilityHint
prop (e.g., "Logs you out of the application") so screen readers convey the
consequence of the action; ensure the hint string is consistent with your
i18n/localization approach if applicable.
In `@mobile/screens/GroupSettingsScreen.js`:
- Around line 280-285: The IconButton for removing a member is missing an
accessibilityHint which would clarify the destructive action; update the
IconButton (the instance rendering icon="account-remove") to add an
accessibilityHint prop (e.g., using displayName or static text) that explains
the result like "Removes this member from the group" so screen readers convey
the action's consequence when invoking onKick(userId, displayName).
|
@jules In
In
|
Thank you for the detailed feedback! I have implemented the requested changes:
These additions will clarify the consequences of these destructive actions for screen reader users. |
Co-authored-by: Devasy <110348311+Devasy@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mobile/screens/GroupSettingsScreen.js (2)
317-329:⚠️ Potential issue | 🟡 MinorAdd
accessibilityStateto communicate selection status.The selected icon is visually indicated by
mode="contained"vs"outlined", but screen reader users won't know which icon is currently selected. AddaccessibilityStateto convey this state.♿ Proposed fix
<Button key={i} mode={icon === i ? "contained" : "outlined"} style={styles.iconBtn} onPress={() => setIcon(i)} disabled={!isAdmin} accessibilityLabel={`Select icon ${i}`} accessibilityRole="button" + accessibilityState={{ selected: icon === i }} >
359-370: 🧹 Nitpick | 🔵 TrivialConsider adding
accessibilityStatefor busy/disabled feedback.The button's
disabledandloadingstates should be communicated to assistive technologies. While react-native-paper may handledisabled, explicitly settingaccessibilityStateensures screen readers announce the busy state during save operations.♿ Suggested enhancement
<Button mode="contained" style={{ marginTop: 12 }} loading={saving} disabled={saving} onPress={onSave} accessibilityLabel="Save Changes" accessibilityRole="button" + accessibilityState={{ disabled: saving, busy: saving }} >
Devasy
deleted the
jules/mobile-accessibility-audit-2448891226254195206
branch
Implemented comprehensive accessibility improvements for the mobile application.
Changes:
accessibilityLabelto all interactive elements (buttons, inputs, list items) across all screens inmobile/screens/.accessibilityRole(button, checkbox, progressbar) to ensure correct element identification by screen readers.accessibilityHintfor destructive actions (e.g., leaving group, deleting group) and complex interactions.accessibilityStatefor custom checkbox implementations.accessibilityRole="progressbar".This ensures the mobile app is usable by screen reader users.
PR created automatically by Jules for task 2448891226254195206 started by @Devasy23
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.