[jules] ux: Complete skeleton loading for HomeScreen groups#321
[jules] ux: Complete skeleton loading for HomeScreen groups#321
Conversation
Replaced the generic `ActivityIndicator` on the Home screen with a `GroupListSkeleton` utilizing a newly created `Skeleton` primitive. The primitive uses React Native's `Animated` with `useNativeDriver: true` for efficient pulsing animations. Updated tracking files `.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.
|
WalkthroughThis PR introduces skeleton loading UI components to replace the activity indicator on the mobile HomeScreen. A reusable 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 #321 +/- ##
=======================================
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: 5
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 (1)
291-295: 🧹 Nitpick | 🔵 TrivialDead style:
loaderContaineris no longer referenced.With
ActivityIndicatorreplaced byGroupListSkeletonon line 261, theloaderContainerstyle has no remaining consumers. Also consider dropping the now-unusedActivityIndicatorimport on line 4 if it isn't used elsewhere in the file.🧹 Proposed cleanup
- loaderContainer: { - flex: 1, - justifyContent: "center", - alignItems: "center", - },🤖 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, The style object loaderContainer is dead (no longer used after replacing ActivityIndicator with GroupListSkeleton) and should be removed from the styles definition; also remove the unused ActivityIndicator import at the top of the file if there are no other references to it. Locate the styles object that contains loaderContainer and delete that property, and then remove ActivityIndicator from the import list (or the whole import statement if it becomes empty), ensuring no other code references loaderContainer or ActivityIndicator (check for usages in HomeScreen, GroupListSkeleton, and related render methods)..Jules/todo.md (1)
60-67: 🧹 Nitpick | 🔵 TrivialRedundant metadata after marking task complete.
Lines 63–67 duplicate information that's now captured by the new
Files modified(line 62) and the task summary, and retain pre-completion fields (File:,Context:,Size:,Added:) that other completed Mobile entries in this file (e.g., Pull-to-refresh at lines 53–58) drop once finished. Consider trimming for consistency.✏️ Proposed cleanup
- [x] **[ux]** Complete skeleton loading for HomeScreen groups - Completed: 2026-04-16 - Files modified: `mobile/screens/HomeScreen.js`, `mobile/components/ui/Skeleton.js`, `mobile/components/skeletons/GroupListSkeleton.js` - - File: `mobile/screens/HomeScreen.js` - - Context: Replace ActivityIndicator with skeleton group cards - Impact: Better loading experience, less jarring - - Size: ~40 lines - - Added: 2026-01-01🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.Jules/todo.md around lines 60 - 67, The completed task entry "Complete skeleton loading for HomeScreen groups" still contains redundant pre-completion metadata ("File:", "Context:", "Size:", "Added:") under the checked item; remove those extra lines and leave the task title, completion date and the new "Files modified" line so the entry matches the style used by other completed Mobile tasks (e.g., "Pull-to-refresh"). Ensure the remaining content only includes the summary, the Completed date, and Files modified.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.Jules/changelog.md:
- Around line 3-12: The changelog contains two identical "## [Unreleased]"
headings; merge the new entries under the existing "## [Unreleased]" block
instead of adding a duplicate heading: move the "Added" lines referencing
Skeleton (mobile/components/ui/Skeleton.js) and GroupListSkeleton
(mobile/components/skeletons/GroupListSkeleton.js) and the "Changed" line about
replacing ActivityIndicator with GroupListSkeleton
(mobile/screens/HomeScreen.js) into the existing "## [Unreleased]" section,
combining them into the appropriate "Added"/"Changed" subsections and removing
the second "## [Unreleased]" header to avoid duplicate headings and MD024
violations.
In `@mobile/components/skeletons/GroupListSkeleton.js`:
- Around line 25-27: Replace the inline literal [1,2,3,4,5] used to render
GroupListSkeletonItem with a clearer, tunable helper like Array.from({ length: 5
}) and use the map index as the stable key (key={index}) when mapping to
GroupListSkeletonItem; update the map callback that currently renders
GroupListSkeletonItem to accept (_, index) and pass index as the key to make the
count explicit and keys stable.
- Around line 8-15: Card.Title is receiving a JSX Skeleton via its title prop
which can cause Text nesting and clipping; change GroupListSkeleton.js so
Card.Title uses a plain string (e.g., an empty string or placeholder) for the
title prop and move the Skeleton rendering into the left prop (replace the
current left render with the avatar Skeleton) so the Skeleton component is
rendered as a left element rather than inside the title; update any props passed
to the left render function (the arrow function currently using (props) => <View
{...props}>...) to continue forwarding props to the container for proper layout.
In `@mobile/components/ui/Skeleton.js`:
- Around line 48-52: The styles object contains an unused overflow property on
the skeleton style; remove the line "overflow: 'hidden'," from the const styles
= StyleSheet.create({ skeleton: { ... } }) block (or, if you plan to add a
shimmer child later, add a clarifying comment above styles.skeleton explaining
why overflow is kept) so the Skeleton component's style no longer includes an
unnecessary property.
- Around line 5-7: The Skeleton component lacks defaults for width/height so
callers who omit them render a 0×0 animated view; update the Skeleton function
to provide sensible defaults for width and height (e.g., width = '100%' and
height = 16) in the parameter destructuring or add explicit
PropTypes/defaultProps or JSDoc to require them; modify the destructuring in
Skeleton (and update any defaultProps or PropTypes for Skeleton) so consumers
get predictable sizing and add a brief JSDoc comment above the Skeleton
declaration documenting the props.
---
Outside diff comments:
In @.Jules/todo.md:
- Around line 60-67: The completed task entry "Complete skeleton loading for
HomeScreen groups" still contains redundant pre-completion metadata ("File:",
"Context:", "Size:", "Added:") under the checked item; remove those extra lines
and leave the task title, completion date and the new "Files modified" line so
the entry matches the style used by other completed Mobile tasks (e.g.,
"Pull-to-refresh"). Ensure the remaining content only includes the summary, the
Completed date, and Files modified.
In `@mobile/screens/HomeScreen.js`:
- Around line 291-295: The style object loaderContainer is dead (no longer used
after replacing ActivityIndicator with GroupListSkeleton) and should be removed
from the styles definition; also remove the unused ActivityIndicator import at
the top of the file if there are no other references to it. Locate the styles
object that contains loaderContainer and delete that property, and then remove
ActivityIndicator from the import list (or the whole import statement if it
becomes empty), ensuring no other code references loaderContainer or
ActivityIndicator (check for usages in HomeScreen, GroupListSkeleton, and
related render methods).
🪄 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: 722b26e0-c161-43a1-a973-53f8ba6dc64a
📒 Files selected for processing (5)
.Jules/changelog.md.Jules/todo.mdmobile/components/skeletons/GroupListSkeleton.jsmobile/components/ui/Skeleton.jsmobile/screens/HomeScreen.js
|
|
||
| ## [Unreleased] | ||
|
|
||
| ### Added | ||
| - Created reusable `Skeleton` UI primitive in `mobile/components/ui/Skeleton.js` with pulsing animation. | ||
| - Created `GroupListSkeleton` in `mobile/components/skeletons/GroupListSkeleton.js` using the new `Skeleton` component. | ||
|
|
||
| ### Changed | ||
| - Replaced `ActivityIndicator` with `GroupListSkeleton` in `mobile/screens/HomeScreen.js` for a better loading experience. | ||
|
|
There was a problem hiding this comment.
Duplicate ## [Unreleased] section.
A new ## [Unreleased] block is added at lines 4–11 while another ## [Unreleased] section already exists at line 17. Two sections with the same heading in one changelog are confusing and will also trip MD024 (duplicate headings). Merge the new Added/Changed entries into the existing [Unreleased] section instead of creating a second one.
✏️ Proposed merge
-
-## [Unreleased]
-
-### Added
-- Created reusable `Skeleton` UI primitive in `mobile/components/ui/Skeleton.js` with pulsing animation.
-- Created `GroupListSkeleton` in `mobile/components/skeletons/GroupListSkeleton.js` using the new `Skeleton` component.
-
-### Changed
-- Replaced `ActivityIndicator` with `GroupListSkeleton` in `mobile/screens/HomeScreen.js` for a better loading experience.
-
> All UI/UX changes made by Jules automated enhancement agent.
---
## [Unreleased]
### Added
+- Created reusable `Skeleton` UI primitive in `mobile/components/ui/Skeleton.js` with pulsing animation.
+- Created `GroupListSkeleton` in `mobile/components/skeletons/GroupListSkeleton.js` using the new `Skeleton` component.
- **Password Strength Meter:** ...
@@
### Changed
+- Replaced `ActivityIndicator` with `GroupListSkeleton` in `mobile/screens/HomeScreen.js` for a better loading experience.
- **Web App:** ...🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 10-10: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.Jules/changelog.md around lines 3 - 12, The changelog contains two
identical "## [Unreleased]" headings; merge the new entries under the existing
"## [Unreleased]" block instead of adding a duplicate heading: move the "Added"
lines referencing Skeleton (mobile/components/ui/Skeleton.js) and
GroupListSkeleton (mobile/components/skeletons/GroupListSkeleton.js) and the
"Changed" line about replacing ActivityIndicator with GroupListSkeleton
(mobile/screens/HomeScreen.js) into the existing "## [Unreleased]" section,
combining them into the appropriate "Added"/"Changed" subsections and removing
the second "## [Unreleased]" header to avoid duplicate headings and MD024
violations.
| <Card.Title | ||
| title={<Skeleton width={120} height={20} />} | ||
| left={(props) => ( | ||
| <View {...props}> | ||
| <Skeleton width={40} height={40} borderRadius={20} /> | ||
| </View> | ||
| )} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
react-native-paper Card.Title title prop accepts ReactNode or only string in v5
💡 Result:
In react-native-paper v5, Card.Title’s title prop accepts React.ReactNode (not only a string). Specifically, the CardTitle docs state: “title (required) Type: React.ReactNode” for the title text. So you can pass a React element/node (e.g., JSX that results in a renderable node), not just a plain string.
Citations:
🏁 Script executed:
# Check react-native-paper version
fd -type f -name "package.json" | xargs grep -l "react-native-paper" 2>/dev/null | head -5Repository: Devasy/splitwiser
Length of output: 231
🏁 Script executed:
# Look for other Card.Title usages in the codebase
rg "Card\.Title" --type js --type jsx --type ts --type tsx -A 5 | head -100Repository: Devasy/splitwiser
Length of output: 88
🏁 Script executed:
# Check the full context of the file mentioned in the review
fd -type f -name "GroupListSkeleton.js" -o -name "GroupListSkeleton.jsx" -o -name "GroupListSkeleton.ts" -o -name "GroupListSkeleton.tsx" | xargs cat -n 2>/dev/nullRepository: Devasy/splitwiser
Length of output: 231
🏁 Script executed:
# Find package.json and check react-native-paper version
find . -name "package.json" -type f | head -1 | xargs cat | grep -A 2 -B 2 "react-native-paper"Repository: Devasy/splitwiser
Length of output: 241
🏁 Script executed:
# Search for Card.Title usages in JS/TS files
rg "Card\.Title" -A 5 --max-count 20Repository: Devasy/splitwiser
Length of output: 3631
🏁 Script executed:
# Find and display GroupListSkeleton.js
fd "GroupListSkeleton" -e js -e jsx -e ts -e tsx | xargs cat -nRepository: Devasy/splitwiser
Length of output: 1250
🌐 Web query:
react-native-paper 5.14.5 Card.Title JSX element rendering Text component
💡 Result:
In react-native-paper v5.14.5, Card.Title’s title prop is intended to be a ReactNode “text” value (practically: a string or a Text-based node), and it is implemented/rendered such that you generally cannot pass a React Native component as you would normally in JSX. 1) What Card.Title expects (types/docs) - Card.Title’s title prop type is React.ReactNode and the docs explicitly note it “will only accept a string or …-based node” (i.e., it’s constrained to text-like children rather than arbitrary JSX/React Native components). 2) Why your JSX like ... fails - Community reports show that passing a react-native Text (or paper Text) directly to Card.Title’s title often triggers errors such as “Text cannot be used as a JSX component…” after upgrading paper. - The underlying limitation has been discussed historically: even if the prop type is React.ReactNode, Card.Title is rendered inside paper’s styled text implementation, which effectively limits what you can nest inside. 3) What to do instead - Pass a string (recommended): <Card.Title title={Top ${sites.length} Sights in ${country.name}} /> - If you need rich formatting (e.g., inline icons), prefer the Card.Title left/right props for non-text elements: <Card.Title title="Card Title" left={(props) => <Avatar.Icon {...props} icon="folder" />} right={(props) => <IconButton {...props} icon="dots-vertical" onPress={...} />} /> Bottom line: for v5.14.5, avoid wrapping Card.Title title in elements; use a plain string for title, and use left/right to add other UI next to the title.
Citations:
- 1: https://callstack.github.io/react-native-paper/docs/components/Card/CardTitle/
- 2: https://stackoverflow.com/questions/76808479/set-the-title-dynamically-in-react-native-paper-card-title
- 3: Card.Title expects type
React.ReactNodebut is rendered inside aStyledText-- which means in effect it can only accept a Text node callstack/react-native-paper#1015 - 4: https://app.unpkg.com/react-native-paper@5.14.1/files/lib/typescript/components/Card/CardTitle.d.ts
🌐 Web query:
react-native-paper Card.Title rendering ReactNode behavior issues v5
💡 Result:
In react-native-paper v5, Card.Title’s title/subtitle props accept React.ReactNode, but the component renders them inside a react-native-paper StyledText/Text context. As a result, not all ReactNode types behave well: passing components that internally render native can trigger runtime errors like “Nesting of within is not currently supported”, or formatting/layout issues because they’re effectively treated as “text-like” content. What to do instead 1) Pass plain strings (most reliable): <Card.Title title={Top ${sites.length} Sights} /> 2) If you need inline styling/icons, use the paper Text component (not react-native’s Text), and keep it compatible with the parent Text nesting rules. Many people hit errors by using react-native’s inside Card.Title’s title prop. 3) Prefer the left/right slots for non-text elements (avatar/icons). Card.Title exposes left and right callbacks (with leftStyle/rightStyle wrappers in v5) specifically for element-side customization, rather than trying to inject icons inside the title node. Evidence from v5 docs and related issues - The official CardTitle docs state that title and subtitle are React.ReactNode but “will only accept a string or -based node” (i.e., effectively Text-based content). This matches the observed nesting constraints. - A reported issue documents the root cause: even though the type says React.ReactNode, Card.Title is wrapped in StyledText, meaning it can only accept text nodes; attempting to pass a node that uses results in “Nesting of within is not currently supported”. - The docs also show left/right props as the supported way to render elements on the sides, and v5 includes leftStyle/rightStyle to help with wrapper styling. Related practical footgun (separate from ReactNode nesting) - If you increase title/subtitle fontSize dramatically, there have been bug reports that card height/line-height may not adjust correctly on Android/iOS; suggestions in v5 point to theme variant lineHeight and/or setting lineHeight explicitly in styles. If you share your exact Card.Title usage snippet and the error message/stack trace, I can tell you which of the above cases you’re in and the minimal change to fix it.
Citations:
- 1: Card.Title expects type
React.ReactNodebut is rendered inside aStyledText-- which means in effect it can only accept a Text node callstack/react-native-paper#1015 - 2: https://stackoverflow.com/questions/76808479/set-the-title-dynamically-in-react-native-paper-card-title
- 3: https://callstack.github.io/react-native-paper/4.0/card-title.html
- 4: https://callstack.github.io/react-native-paper/docs/components/Card/CardTitle/
- 5: Card.Title (Android & iOS) - The height of the card does not automatically adjust when Title and Subtitle fontSize gets larger. callstack/react-native-paper#4184
Avoid passing JSX elements to Card.Title's title prop; use the left prop instead.
In react-native-paper v5.14.5, Card.Title's title prop is rendered inside a StyledText component, which constrains it to text-like content only. Passing JSX elements (like <Skeleton />) can trigger "Nesting of within is not currently supported" errors or layout/height clipping issues, even though the prop accepts React.ReactNode at the type level. The left and right props are the intended way to render custom elements around the title.
Apply the suggested fix: use a plain string or placeholder for title, and render the skeleton via left:
Suggested change
<Card.Title
- title={<Skeleton width={120} height={20} />}
+ title=" "
+ titleStyle={{ height: 20 }}
left={(props) => (
<View {...props}>
<Skeleton width={40} height={40} borderRadius={20} />
</View>
)}
/>Verify the output renders correctly on both iOS and Android without layout clipping or warnings.
📝 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.
| <Card.Title | |
| title={<Skeleton width={120} height={20} />} | |
| left={(props) => ( | |
| <View {...props}> | |
| <Skeleton width={40} height={40} borderRadius={20} /> | |
| </View> | |
| )} | |
| /> | |
| <Card.Title | |
| title=" " | |
| titleStyle={{ height: 20 }} | |
| left={(props) => ( | |
| <View {...props}> | |
| <Skeleton width={40} height={40} borderRadius={20} /> | |
| </View> | |
| )} | |
| /> |
🤖 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 8 - 15,
Card.Title is receiving a JSX Skeleton via its title prop which can cause Text
nesting and clipping; change GroupListSkeleton.js so Card.Title uses a plain
string (e.g., an empty string or placeholder) for the title prop and move the
Skeleton rendering into the left prop (replace the current left render with the
avatar Skeleton) so the Skeleton component is rendered as a left element rather
than inside the title; update any props passed to the left render function (the
arrow function currently using (props) => <View {...props}>...) to continue
forwarding props to the container for proper layout.
| {[1, 2, 3, 4, 5].map((key) => ( | ||
| <GroupListSkeletonItem key={key} /> | ||
| ))} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer stable keys / avoid magic count.
Using an inline array [1,2,3,4,5] is fine but slightly opaque. A small helper (Array.from({ length: 5 })) with key={index} is clearer and makes the count easy to tune. Purely stylistic.
🤖 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 25 - 27,
Replace the inline literal [1,2,3,4,5] used to render GroupListSkeletonItem with
a clearer, tunable helper like Array.from({ length: 5 }) and use the map index
as the stable key (key={index}) when mapping to GroupListSkeletonItem; update
the map callback that currently renders GroupListSkeletonItem to accept (_,
index) and pass index as the key to make the count explicit and keys stable.
| const Skeleton = ({ width, height, borderRadius = 4, style }) => { | ||
| const theme = useTheme(); | ||
| const opacity = useRef(new Animated.Value(0.3)).current; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Missing defaults for width / height.
width and height have no defaults, so consumers who forget to pass them get an Animated.View sized to content (effectively 0×0) with no warning. Given this is a reusable primitive, either add sensible defaults or document the required props via PropTypes/JSDoc.
🛡️ Proposed defensive default
-const Skeleton = ({ width, height, borderRadius = 4, style }) => {
+const Skeleton = ({ width = '100%', height = 16, borderRadius = 4, style }) => {🤖 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 5 - 7, The Skeleton component
lacks defaults for width/height so callers who omit them render a 0×0 animated
view; update the Skeleton function to provide sensible defaults for width and
height (e.g., width = '100%' and height = 16) in the parameter destructuring or
add explicit PropTypes/defaultProps or JSDoc to require them; modify the
destructuring in Skeleton (and update any defaultProps or PropTypes for
Skeleton) so consumers get predictable sizing and add a brief JSDoc comment
above the Skeleton declaration documenting the props.
| const styles = StyleSheet.create({ | ||
| skeleton: { | ||
| overflow: 'hidden', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
overflow: 'hidden' on an empty view is unused.
The skeleton has no children, so overflow: 'hidden' doesn't do anything today. Harmless, but can be removed unless you intend to add a shimmer gradient child later.
🤖 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 48 - 52, The styles object
contains an unused overflow property on the skeleton style; remove the line
"overflow: 'hidden'," from the const styles = StyleSheet.create({ skeleton: {
... } }) block (or, if you plan to add a shimmer child later, add a clarifying
comment above styles.skeleton explaining why overflow is kept) so the Skeleton
component's style no longer includes an unnecessary property.
1 participant
mobile/components/ui/Skeleton.jsto serve as a reusable skeleton loading component.mobile/components/skeletons/GroupListSkeleton.jsto represent a layout matching theHapticCardon the HomeScreen.ActivityIndicatorinHomeScreen.jswithGroupListSkeleton.react-native-papertheming (theme.colors.surfaceVariant) and lifecycle hooks (loop.stop()cleanup).PR created automatically by Jules for task 4050041467099439475 started by @Devasy23
Summary by CodeRabbit
New Features
Documentation