[jules] refactor: Use generic Skeleton component in FriendsScreen#305
[jules] refactor: Use generic Skeleton component in FriendsScreen#305
Conversation
Extracted the inline skeleton animation logic from FriendsScreen into a highly reusable `Skeleton` primitive in `mobile/components/ui/`. Refactored `FriendsScreen` to import and utilize the new generic component to produce a consistent and visually identical loading state while supporting light and dark themes out-of-the-box. 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.
|
WalkthroughA new reusable Changes
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 #305 +/- ##
=======================================
Coverage ? 74.43%
=======================================
Files ? 17
Lines ? 1635
Branches ? 144
=======================================
Hits ? 1217
Misses ? 367
Partials ? 51
🚀 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 7-27: The Skeleton component currently creates its own
Animated.loop via the opacityAnim useRef and useEffect, causing many concurrent
loops; change Skeleton to accept an optional shared animated value prop (e.g.,
sharedOpacityAnim) and, in the component (identify opacityAnim and useEffect
that calls Animated.loop/loop.start/loop.stop), use the passed sharedOpacityAnim
when provided and only create the local Animated.Value and loop when no shared
value is supplied; ensure the shared loop is started/stopped in a single central
place (or lazily once in a module-level initializer) while keeping the current
local fallback behavior so existing usage still works.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b0418a4b-f54d-425a-8328-a901094571d2
📒 Files selected for processing (2)
mobile/components/ui/Skeleton.jsmobile/screens/FriendsScreen.js
| const opacityAnim = useRef(new Animated.Value(0.3)).current; | ||
|
|
||
| useEffect(() => { | ||
| const loop = Animated.loop( | ||
| Animated.sequence([ | ||
| Animated.timing(opacityAnim, { | ||
| toValue: 1, | ||
| duration: 700, | ||
| useNativeDriver: true, | ||
| }), | ||
| Animated.timing(opacityAnim, { | ||
| toValue: 0.3, | ||
| duration: 700, | ||
| useNativeDriver: true, | ||
| }), | ||
| ]) | ||
| ); | ||
| loop.start(); | ||
|
|
||
| return () => loop.stop(); | ||
| }, [opacityAnim]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'new Animated\.Value|Animated\.loop|useEffect' mobile/components/ui/Skeleton.js
rg -n -C2 '<Skeleton' mobile/screens/FriendsScreen.js
rg -n -C2 'Array\.from\(\{ length: 5 \}\)' mobile/screens/FriendsScreen.jsRepository: Devasy/splitwiser
Length of output: 1255
🏁 Script executed:
rg -n 'import.*Skeleton|<Skeleton' mobile/screens/ --type jsRepository: Devasy/splitwiser
Length of output: 537
Share animation state instead of starting one pulse loop per Skeleton instance.
Each mounted Skeleton starts its own infinite Animated.loop. In FriendsScreen's loading state, 5 rows render 3 Skeleton components each, resulting in 15 concurrent animation loops. This adds avoidable animation overhead and can impact smoothness on lower-end devices.
⚙️ Proposed refactor (allow shared animated value, keep local fallback)
-const Skeleton = ({ width, height, borderRadius, style }) => {
+const Skeleton = ({ width, height, borderRadius, style, animatedOpacity }) => {
const theme = useTheme();
- const opacityAnim = useRef(new Animated.Value(0.3)).current;
+ const localOpacityAnim = useRef(new Animated.Value(0.3)).current;
+ const opacityAnim = animatedOpacity ?? localOpacityAnim;
useEffect(() => {
+ if (animatedOpacity) return undefined;
const loop = Animated.loop(
Animated.sequence([
Animated.timing(opacityAnim, {
toValue: 1,
duration: 700,
useNativeDriver: true,
}),
Animated.timing(opacityAnim, {
toValue: 0.3,
duration: 700,
useNativeDriver: true,
}),
])
);
loop.start();
return () => loop.stop();
- }, [opacityAnim]);
+ }, [animatedOpacity, localOpacityAnim]);🤖 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 7 - 27, The Skeleton component
currently creates its own Animated.loop via the opacityAnim useRef and
useEffect, causing many concurrent loops; change Skeleton to accept an optional
shared animated value prop (e.g., sharedOpacityAnim) and, in the component
(identify opacityAnim and useEffect that calls
Animated.loop/loop.start/loop.stop), use the passed sharedOpacityAnim when
provided and only create the local Animated.Value and loop when no shared value
is supplied; ensure the shared loop is started/stopped in a single central place
(or lazily once in a module-level initializer) while keeping the current local
fallback behavior so existing usage still works.
1 participant
Extracted the inline skeleton animation logic from FriendsScreen into a highly reusable
Skeletonprimitive inmobile/components/ui/. RefactoredFriendsScreento import and utilize the new generic component to produce a consistent and visually identical loading state while supporting light and dark themes out-of-the-box.PR created automatically by Jules for task 6500990729041499595 started by @Devasy23
Summary by CodeRabbit
New Features
Refactor