[jules] ux: Add skeleton loading state to HomeScreen #288
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import React from 'react'; | ||
| import { View, StyleSheet } from 'react-native'; | ||
| import { Card } from 'react-native-paper'; | ||
| import Skeleton from '../ui/Skeleton'; | ||
|
|
||
| const GroupListSkeleton = () => { | ||
| // Render 6 skeleton items to fill the screen | ||
| return ( | ||
| <View | ||
| style={styles.container} | ||
| accessible={true} | ||
| accessibilityRole="progressbar" | ||
| accessibilityLabel="Loading groups" | ||
| > | ||
|
Comment on lines
+8
to
+14
Contributor
There was a problem hiding this comment. Loading container has no scroll capability — 6 cards may be clipped on small screens. Six skeleton cards at roughly 100 px each (~600 px total) plus padding will exceed the available viewport on smaller devices (e.g., iPhone SE with ~611 px below the appbar). Wrapping in a ♻️ Proposed fix-import { View, StyleSheet } from 'react-native';
+import { View, StyleSheet, ScrollView } from 'react-native';
return (
- <View
+ <ScrollView
+ contentContainerStyle={styles.container}
accessible={true}
accessibilityRole="progressbar"
accessibilityLabel="Loading groups"
>
{[...Array(6)].map((_, index) => ( ... ))}
- </View>
+ </ScrollView>
);
const styles = StyleSheet.create({
container: {
padding: 16,
},🤖 Prompt for AI Agents |
||
| {[...Array(6)].map((_, index) => ( | ||
| <Card key={index} style={styles.card}> | ||
| <View style={styles.header}> | ||
| {/* Avatar Skeleton */} | ||
| <Skeleton width={40} height={40} borderRadius={20} /> | ||
|
|
||
| {/* Title Skeleton */} | ||
| <View style={styles.titleContainer}> | ||
| <Skeleton width={120} height={20} borderRadius={4} /> | ||
| </View> | ||
| </View> | ||
|
|
||
| <Card.Content> | ||
| {/* Status Skeleton */} | ||
| <Skeleton width={200} height={16} borderRadius={4} style={styles.status} /> | ||
|
Comment on lines
+23
to
+29
Contributor
There was a problem hiding this comment. Hardcoded pixel widths ( On narrow devices or with large accessibility font scales these values can exceed the available card width, causing visual overflow or misalignment. Use ♻️ Proposed refactor+import { View, StyleSheet, useWindowDimensions } from 'react-native';
const GroupListSkeleton = () => {
+ const { width: screenWidth } = useWindowDimensions();
+ const cardInnerWidth = screenWidth - 32 - 32; // container padding (16×2) + card padding (16×2)
...
{/* Title Skeleton */}
- <Skeleton width={120} height={20} borderRadius={4} />
+ <Skeleton width={cardInnerWidth * 0.5} height={20} borderRadius={4} />
...
{/* Status Skeleton */}
- <Skeleton width={200} height={16} borderRadius={4} style={styles.status} />
+ <Skeleton width={cardInnerWidth * 0.75} height={16} borderRadius={4} style={styles.status} />🤖 Prompt for AI Agents |
||
| </Card.Content> | ||
| </Card> | ||
| ))} | ||
| </View> | ||
| ); | ||
| }; | ||
|
|
||
| const styles = StyleSheet.create({ | ||
| container: { | ||
| padding: 16, | ||
| }, | ||
| card: { | ||
| marginBottom: 16, | ||
| }, | ||
| header: { | ||
| flexDirection: 'row', | ||
| alignItems: 'center', | ||
| padding: 16, // Mimics Card.Title padding | ||
| }, | ||
| titleContainer: { | ||
| marginLeft: 16, | ||
| flex: 1, | ||
| justifyContent: 'center', | ||
| }, | ||
| status: { | ||
| marginTop: 4, | ||
| } | ||
| }); | ||
|
|
||
| export default GroupListSkeleton; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,46 @@ | ||||||
| import React, { useEffect, useRef } from 'react'; | ||||||
|
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial Unnecessary default The project uses the new JSX transform (see -import React, { useEffect, useRef } from 'react';
+import { useEffect, useRef } from 'react';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| import { Animated } from 'react-native'; | ||||||
| import { useTheme } from 'react-native-paper'; | ||||||
|
|
||||||
| const Skeleton = ({ width, height, borderRadius = 4, style }) => { | ||||||
|
Contributor
There was a problem hiding this comment.
If a caller omits either prop, the Animated.View renders at 0×0 and the skeleton disappears silently with no warning. Add explicit defaults or mark them required. 🛡️ Proposed fix-const Skeleton = ({ width, height, borderRadius = 4, style }) => {
+const Skeleton = ({ width = 0, height = 0, borderRadius = 4, style }) => {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| const theme = useTheme(); | ||||||
| const opacity = useRef(new Animated.Value(0.5)).current; | ||||||
|
|
||||||
| useEffect(() => { | ||||||
| const animation = Animated.loop( | ||||||
| Animated.sequence([ | ||||||
| Animated.timing(opacity, { | ||||||
| toValue: 1, | ||||||
| duration: 800, | ||||||
| useNativeDriver: true, | ||||||
| }), | ||||||
| Animated.timing(opacity, { | ||||||
| toValue: 0.5, | ||||||
| duration: 800, | ||||||
| useNativeDriver: true, | ||||||
| }), | ||||||
|
Comment on lines
+12
to
+21
Contributor
There was a problem hiding this comment. 🧹 Nitpick | 🔵 Trivial Missing Skeleton animations conventionally use ♻️ Proposed refactor+import { Animated, Easing } from 'react-native';
Animated.timing(opacity, {
toValue: 1,
duration: 800,
+ easing: Easing.linear,
useNativeDriver: true,
}),
Animated.timing(opacity, {
toValue: 0.5,
duration: 800,
+ easing: Easing.linear,
useNativeDriver: true,
}),🤖 Prompt for AI Agents |
||||||
| ]) | ||||||
| ); | ||||||
|
|
||||||
| animation.start(); | ||||||
|
|
||||||
| return () => animation.stop(); | ||||||
| }, [opacity]); | ||||||
|
|
||||||
| return ( | ||||||
| <Animated.View | ||||||
| style={[ | ||||||
| { | ||||||
| width, | ||||||
| height, | ||||||
| borderRadius, | ||||||
| backgroundColor: theme.colors.surfaceVariant, | ||||||
| opacity, | ||||||
| }, | ||||||
| style, | ||||||
| ]} | ||||||
| /> | ||||||
| ); | ||||||
| }; | ||||||
|
|
||||||
| export default Skeleton; | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🧹 Nitpick | 🔵 Trivial
Unnecessary default
Reactimport — same redundancy as inSkeleton.jsgiven the project's new JSX transform.📝 Committable suggestion
🤖 Prompt for AI Agents