[jules] ux: Complete skeleton loading for HomeScreen groups #321
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||||||||||||||||||||||||||||||||
| import React from 'react'; | ||||||||||||||||||||||||||||||||||||
| import { StyleSheet, View } from 'react-native'; | ||||||||||||||||||||||||||||||||||||
| import { Card } from 'react-native-paper'; | ||||||||||||||||||||||||||||||||||||
| import Skeleton from '../ui/Skeleton'; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const GroupListSkeletonItem = () => ( | ||||||||||||||||||||||||||||||||||||
| <Card style={styles.card}> | ||||||||||||||||||||||||||||||||||||
| <Card.Title | ||||||||||||||||||||||||||||||||||||
| title={<Skeleton width={120} height={20} />} | ||||||||||||||||||||||||||||||||||||
| left={(props) => ( | ||||||||||||||||||||||||||||||||||||
| <View {...props}> | ||||||||||||||||||||||||||||||||||||
| <Skeleton width={40} height={40} borderRadius={20} /> | ||||||||||||||||||||||||||||||||||||
| </View> | ||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+8
to
+15
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🌐 Web query:
💡 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:
💡 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:
🌐 Web query:
💡 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:
Avoid passing JSX elements to In react-native-paper v5.14.5, Apply the suggested fix: use a plain string or placeholder for 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| <Card.Content> | ||||||||||||||||||||||||||||||||||||
| <Skeleton width={200} height={16} style={{ marginTop: 4 }} /> | ||||||||||||||||||||||||||||||||||||
| </Card.Content> | ||||||||||||||||||||||||||||||||||||
| </Card> | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const GroupListSkeleton = () => { | ||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||
| <View style={styles.container}> | ||||||||||||||||||||||||||||||||||||
| {[1, 2, 3, 4, 5].map((key) => ( | ||||||||||||||||||||||||||||||||||||
| <GroupListSkeletonItem key={key} /> | ||||||||||||||||||||||||||||||||||||
| ))} | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+25
to
+27
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial Prefer stable keys / avoid magic count. Using an inline array 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| </View> | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const styles = StyleSheet.create({ | ||||||||||||||||||||||||||||||||||||
| container: { | ||||||||||||||||||||||||||||||||||||
| padding: 16, | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| card: { | ||||||||||||||||||||||||||||||||||||
| marginBottom: 16, | ||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| export default GroupListSkeleton; | ||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| import React, { useEffect, useRef } from 'react'; | ||
| import { Animated, StyleSheet } from 'react-native'; | ||
| import { useTheme } from 'react-native-paper'; | ||
|
|
||
| const Skeleton = ({ width, height, borderRadius = 4, style }) => { | ||
| const theme = useTheme(); | ||
| const opacity = useRef(new Animated.Value(0.3)).current; | ||
|
Comment on lines
+5
to
+7
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial Missing defaults for
🛡️ Proposed defensive default-const Skeleton = ({ width, height, borderRadius = 4, style }) => {
+const Skeleton = ({ width = '100%', height = 16, borderRadius = 4, style }) => {🤖 Prompt for AI Agents |
||
|
|
||
| useEffect(() => { | ||
| const loop = Animated.loop( | ||
| Animated.sequence([ | ||
| Animated.timing(opacity, { | ||
| toValue: 0.7, | ||
| duration: 800, | ||
| useNativeDriver: true, | ||
| }), | ||
| Animated.timing(opacity, { | ||
| toValue: 0.3, | ||
| duration: 800, | ||
| useNativeDriver: true, | ||
| }), | ||
| ]) | ||
| ); | ||
| loop.start(); | ||
|
|
||
| return () => { | ||
| loop.stop(); | ||
| }; | ||
| }, [opacity]); | ||
|
|
||
| return ( | ||
| <Animated.View | ||
| style={[ | ||
| styles.skeleton, | ||
| { | ||
| width, | ||
| height, | ||
| borderRadius, | ||
| backgroundColor: theme.colors.surfaceVariant, | ||
| opacity, | ||
| }, | ||
| style, | ||
| ]} | ||
| /> | ||
| ); | ||
| }; | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| skeleton: { | ||
| overflow: 'hidden', | ||
| }, | ||
| }); | ||
|
Comment on lines
+48
to
+52
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial
The skeleton has no children, so 🤖 Prompt for AI Agents |
||
|
|
||
| export default Skeleton; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 newAdded/Changedentries into the existing[Unreleased]section instead of creating a second one.✏️ Proposed merge
🧰 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