[jules] ux: Complete skeleton loading for HomeScreen groups#299
[jules] ux: Complete skeleton loading for HomeScreen groups#299
Conversation
- Replaced native `ActivityIndicator` with modern skeleton cards - Created reusable `Skeleton` primitive using `Animated.loop` - Created `GroupListSkeleton` specifically for `HomeScreen` group layout - Improved perceived loading time and overall user experience - Updated `.Jules/todo.md` and `.Jules/changelog.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.
|
WalkthroughImplements skeleton loading for HomeScreen groups by introducing a reusable Skeleton component with pulsing animation and a GroupListSkeleton layout. Replaces ActivityIndicator with skeleton cards during data loading. Updates documentation accordingly. Changes
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 #299 +/- ##
=======================================
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mobile/screens/HomeScreen.js (1)
4-4:⚠️ Potential issue | 🟡 MinorRemove unused
ActivityIndicatorimport.
ActivityIndicatoris imported on line 4 but is not used anywhere in this file. It was replaced withGroupListSkeletonfor the loading state.Proposed fix
import { - ActivityIndicator, Appbar, Avatar, Modal,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/screens/HomeScreen.js` at line 4, Remove the unused ActivityIndicator import from the HomeScreen component import list: locate the import statement that includes "ActivityIndicator" in mobile/screens/HomeScreen.js and delete that identifier (or the entire import line if it only contains ActivityIndicator) so only used imports (like GroupListSkeleton) remain; ensure no other references to ActivityIndicator exist in the file before committing.
🤖 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 42-55: The Skeleton component's Animated.View is being announced
by screen readers; update the Animated.View in Skeleton to be hidden from the
accessibility tree by adding accessible={false},
accessibilityElementsHidden={true}, and
importantForAccessibility="no-hide-descendants" (place these props after
{...props} so they override any incoming accessibility props) so individual
placeholders are not announced while the parent GroupListSkeleton retains
accessibilityRole="progressbar" and accessibilityLabel="Loading groups".
---
Outside diff comments:
In `@mobile/screens/HomeScreen.js`:
- Line 4: Remove the unused ActivityIndicator import from the HomeScreen
component import list: locate the import statement that includes
"ActivityIndicator" in mobile/screens/HomeScreen.js and delete that identifier
(or the entire import line if it only contains ActivityIndicator) so only used
imports (like GroupListSkeleton) remain; ensure no other references to
ActivityIndicator exist in the file before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6c5ef5aa-bda9-4296-889f-28264fe827b6
📒 Files selected for processing (5)
.Jules/changelog.md.Jules/todo.mdmobile/components/skeletons/GroupListSkeleton.jsmobile/components/ui/Skeleton.jsmobile/screens/HomeScreen.js
| return ( | ||
| <Animated.View | ||
| style={[ | ||
| { | ||
| width, | ||
| height, | ||
| borderRadius, | ||
| backgroundColor: skeletonColor, | ||
| opacity: opacityAnim, | ||
| }, | ||
| style, | ||
| ]} | ||
| {...props} | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider hiding skeleton elements from accessibility tree.
Individual skeleton elements should not be announced by screen readers since the parent GroupListSkeleton already has accessibilityRole="progressbar" and accessibilityLabel="Loading groups". Adding accessibility props here prevents VoiceOver/TalkBack from announcing each placeholder.
♻️ Proposed fix
return (
<Animated.View
+ accessible={false}
+ importantForAccessibility="no-hide-descendants"
style={[
{
width,
height,
borderRadius,
backgroundColor: skeletonColor,
opacity: opacityAnim,
},
style,
]}
{...props}
/>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <Animated.View | |
| style={[ | |
| { | |
| width, | |
| height, | |
| borderRadius, | |
| backgroundColor: skeletonColor, | |
| opacity: opacityAnim, | |
| }, | |
| style, | |
| ]} | |
| {...props} | |
| /> | |
| return ( | |
| <Animated.View | |
| accessible={false} | |
| importantForAccessibility="no-hide-descendants" | |
| style={[ | |
| { | |
| width, | |
| height, | |
| borderRadius, | |
| backgroundColor: skeletonColor, | |
| opacity: opacityAnim, | |
| }, | |
| style, | |
| ]} | |
| {...props} | |
| /> |
🤖 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 42 - 55, The Skeleton
component's Animated.View is being announced by screen readers; update the
Animated.View in Skeleton to be hidden from the accessibility tree by adding
accessible={false}, accessibilityElementsHidden={true}, and
importantForAccessibility="no-hide-descendants" (place these props after
{...props} so they override any incoming accessibility props) so individual
placeholders are not announced while the parent GroupListSkeleton retains
accessibilityRole="progressbar" and accessibilityLabel="Loading groups".
1 participant
Replaced native
ActivityIndicatorwith modern skeleton cardsSkeletonprimitive usingAnimated.loopGroupListSkeletonspecifically forHomeScreengroup layout.Jules/todo.mdand.Jules/changelog.mdPR created automatically by Jules for task 11739445088885371149 started by @Devasy23
Summary by CodeRabbit