[jules] enhance: Add skeleton loading for HomeScreen groups#303
[jules] enhance: Add skeleton loading for HomeScreen groups#303
Conversation
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 pull request introduces mobile skeleton loading UI components to replace the basic ActivityIndicator on HomeScreen. It adds a new animated Skeleton primitive component and a GroupListSkeleton component that mirrors the HapticCard layout, integrates them into HomeScreen, and updates project documentation. 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. 📝 Coding Plan
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 #303 +/- ##
=======================================
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: 3
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)
290-294:⚠️ Potential issue | 🟡 MinorRemove unused
loaderContainerstyle.This style was likely used to center the previous
ActivityIndicatorbut is no longer referenced after switching toGroupListSkeleton.🧹 Proposed fix
container: { flex: 1, }, - loaderContainer: { - flex: 1, - justifyContent: "center", - alignItems: "center", - }, list: { padding: 16, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mobile/screens/HomeScreen.js` around lines 290 - 294, Remove the unused loaderContainer style from the styles object in HomeScreen.js (it’s no longer referenced after replacing ActivityIndicator with GroupListSkeleton). Locate the styles declaration and delete the loaderContainer entry to keep the stylesheet clean and avoid dead code; ensure no other part of the file references loaderContainer (e.g., search for "loaderContainer") 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/skeletons/GroupListSkeleton.js`:
- Around line 7-21: The renderSkeletonItem function currently destructures an
unused item parameter; update its signature to reflect it's intentionally unused
by renaming it to _item (or replace with a single unused placeholder like _),
e.g., change "const renderSkeletonItem = ({ item }) =>" to "const
renderSkeletonItem = ({ _item }) =>" (or "const renderSkeletonItem = (_) =>") so
linters won’t flag the unused variable; keep the body of renderSkeletonItem and
Card rendering unchanged.
- Around line 10-18: Update the avatar skeleton in GroupListSkeleton.js so its
Skeleton matches the real Avatar size: change the Skeleton inside the left
render (currently width=40 height=40 borderRadius=20) to use 64×64 with
borderRadius 32, and update styles.avatarPlaceholder if it assumes the smaller
size; also extract the avatar size (64) and avatar borderRadius (32) into shared
constants (e.g., AVATAR_SIZE, AVATAR_RADIUS) so both the Skeleton and the real
Avatar use the same values to avoid future mismatches.
In `@mobile/components/ui/Skeleton.js`:
- Around line 1-3: The import list in the Skeleton component includes an unused
symbol `View`; remove `View` from the react-native import (adjust the line
importing `Animated` and `StyleSheet`) so only `Animated` and `StyleSheet` are
imported, and verify the `Skeleton` component (and any helpers) do not reference
`View` before committing.
---
Outside diff comments:
In `@mobile/screens/HomeScreen.js`:
- Around line 290-294: Remove the unused loaderContainer style from the styles
object in HomeScreen.js (it’s no longer referenced after replacing
ActivityIndicator with GroupListSkeleton). Locate the styles declaration and
delete the loaderContainer entry to keep the stylesheet clean and avoid dead
code; ensure no other part of the file references loaderContainer (e.g., search
for "loaderContainer") before committing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e7e1020-9d20-4821-a973-5198e5f0d1fd
📒 Files selected for processing (5)
.Jules/changelog.md.Jules/todo.mdmobile/components/skeletons/GroupListSkeleton.jsmobile/components/ui/Skeleton.jsmobile/screens/HomeScreen.js
| const renderSkeletonItem = ({ item }) => ( | ||
| <Card style={styles.card}> | ||
| <Card.Title | ||
| title={<Skeleton width={120} height={20} borderRadius={4} />} | ||
| left={(props) => ( | ||
| <View style={[props.style, styles.avatarPlaceholder]}> | ||
| <Skeleton width={40} height={40} borderRadius={20} /> | ||
| </View> | ||
| )} | ||
| /> | ||
| <Card.Content> | ||
| <Skeleton width={180} height={16} borderRadius={4} style={styles.contentSkeleton} /> | ||
| </Card.Content> | ||
| </Card> | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused item parameter in renderSkeletonItem.
The item parameter is destructured but never used since all skeleton items are identical. Consider using an underscore prefix to indicate it's intentionally unused, or simplify to just use the index.
♻️ Proposed fix
- const renderSkeletonItem = ({ item }) => (
+ const renderSkeletonItem = ({ item: _item }) => (
<Card style={styles.card}>📝 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.
| const renderSkeletonItem = ({ item }) => ( | |
| <Card style={styles.card}> | |
| <Card.Title | |
| title={<Skeleton width={120} height={20} borderRadius={4} />} | |
| left={(props) => ( | |
| <View style={[props.style, styles.avatarPlaceholder]}> | |
| <Skeleton width={40} height={40} borderRadius={20} /> | |
| </View> | |
| )} | |
| /> | |
| <Card.Content> | |
| <Skeleton width={180} height={16} borderRadius={4} style={styles.contentSkeleton} /> | |
| </Card.Content> | |
| </Card> | |
| ); | |
| const renderSkeletonItem = ({ item: _item }) => ( | |
| <Card style={styles.card}> | |
| <Card.Title | |
| title={<Skeleton width={120} height={20} borderRadius={4} />} | |
| left={(props) => ( | |
| <View style={[props.style, styles.avatarPlaceholder]}> | |
| <Skeleton width={40} height={40} borderRadius={20} /> | |
| </View> | |
| )} | |
| /> | |
| <Card.Content> | |
| <Skeleton width={180} height={16} borderRadius={4} style={styles.contentSkeleton} /> | |
| </Card.Content> | |
| </Card> | |
| ); |
🤖 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 7 - 21, The
renderSkeletonItem function currently destructures an unused item parameter;
update its signature to reflect it's intentionally unused by renaming it to
_item (or replace with a single unused placeholder like _), e.g., change "const
renderSkeletonItem = ({ item }) =>" to "const renderSkeletonItem = ({ _item })
=>" (or "const renderSkeletonItem = (_) =>") so linters won’t flag the unused
variable; keep the body of renderSkeletonItem and Card rendering unchanged.
| title={<Skeleton width={120} height={20} borderRadius={4} />} | ||
| left={(props) => ( | ||
| <View style={[props.style, styles.avatarPlaceholder]}> | ||
| <Skeleton width={40} height={40} borderRadius={20} /> | ||
| </View> | ||
| )} | ||
| /> | ||
| <Card.Content> | ||
| <Skeleton width={180} height={16} borderRadius={4} style={styles.contentSkeleton} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Avatar dimensions in HapticCard or related components to verify consistency
rg -n "Avatar\.(Image|Text)" --type=js -A2 -B2 mobile/
# Check Card.Title usage patterns for dimension hints
ast-grep --pattern 'Card.Title
$$$
'Repository: Devasy/splitwiser
Length of output: 3573
🏁 Script executed:
# Find HapticCard component definition
fd -type f -name "*.js" | xargs grep -l "HapticCard" | head -5Repository: Devasy/splitwiser
Length of output: 231
🏁 Script executed:
# Search for Card.Title in HomeScreen to see actual card structure
rg -n "Card\." mobile/screens/HomeScreen.js -A3 -B1Repository: Devasy/splitwiser
Length of output: 472
🏁 Script executed:
# Find the actual GroupListSkeleton usage and HapticCard definition
fd -type f -name "*Skeleton*" mobile/components/Repository: Devasy/splitwiser
Length of output: 231
🏁 Script executed:
# Find HapticCard component definition
find mobile -name "*.js" | xargs grep -l "HapticCard" | head -10Repository: Devasy/splitwiser
Length of output: 161
🏁 Script executed:
# Search for HapticCard exports or definitions
rg -n "export.*HapticCard|const HapticCard|function HapticCard" mobile/Repository: Devasy/splitwiser
Length of output: 234
🏁 Script executed:
# Check for HapticCard component file directly
find mobile -name "*HapticCard*" -o -name "*Card*"Repository: Devasy/splitwiser
Length of output: 95
🏁 Script executed:
# Look at the actual dimensions in HomeScreen card rendering
rg -n "const GroupCard\|function GroupCard" mobile/screens/HomeScreen.js -A 50Repository: Devasy/splitwiser
Length of output: 43
🏁 Script executed:
# Read HapticCard component to understand its structure
cat mobile/components/ui/HapticCard.jsRepository: Devasy/splitwiser
Length of output: 413
🏁 Script executed:
# Read HomeScreen GroupCard function to see actual dimensions
sed -n '180,230p' mobile/screens/HomeScreen.jsRepository: Devasy/splitwiser
Length of output: 1586
🏁 Script executed:
# Read the complete GroupListSkeleton file
cat mobile/components/skeletons/GroupListSkeleton.jsRepository: Devasy/splitwiser
Length of output: 1635
🏁 Script executed:
# Check React Paper version to determine default Avatar size
cat mobile/package.json | grep -A2 -B2 "react-native-paper"Repository: Devasy/splitwiser
Length of output: 241
🏁 Script executed:
# Search for Avatar default size documentation or any size specifications in HomeScreen
rg -n "Avatar" mobile/screens/HomeScreen.js -B3 -A3Repository: Devasy/splitwiser
Length of output: 518
🏁 Script executed:
# Check styles.card in HomeScreen to see actual card dimensions/padding
rg -n "styles\." mobile/screens/HomeScreen.js | grep -i "card\|avatar"Repository: Devasy/splitwiser
Length of output: 92
🏁 Script executed:
# Find the StyleSheet definition in HomeScreen
rg -n "const styles = StyleSheet" mobile/screens/HomeScreen.js -A 100Repository: Devasy/splitwiser
Length of output: 849
🏁 Script executed:
# Search for React Paper Avatar default size information or any comments about it
rg -n "Avatar" mobile/ --type-list | head -20Repository: Devasy/splitwiser
Length of output: 450
🏁 Script executed:
# Check if there are any size references in component files or constants
rg -n "size.*=" mobile/components/ | grep -i avatarRepository: Devasy/splitwiser
Length of output: 43
🏁 Script executed:
# Search React Paper documentation for Avatar default size - check node_modules or look for any size-related comments
find mobile -name "*.js" -exec grep -l "Avatar.*size" {} \; | head -5Repository: Devasy/splitwiser
Length of output: 198
🏁 Script executed:
# Look at the React Paper v5 Avatar documentation through web search or check if there's any inline documentation
# For now, check if any dev comments mention dimensions or let's see Card.Title default height
rg -n "Card\.Title\|height.*20\|width.*120" mobile/screens/ -iRepository: Devasy/splitwiser
Length of output: 43
🌐 Web query:
React Native Paper v5 Avatar default size when not specified
💡 Result:
The default size for Avatar components (Avatar.Image, Avatar.Icon, Avatar.Text) in React Native Paper v5, when the size prop is not specified, is 64 pixels.
Citations:
- 1: https://callstack.github.io/react-native-paper/docs/components/Avatar/AvatarImage/
- 2: https://callstack.github.io/react-native-paper/docs/components/Avatar/AvatarText/
- 3: https://callstack.github.io/react-native-paper/docs/components/Avatar/AvatarIcon/
🌐 Web query:
React Native Paper Card.Title default height dimensions
💡 Result:
React Native Paper's Card.Title component does not have a fixed default height. Its height is content-driven and auto-adjusts based on the title text (default variant: bodyLarge, 1 line), optional subtitle (default variant: bodyMedium, 1 line), and optional left/right icons/avatars. Key dimensions from official theme (MD3): - bodyLarge: fontSize 16, lineHeight 24 - bodyMedium: fontSize 14, lineHeight 20 The component uses a flex row layout with padding (likely similar to CardContent's 16 horizontal, vertical varies by position). Without subtitle or icons, height approximates the title's lineHeight (~24dp) plus minimal padding. With subtitle stacked below, height sums both lineHeights plus spacing (~48-56dp total, depending on padding). No explicit height is defined in docs or source snippets; use style={{ height: X }} to set custom height. For precise measurement, render in app and inspect with React Native Debugger.
Citations:
- 1: https://callstack.github.io/react-native-paper/docs/components/Card/CardTitle/
- 2: https://callstack.github.io/react-native-paper/docs/components/Card/CardTitle
- 3: https://callstack.github.io/react-native-paper/docs/components/Card/
- 4: https://callstack.github.io/react-native-paper/docs/components/Text/
- 5: Variant labelLarge was not provided properly. Valid variants are regular, medium, bold, heavy. callstack/react-native-paper#4573
- 6: https://callstack.github.io/react-native-paper/docs/guides/fonts
- 7: https://www.papelnort.com
- 8: https://github.com/callstack/react-native-paper/blob/main/src/components/Card/CardContent.tsx
- 9: https://app.unpkg.com/react-native-paper@5.14.1/files/lib/typescript/components/Card/CardTitle.d.ts
🏁 Script executed:
# Let's check if there are any related issues or TODOs about layout shift or dimensions
rg -n "layout.*shift\|dimension\|size.*match" mobile/ -iRepository: Devasy/splitwiser
Length of output: 43
🏁 Script executed:
# Check if Avatar has a default size constant anywhere
rg -n "const.*Avatar\|Avatar.*40\|Avatar.*size" mobile/components/ -iRepository: Devasy/splitwiser
Length of output: 43
Avatar skeleton dimensions incorrect—causing layout shift instead of preventing it.
The Avatar skeleton is 40×40, but HomeScreen's Avatar components use the React Native Paper v5 default size of 64×64 (no explicit size prop is passed). When the skeleton is replaced with real content, this 24px difference causes layout shift. Update the skeleton to use 64×64 for the avatar to match the actual component:
<Skeleton width={40} height={40} borderRadius={20} />should be:
<Skeleton width={64} height={64} borderRadius={32} />Also consider extracting these dimensions (64 for avatar, 32 for borderRadius) as shared constants to prevent future mismatches.
📝 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.
| title={<Skeleton width={120} height={20} borderRadius={4} />} | |
| left={(props) => ( | |
| <View style={[props.style, styles.avatarPlaceholder]}> | |
| <Skeleton width={40} height={40} borderRadius={20} /> | |
| </View> | |
| )} | |
| /> | |
| <Card.Content> | |
| <Skeleton width={180} height={16} borderRadius={4} style={styles.contentSkeleton} /> | |
| title={<Skeleton width={120} height={20} borderRadius={4} />} | |
| left={(props) => ( | |
| <View style={[props.style, styles.avatarPlaceholder]}> | |
| <Skeleton width={64} height={64} borderRadius={32} /> | |
| </View> | |
| )} | |
| /> | |
| <Card.Content> | |
| <Skeleton width={180} height={16} borderRadius={4} style={styles.contentSkeleton} /> |
🤖 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 10 - 18,
Update the avatar skeleton in GroupListSkeleton.js so its Skeleton matches the
real Avatar size: change the Skeleton inside the left render (currently width=40
height=40 borderRadius=20) to use 64×64 with borderRadius 32, and update
styles.avatarPlaceholder if it assumes the smaller size; also extract the avatar
size (64) and avatar borderRadius (32) into shared constants (e.g., AVATAR_SIZE,
AVATAR_RADIUS) so both the Skeleton and the real Avatar use the same values to
avoid future mismatches.
| import React, { useEffect, useRef } from 'react'; | ||
| import { Animated, View, StyleSheet } from 'react-native'; | ||
| import { useTheme } from 'react-native-paper'; |
There was a problem hiding this comment.
Remove unused View import.
The View import from react-native is not used in this component—only Animated.View is rendered.
🧹 Proposed fix
import React, { useEffect, useRef } from 'react';
-import { Animated, View, StyleSheet } from 'react-native';
+import { Animated, StyleSheet } from 'react-native';
import { useTheme } from 'react-native-paper';📝 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 React, { useEffect, useRef } from 'react'; | |
| import { Animated, View, StyleSheet } from 'react-native'; | |
| import { useTheme } from 'react-native-paper'; | |
| import React, { useEffect, useRef } from 'react'; | |
| import { Animated, StyleSheet } from 'react-native'; | |
| import { useTheme } from 'react-native-paper'; |
🤖 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 1 - 3, The import list in the
Skeleton component includes an unused symbol `View`; remove `View` from the
react-native import (adjust the line importing `Animated` and `StyleSheet`) so
only `Animated` and `StyleSheet` are imported, and verify the `Skeleton`
component (and any helpers) do not reference `View` before committing.
1 participant
Replaced the basic
ActivityIndicatoron theHomeScreenwith a sophisticatedGroupListSkeletonthat perfectly mimics the layout of theHapticCardelements. Created a reusable animatedSkeletonprimitive. This provides a much smoother, professional loading experience and reduces layout shift when groups load. Added corresponding updates to tracking files.PR created automatically by Jules for task 12872660234203792430 started by @Devasy23
Summary by CodeRabbit