Conversation
- Created reusable `Skeleton` component with pulsing animation in `mobile/components/ui/Skeleton.js`. - Created `GroupListSkeleton` component in `mobile/components/skeletons/GroupListSkeleton.js` to mimic the home screen layout. - Replaced `ActivityIndicator` in `mobile/screens/HomeScreen.js` with the new skeleton loader. - Updated tracking files (`todo.md`, `changelog.md`, `knowledge.md`). Co-authored-by: Devasy23 <110348311+Devasy23@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.
|
WalkthroughIntroduces a Mobile Skeleton Loading feature with a reusable Skeleton component featuring pulsing animation and theme support. Adds GroupListSkeleton to display loading group cards, replaces ActivityIndicator in HomeScreen with skeleton loader, and updates documentation to reflect the implementation. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #291 +/- ##
=======================================
Coverage ? 63.55%
=======================================
Files ? 21
Lines ? 2456
Branches ? 254
=======================================
Hits ? 1561
Misses ? 831
Partials ? 64
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mobile/components/ui/Skeleton.js`:
- Around line 30-43: The Skeleton component's Animated.View currently sets
accessibilityRole="progressbar" and accessibilityLabel="Loading..." which causes
excessive screen reader announcements; remove both accessibilityRole and
accessibilityLabel from the Animated.View in mobile/components/ui/Skeleton.js
(the Animated.View instance rendering the skeleton with style including
opacity/width/height/borderRadius/backgroundColor) so individual skeletons are
treated as decorative and rely on the container's single "Loading groups"
announcement for accessibility.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.Jules/changelog.md.Jules/knowledge.md.Jules/todo.mdmobile/components/skeletons/GroupListSkeleton.jsmobile/components/ui/Skeleton.jsmobile/screens/HomeScreen.js
| <Animated.View | ||
| style={[ | ||
| { | ||
| opacity, | ||
| width, | ||
| height, | ||
| borderRadius, | ||
| backgroundColor: theme.colors.surfaceVariant, | ||
| }, | ||
| style, | ||
| ]} | ||
| accessibilityRole="progressbar" | ||
| accessibilityLabel="Loading..." | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find mobile -name "Skeleton.js" -type fRepository: Devasy/splitwiser
Length of output: 93
🏁 Script executed:
cat -n mobile/components/ui/Skeleton.jsRepository: Devasy/splitwiser
Length of output: 1476
🏁 Script executed:
find mobile -name "*GroupList*" -o -name "*Skeleton*" | head -20Repository: Devasy/splitwiser
Length of output: 142
🏁 Script executed:
cat -n mobile/components/skeletons/GroupListSkeleton.jsRepository: Devasy/splitwiser
Length of output: 1315
🌐 Web query:
React Native accessible={false} importantForAccessibility="no" accessibilityElementsHidden
💡 Result:
-
accessible={false}: only means the View itself is not treated as a single “accessibility element” (iOS:isAccessibilityElement, Android:focusable). It does not automatically remove the view (or its children) from the screen reader’s accessibility tree. [1] -
importantForAccessibility="no"(Android): tells Android accessibility services the view itself is not important (won’t be reported), but it does not hide descendants. To hide a view and all children on Android, useimportantForAccessibility="no-hide-descendants". [1][2] -
accessibilityElementsHidden(iOS): whentrue, hides the view and any accessibility elements it contains from VoiceOver. This is essentially the iOS equivalent of Android’simportantForAccessibility="no-hide-descendants". [1][2]
Practical mapping (hide subtree from screen readers)
<View
accessibilityElementsHidden={true} // iOS: hide subtree
importantForAccessibility="no-hide-descendants" // Android: hide subtree
>
...
</View>Using accessible={false} and/or importantForAccessibility="no" is not sufficient if your goal is “screen reader should ignore this whole section.” [1][2]
Sources: React Native Accessibility docs [1]; React Native Testing Library notes on what actually makes elements inaccessible [2].
🌐 Web query:
React Native accessibility best practices skeleton loading screen reader announcement
💡 Result:
Best practices (React Native) for an accessible skeleton loading state + screen reader announcement
-
Don’t expose the skeleton UI to screen readers
- Treat skeleton blocks as decorative placeholders and hide them from accessibility (so they don’t become dozens of meaningless focus stops). Common guidance is to hide skeletons and instead expose one loading message/state on a parent region. [4], [7]
-
Expose a single “loading” state on the real content container
- Put the container that will eventually hold the loaded content into a “busy/loading” state:
accessibilityState={{ busy: true }}while loading, thenbusy: falsewhen done. (RN supportsbusyinaccessibilityState.) [1]
- Optionally pair with a meaningful role/label such as:
accessibilityRole="progressbar"(or a status-like pattern) andaccessibilityLabel="Loading results"so the purpose is clear. [1], [2]
- Put the container that will eventually hold the loaded content into a “busy/loading” state:
-
Announce loading changes via “live region” where possible (Android)
- On Android, RN supports
accessibilityLiveRegion="polite"/"assertive"to announce updates when text changes. Prefer polite for loading. [1] - Note:
accessibilityLiveRegionis Android-focused and has had consistency gaps across platforms. [9]
- On Android, RN supports
-
Use direct announcements sparingly
AccessibilityInfo.announceForAccessibility(...)exists, but some platform docs caution against relying on forced announcements and recommend semantic/live-region approaches where possible. [2]
Minimal pattern (container announces; skeleton hidden)
import {AccessibilityInfo, Platform, View, Text} from "react-native";
function Screen({loading}: {loading: boolean}) {
// Optional: only when loading is long enough / user-triggered
// if (loading) AccessibilityInfo.announceForAccessibility("Loading content");
// else AccessibilityInfo.announceForAccessibility("Content loaded");
return (
<View
accessible
accessibilityState={{ busy: loading }}
accessibilityLabel={loading ? "Loading content" : "Content"}
>
{/* Android: live region announcement when this text changes */}
<Text accessibilityLiveRegion={Platform.OS === "android" ? "polite" : undefined}>
{loading ? "Loading…" : ""}
</Text>
{loading ? (
// Skeleton UI: hide from screen readers (decorative)
<View importantForAccessibility="no-hide-descendants">
{/* skeleton blocks here */}
</View>
) : (
<View>{/* real content here */}</View>
)}
</View>
);
}Key idea: one clear, non-spammy loading message/state on the container (busy + label / live update), while keeping skeleton visuals out of the accessibility tree. [1], [4], [7]
Remove individual accessibility announcements from decorative skeletons.
Each skeleton element exposes itself as a progressbar with "Loading..." label. With ~15 skeleton elements across the list (5 cards × 3 skeletons each), this creates excessive screen reader noise. The container already provides a single "Loading groups" announcement, which is sufficient.
Remove the accessibility role and label from individual skeletons since they are decorative placeholders:
🔧 Suggested adjustment
<Animated.View
style={[
{
opacity,
width,
height,
borderRadius,
backgroundColor: theme.colors.surfaceVariant,
},
style,
]}
- accessibilityRole="progressbar"
- accessibilityLabel="Loading..."
+ importantForAccessibility="no-hide-descendants"
+ accessibilityElementsHidden={true}
/>Note: If simply removing the role/label is preferred, use both props above for proper cross-platform support (Android + iOS).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/components/ui/Skeleton.js` around lines 30 - 43, The Skeleton
component's Animated.View currently sets accessibilityRole="progressbar" and
accessibilityLabel="Loading..." which causes excessive screen reader
announcements; remove both accessibilityRole and accessibilityLabel from the
Animated.View in mobile/components/ui/Skeleton.js (the Animated.View instance
rendering the skeleton with style including
opacity/width/height/borderRadius/backgroundColor) so individual skeletons are
treated as decorative and rely on the container's single "Loading groups"
announcement for accessibility.
1 participant
Implemented a skeleton loading state for the mobile Home Screen to improve perceived performance and visual consistency. Replaced the generic
ActivityIndicatorwith aGroupListSkeletonthat mimics the layout of the group cards, including avatars and status text. Created a reusable baseSkeletoncomponent usingAnimatedfor pulsing effects andreact-native-papertheming.PR created automatically by Jules for task 1918836578316525257 started by @Devasy23
Summary by CodeRabbit
New Features
Documentation