[jules] ux: Complete skeleton loading for HomeScreen groups#316
[jules] ux: Complete skeleton loading for HomeScreen groups#316
Conversation
- Implemented `Skeleton` primitive component in `mobile/components/ui/Skeleton.js` utilizing `react-native-reanimated` for smooth pulsing animations. - Created `GroupListSkeleton` in `mobile/components/skeletons/GroupListSkeleton.js` designed to mimic `react-native-paper`'s `Card` elements for a seamless transition. - Replaced the generic `ActivityIndicator` in `mobile/screens/HomeScreen.js` with the new `GroupListSkeleton`. - Applied proper accessibility role (`progressbar`) to loading states. - Handled animation lifecycle correctly to avoid memory leaks. - Updated `todo.md` and `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.
|
WalkthroughThis PR introduces skeleton loading UI for the Groups list on the HomeScreen. It adds two new React Native components using react-native-reanimated for pulsing animations, updates HomeScreen to use the skeleton instead of an activity indicator, adds required dependencies, and documents the changes in project files. 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 #316 +/- ##
=======================================
Coverage ? 63.55%
=======================================
Files ? 21
Lines ? 2456
Branches ? 254
=======================================
Hits ? 1561
Misses ? 831
Partials ? 64
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mobile/screens/HomeScreen.js (2)
4-4: 🧹 Nitpick | 🔵 TrivialRemove unused
ActivityIndicatorimport.The
ActivityIndicatorimport is no longer used after switching toGroupListSkeleton.🧹 Proposed cleanup
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 import list in the HomeScreen component (it's no longer used after switching to GroupListSkeleton); update the import statement so it no longer references ActivityIndicator and keep only the currently used symbols (e.g., GroupListSkeleton) to clean up dead imports.
291-295: 🧹 Nitpick | 🔵 TrivialRemove unused
loaderContainerstyle.The
loaderContainerstyle was used for the previousActivityIndicatorand is now dead code.🧹 Proposed cleanup
container: { flex: 1, }, - loaderContainer: { - flex: 1, - justifyContent: "center", - alignItems: "center", - }, list: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/screens/HomeScreen.js` around lines 291 - 295, Remove the dead `loaderContainer` style from the styles object in HomeScreen.js: locate the `loaderContainer` key in the exported styles (the object where other styles like `container`, etc. live) and delete that property since the ActivityIndicator that referenced it was removed; also run a quick search for `loaderContainer` to ensure there are no remaining references and remove them if found.
🤖 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/skeletons/GroupListSkeleton.js`:
- Around line 20-30: The skeleton layout uses a separate Card.Content block for
the status which causes a layout shift; update GroupListSkeleton.js to put the
status Skeleton into the Card.Title's subtitle prop (so Card.Title renders
title={<Skeleton .../>} subtitle={<Skeleton style={styles.statusSkeleton}
.../>}) keep the existing left={(props)=>...} avatar skeleton, and remove the
Card.Content block; ensure you use the same styles.statusSkeleton sizing and
borderRadius as currently defined so the skeleton matches the HapticCard.Title
subtitle structure seen in HomeScreen.js.
In `@mobile/components/ui/Skeleton.js`:
- Line 2: The import line in the Skeleton component unnecessarily includes View;
remove View from the import statement (i.e., change "import { View, StyleSheet }
from 'react-native';" to only import StyleSheet) so the unused symbol is not
imported in the Skeleton component.
In `@mobile/package.json`:
- Around line 25-27: Add a root-level Babel config to enable the
react-native-reanimated plugin so worklet functions in Skeleton.js compile:
create a babel.config.js exporting presets ['babel-preset-expo'] and plugins
['react-native-reanimated/plugin'] to ensure APIs used in Skeleton.js
(useSharedValue, useAnimatedStyle, withRepeat, withTiming, withSequence,
cancelAnimation) run correctly; also audit react-native-gesture-handler usage
and remove it if it is only present as a peer dependency and not used elsewhere.
---
Outside diff comments:
In `@mobile/screens/HomeScreen.js`:
- Line 4: Remove the unused ActivityIndicator import from the import list in the
HomeScreen component (it's no longer used after switching to GroupListSkeleton);
update the import statement so it no longer references ActivityIndicator and
keep only the currently used symbols (e.g., GroupListSkeleton) to clean up dead
imports.
- Around line 291-295: Remove the dead `loaderContainer` style from the styles
object in HomeScreen.js: locate the `loaderContainer` key in the exported styles
(the object where other styles like `container`, etc. live) and delete that
property since the ActivityIndicator that referenced it was removed; also run a
quick search for `loaderContainer` to ensure there are no remaining references
and remove them if found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56675951-935b-461e-8c50-12eb27703bdd
⛔ Files ignored due to path filters (1)
mobile/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
.Jules/changelog.md.Jules/todo.mdmobile/components/skeletons/GroupListSkeleton.jsmobile/components/ui/Skeleton.jsmobile/package.jsonmobile/screens/HomeScreen.js
| <Card.Title | ||
| title={<Skeleton width={120} height={20} />} | ||
| left={(props) => ( | ||
| <View {...props}> | ||
| <Skeleton width={40} height={40} borderRadius={20} /> | ||
| </View> | ||
| )} | ||
| /> | ||
| <Card.Content> | ||
| <Skeleton width={150} height={16} style={styles.statusSkeleton} /> | ||
| </Card.Content> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Skeleton layout may not match the actual card structure.
Looking at HomeScreen.js (lines 194-207 in context), the actual group cards use HapticCard.Title with a subtitle prop for the settlement status, not a separate Card.Content block. The skeleton here uses Card.Content which may cause a slight layout shift when data loads.
Consider using the subtitle prop on Card.Title to better match the actual layout:
♻️ Proposed fix to match actual card layout
<Card key={item} style={styles.card}>
<Card.Title
title={<Skeleton width={120} height={20} />}
+ subtitle={<Skeleton width={150} height={16} style={styles.statusSkeleton} />}
left={(props) => (
<View {...props}>
<Skeleton width={40} height={40} borderRadius={20} />
</View>
)}
/>
- <Card.Content>
- <Skeleton width={150} height={16} style={styles.statusSkeleton} />
- </Card.Content>
</Card>#!/bin/bash
# Verify the actual card structure in HomeScreen.js
echo "=== Checking HapticCard structure in HomeScreen.js ==="
rg -A 20 'HapticCard.Title' mobile/screens/HomeScreen.js🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/components/skeletons/GroupListSkeleton.js` around lines 20 - 30, The
skeleton layout uses a separate Card.Content block for the status which causes a
layout shift; update GroupListSkeleton.js to put the status Skeleton into the
Card.Title's subtitle prop (so Card.Title renders title={<Skeleton .../>}
subtitle={<Skeleton style={styles.statusSkeleton} .../>}) keep the existing
left={(props)=>...} avatar skeleton, and remove the Card.Content block; ensure
you use the same styles.statusSkeleton sizing and borderRadius as currently
defined so the skeleton matches the HapticCard.Title subtitle structure seen in
HomeScreen.js.
| @@ -0,0 +1,61 @@ | |||
| import React, { useEffect } from 'react'; | |||
| import { View, StyleSheet } from 'react-native'; | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused View import.
View is imported but not used in this component.
🧹 Proposed cleanup
-import { View, StyleSheet } from 'react-native';
+import { StyleSheet } from 'react-native';📝 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.
| import { View, StyleSheet } from 'react-native'; | |
| import { StyleSheet } from 'react-native'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/components/ui/Skeleton.js` at line 2, The import line in the Skeleton
component unnecessarily includes View; remove View from the import statement
(i.e., change "import { View, StyleSheet } from 'react-native';" to only import
StyleSheet) so the unused symbol is not imported in the Skeleton component.
| "react-native-gesture-handler": "~2.28.0", | ||
| "react-native-paper": "^5.14.5", | ||
| "react-native-reanimated": "~4.1.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if babel.config.js exists and contains the reanimated plugin
echo "=== Checking babel.config.js for reanimated plugin ==="
fd -t f 'babel.config.js' mobile --exec cat {}
echo ""
echo "=== Checking if gesture-handler is used anywhere in the codebase ==="
rg -l 'react-native-gesture-handler' --type js mobile/Repository: Devasy/splitwiser
Length of output: 185
🏁 Script executed:
#!/bin/bash
# Check for babel config at repository root
echo "=== Checking for babel.config.js at repository root ==="
cat babel.config.js 2>/dev/null || echo "Not found at root"
echo ""
echo "=== Checking for alternative babel config files ==="
fd -t f '(babel\.config|\.babelrc)' --type f
echo ""
echo "=== Viewing complete mobile/package.json to see context of changes ==="
cat -n mobile/package.json | head -40
echo ""
echo "=== Check if Skeleton.js uses reanimated APIs ==="
rg -A 5 -B 5 'useSharedValue|useAnimatedStyle|withRepeat' mobile/ --type js || echo "Not found"Repository: Devasy/splitwiser
Length of output: 3494
Add babel.config.js with react-native-reanimated plugin configuration.
react-native-reanimated v4 requires Babel plugin configuration (react-native-reanimated/plugin) to compile worklet functions. The Skeleton.js component uses multiple worklet APIs (useSharedValue, useAnimatedStyle, withRepeat, withTiming, withSequence, cancelAnimation) that will fail at runtime without this configuration. Create a babel.config.js at the repository root with:
module.exports = {
presets: ['babel-preset-expo'],
plugins: ['react-native-reanimated/plugin'],
};
Additionally, react-native-gesture-handler is not directly used in the codebase—it appears to be a peer dependency of react-native-reanimated. If it's included only for that reason, this is acceptable; otherwise, consider removing it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mobile/package.json` around lines 25 - 27, Add a root-level Babel config to
enable the react-native-reanimated plugin so worklet functions in Skeleton.js
compile: create a babel.config.js exporting presets ['babel-preset-expo'] and
plugins ['react-native-reanimated/plugin'] to ensure APIs used in Skeleton.js
(useSharedValue, useAnimatedStyle, withRepeat, withTiming, withSequence,
cancelAnimation) run correctly; also audit react-native-gesture-handler usage
and remove it if it is only present as a peer dependency and not used elsewhere.
Implements a cohesive skeleton loading state for the group list on the mobile
HomeScreen. This replaces the genericActivityIndicator, providing users with better perceived performance and reducing layout shifts when group data is fetched. The skeleton properly matches the visual layout of the groups and incorporates smooth animations usingreact-native-reanimated, along with proper ARIA progressbar labeling for screen readers.PR created automatically by Jules for task 6053097646188209415 started by @Devasy23
Summary by CodeRabbit
New Features
Accessibility