[jules] ux: Complete skeleton loading for HomeScreen groups #271
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.
|
WalkthroughAdds a skeleton loading UI for HomeScreen: a new theme-aware SkeletonGroupCard component with a pulsing opacity animation and replaces the previous ActivityIndicator with multiple skeleton placeholders; updates changelog/todo metadata accordingly. 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
🤖 Fix all issues with AI agents
In @.Jules/todo.md:
- Around line 60-65: The completion date on the Todo item in .Jules/todo.md is
set to the future (Completed: 2026-02-04); update that line to the actual
completion date (e.g., 2026-02-02) or remove the "Completed:" line until it is
truly complete so the log is accurate; edit the checklist entry for "Complete
skeleton loading for HomeScreen groups" (the same block that lists Files:
mobile/screens/HomeScreen.js, mobile/components/SkeletonGroupCard.js) to reflect
the corrected date or omit the completion date entirely.
In `@mobile/components/SkeletonGroupCard.js`:
- Around line 9-24: The Animated.loop started in the useEffect for the opacity
animation never gets stopped, causing a memory leak; capture the returned
animation instance from Animated.loop(Animated.sequence(...)) into a variable
(e.g., anim or loopAnim) before calling .start(), and in the useEffect cleanup
return a function that calls .stop() on that saved animation instance (reference
the useEffect, Animated.loop, Animated.sequence, opacity and .start() to locate
the code) so the animation is explicitly stopped on unmount.
| - [x] **[ux]** Complete skeleton loading for HomeScreen groups | ||
| - Completed: 2026-02-04 | ||
| - Files: `mobile/screens/HomeScreen.js`, `mobile/components/SkeletonGroupCard.js` | ||
| - Context: Replaced ActivityIndicator with pulsing skeleton cards matching list layout | ||
| - Impact: Better loading experience, less jarring | ||
| - Size: ~40 lines | ||
| - Added: 2026-01-01 | ||
| - Size: ~70 lines |
There was a problem hiding this comment.
Fix the completion date (currently in the future).
“Completed: 2026-02-04” is after the PR creation date (2026-02-02) and today (2026-02-02), which makes the log inaccurate. Please set it to the actual completion date or remove it until completion.
🛠️ Possible fix
- - Completed: 2026-02-04
+ - Completed: 2026-02-02📝 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.
| - [x] **[ux]** Complete skeleton loading for HomeScreen groups | |
| - Completed: 2026-02-04 | |
| - Files: `mobile/screens/HomeScreen.js`, `mobile/components/SkeletonGroupCard.js` | |
| - Context: Replaced ActivityIndicator with pulsing skeleton cards matching list layout | |
| - Impact: Better loading experience, less jarring | |
| - Size: ~40 lines | |
| - Added: 2026-01-01 | |
| - Size: ~70 lines | |
| - [x] **[ux]** Complete skeleton loading for HomeScreen groups | |
| - Completed: 2026-02-02 | |
| - Files: `mobile/screens/HomeScreen.js`, `mobile/components/SkeletonGroupCard.js` | |
| - Context: Replaced ActivityIndicator with pulsing skeleton cards matching list layout | |
| - Impact: Better loading experience, less jarring | |
| - Size: ~70 lines |
🤖 Prompt for AI Agents
In @.Jules/todo.md around lines 60 - 65, The completion date on the Todo item in
.Jules/todo.md is set to the future (Completed: 2026-02-04); update that line to
the actual completion date (e.g., 2026-02-02) or remove the "Completed:" line
until it is truly complete so the log is accurate; edit the checklist entry for
"Complete skeleton loading for HomeScreen groups" (the same block that lists
Files: mobile/screens/HomeScreen.js, mobile/components/SkeletonGroupCard.js) to
reflect the corrected date or omit the completion date entirely.
| useEffect(() => { | ||
| Animated.loop( | ||
| Animated.sequence([ | ||
| Animated.timing(opacity, { | ||
| toValue: 0.7, | ||
| duration: 800, | ||
| useNativeDriver: true, | ||
| }), | ||
| Animated.timing(opacity, { | ||
| toValue: 0.3, | ||
| duration: 800, | ||
| useNativeDriver: true, | ||
| }), | ||
| ]) | ||
| ).start(); | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "SkeletonGroupCard.js" --type fRepository: Devasy/splitwiser
Length of output: 99
🏁 Script executed:
cat -n mobile/components/SkeletonGroupCard.jsRepository: Devasy/splitwiser
Length of output: 2752
🌐 Web query:
React Native Animated.loop cleanup memory leak unmount
💡 Result:
To avoid “leaks” (and React warnings) with Animated.loop(...) when a component unmounts, treat the loop like any other long‑running subscription: start it in an effect and stop it in the cleanup.
Why this matters
Animated.loopreturns a CompositeAnimation that can be stopped (and often reset) explicitly. The loop “resets and begins again” each iteration. [1]- While RN generally detaches animated props on unmount (often stopping animations implicitly), people have hit real retention issues in some scenarios—e.g., an
Animation.start()completion callback being retained when an animated component unmounts mid‑animation. [2] Stopping on unmount is the safest pattern.
Recommended cleanup pattern (hooks)
import { useEffect, useRef } from 'react';
import { Animated, Easing } from 'react-native';
export function useLoopingSpin() {
const value = useRef(new Animated.Value(0)).current;
const loopRef = useRef<Animated.CompositeAnimation | null>(null);
useEffect(() => {
const anim = Animated.loop(
Animated.timing(value, {
toValue: 1,
duration: 1000,
easing: Easing.linear,
useNativeDriver: true,
isInteraction: false, // helps avoid blocking VirtualizedList rendering while running [1]
})
);
loopRef.current = anim;
anim.start(); // avoid passing a completion callback unless you truly need it [2]
return () => {
loopRef.current?.stop(); // stop the loop explicitly
loopRef.current = null;
value.stopAnimation(); // extra safety: stop any driver/tracking on the value
value.setValue(0); // optional: reset for next mount
};
}, [value]);
return value;
}Also clean up listeners (common real leak source)
If you used value.addListener(...), remove it in the same cleanup (value.removeListener(id) / value.removeAllListeners()), since listeners are async callbacks that can outlive the screen. [3]
Sources: React Native Animated.loop docs (and isInteraction note) [1]; RN issue documenting callback retention when unmounting mid‑animation [2]; RN animations docs on addListener / async updates [3].
Stop the animation loop on unmount to prevent memory leaks.
The Animated.loop(...).start() call runs indefinitely without cleanup. On component unmount, the animation continues running in the background, retaining references and potentially blocking garbage collection. Store the animation reference and explicitly call .stop() in the effect cleanup function.
🧩 Proposed fix
useEffect(() => {
- Animated.loop(
+ const animation = Animated.loop(
Animated.sequence([
Animated.timing(opacity, {
toValue: 0.7,
duration: 800,
useNativeDriver: true,
}),
Animated.timing(opacity, {
toValue: 0.3,
duration: 800,
useNativeDriver: true,
}),
])
- ).start();
+ );
+ animation.start();
+ return () => animation.stop();
}, []);📝 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.
| useEffect(() => { | |
| Animated.loop( | |
| Animated.sequence([ | |
| Animated.timing(opacity, { | |
| toValue: 0.7, | |
| duration: 800, | |
| useNativeDriver: true, | |
| }), | |
| Animated.timing(opacity, { | |
| toValue: 0.3, | |
| duration: 800, | |
| useNativeDriver: true, | |
| }), | |
| ]) | |
| ).start(); | |
| }, []); | |
| useEffect(() => { | |
| const animation = Animated.loop( | |
| Animated.sequence([ | |
| Animated.timing(opacity, { | |
| toValue: 0.7, | |
| duration: 800, | |
| useNativeDriver: true, | |
| }), | |
| Animated.timing(opacity, { | |
| toValue: 0.3, | |
| duration: 800, | |
| useNativeDriver: true, | |
| }), | |
| ]) | |
| ); | |
| animation.start(); | |
| return () => animation.stop(); | |
| }, []); |
🤖 Prompt for AI Agents
In `@mobile/components/SkeletonGroupCard.js` around lines 9 - 24, The
Animated.loop started in the useEffect for the opacity animation never gets
stopped, causing a memory leak; capture the returned animation instance from
Animated.loop(Animated.sequence(...)) into a variable (e.g., anim or loopAnim)
before calling .start(), and in the useEffect cleanup return a function that
calls .stop() on that saved animation instance (reference the useEffect,
Animated.loop, Animated.sequence, opacity and .start() to locate the code) so
the animation is explicitly stopped on unmount.
Co-authored-by: Devasy <110348311+Devasy@users.noreply.github.com>
Replaces the generic ActivityIndicator in the mobile HomeScreen with a custom SkeletonGroupCard component. The new loader mimics the actual group card layout (Avatar + Title + Status) and uses a pulsing opacity animation. This improves perceived performance and provides a smoother loading experience. Updated todo.md and changelog.md accordingly.
PR created automatically by Jules for task 3290632159999934887 started by @Devasy23
Summary by CodeRabbit