Optimize media library handling and photo grid rendering in Expo app #386
Conversation
The commit improves performance by: - Adding memoization for grid items and layout calculations - Implementing FlatList performance optimizations - Enhancing image caching and transition settings - Adding photo prefetching
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
WalkthroughThis pull request introduces several improvements to media library handling and photo grid rendering in an Expo-based application. The changes focus on optimizing performance, enhancing image loading, and refactoring media library access. A new Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/expo/src/hooks/useMediaLibrary.ts (1)
Line range hint
15-35: Enhance error handling for better user experience.The parallel prefetching optimization is excellent for performance. However, the error handling could be more user-friendly.
const loadRecentPhotos = useCallback(async () => { try { const { assets } = await MediaLibrary.getAssetsAsync({ first: 15, sortBy: MediaLibrary.SortBy.creationTime, mediaType: [MediaLibrary.MediaType.photo], }); const photos = assets.map((asset) => ({ id: asset.id, uri: asset.uri, })); // Prefetch all photos in parallel await Promise.all(photos.map((photo) => Image.prefetch(photo.uri))); setRecentPhotos(photos); } catch (error) { - console.error("Error loading recent photos:", error); + console.error("Error loading recent photos:", error); + toast.error("Failed to load recent photos. Please try again."); + setRecentPhotos([]); } }, [setRecentPhotos]);apps/expo/src/components/PhotoGrid.tsx (2)
29-117: Well-structured memoized GridItem component!The component effectively separates rendering logic and implements performance optimizations.
Consider removing the recyclingKey prop.
Based on previous feedback, the recyclingKey prop isn't necessary for small lists of recent photos.
<ExpoImage source={{ uri: item.uri }} style={{ width: "100%", height: "100%", }} contentFit="cover" contentPosition="center" transition={200} cachePolicy="memory-disk" - recyclingKey={item.id} placeholder={null} placeholderContentFit="cover" className="bg-muted/10" />
232-236: Remove unnecessary FlatList optimizations.Based on previous feedback, these performance optimizations aren't necessary for iOS implementations with a fixed/limited number of items.
<FlatList data={gridData} renderItem={renderItem} numColumns={4} showsVerticalScrollIndicator={false} contentContainerStyle={{ paddingBottom: 100, }} keyExtractor={(item) => item.id} horizontal={false} - windowSize={5} - maxToRenderPerBatch={16} - updateCellsBatchingPeriod={50} - initialNumToRender={12} - removeClippedSubviews={true} getItemLayout={getItemLayout} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/expo/src/app/new.tsx(2 hunks)apps/expo/src/components/EventPreview.tsx(1 hunks)apps/expo/src/components/PhotoGrid.tsx(4 hunks)apps/expo/src/hooks/useMediaLibrary.ts(5 hunks)
🧰 Additional context used
📓 Learnings (1)
apps/expo/src/components/PhotoGrid.tsx (2)
Learnt from: jaronheard
PR: jaronheard/soonlist-turbo#375
File: apps/expo/src/components/PhotoGrid.tsx:93-171
Timestamp: 2025-01-13T03:13:45.462Z
Learning: FlatList performance optimizations (initialNumToRender, maxToRenderPerBatch, windowSize, removeClippedSubviews) are not necessary for iOS implementations with a fixed/limited number of items, as iOS has better performance characteristics for FlatList rendering.
Learnt from: jaronheard
PR: jaronheard/soonlist-turbo#375
File: apps/expo/src/components/PhotoGrid.tsx:150-160
Timestamp: 2025-01-13T03:23:22.636Z
Learning: The PhotoGrid component in the Expo app intentionally omits the recyclingKey prop for ExpoImage components as it's designed for smaller lists of recent photos, where the benefits of recycling don't outweigh potential complexities.
🪛 GitHub Check: lint
apps/expo/src/components/PhotoGrid.tsx
[failure] 150-150:
Unexpected any. Specify a different type
🪛 GitHub Actions: CI
apps/expo/src/components/PhotoGrid.tsx
[error] 150-150: Unexpected any. Specify a different type (@typescript-eslint/no-explicit-any)
🔇 Additional comments (5)
apps/expo/src/hooks/useMediaLibrary.ts (2)
Line range hint
37-76: Well-implemented permission handling and subscription management!The implementation follows React best practices:
- Uses useFocusEffect for permission checks
- Properly cleans up subscription
- Efficiently handles photo library changes
Line range hint
79-89: Clean implementation of the refresh mechanism!The useEffect hook correctly handles the refresh flag and dependencies.
apps/expo/src/components/EventPreview.tsx (1)
65-69: Good image loading optimizations!The changes enhance the image loading experience:
- Smoother transitions with increased duration
- Efficient caching with memory-disk policy
- Better visual feedback during loading states
apps/expo/src/components/PhotoGrid.tsx (1)
140-146: Efficient grid data memoization!The useMemo implementation correctly optimizes the grid data array creation.
apps/expo/src/app/new.tsx (1)
16-16: Clean integration of useMediaLibrary hook!The hook is properly integrated for its side effects.
Also applies to: 28-28
| const getItemLayout = useMemo( | ||
| () => (_: any, index: number) => ({ | ||
| length: imageSize, | ||
| offset: imageSize * Math.floor(index / columns), | ||
| index, | ||
| }), | ||
| [imageSize, columns], | ||
| ); |
There was a problem hiding this comment.
Fix TypeScript any type.
Replace the any type with proper typing to resolve the TypeScript error.
- const getItemLayout = useMemo(
- () => (_: any, index: number) => ({
+ const getItemLayout = useMemo(
+ () => (_: unknown[], index: number) => ({
length: imageSize,
offset: imageSize * Math.floor(index / columns),
index,
}),
[imageSize, columns],
);📝 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 getItemLayout = useMemo( | |
| () => (_: any, index: number) => ({ | |
| length: imageSize, | |
| offset: imageSize * Math.floor(index / columns), | |
| index, | |
| }), | |
| [imageSize, columns], | |
| ); | |
| const getItemLayout = useMemo( | |
| () => (_: unknown[], index: number) => ({ | |
| length: imageSize, | |
| offset: imageSize * Math.floor(index / columns), | |
| index, | |
| }), | |
| [imageSize, columns], | |
| ); |
🧰 Tools
🪛 GitHub Check: lint
[failure] 150-150:
Unexpected any. Specify a different type
🪛 GitHub Actions: CI
[error] 150-150: Unexpected any. Specify a different type (@typescript-eslint/no-explicit-any)
The commit improves the UI by fixing the capture button position, adjusting its styling, and optimizing the photo grid layout with proper bottom padding and TypeScript types
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/expo/src/components/CaptureEventButton.tsx (2)
34-34: Consider using design tokens for colors.Instead of hardcoding hex values, consider using design tokens or theme variables for colors to maintain consistency and make theme changes easier.
- <Sparkles size={20} color={isDisabled ? "#627496" : "#5A32FB"} /> + <Sparkles size={20} color={isDisabled ? colors.neutral[2] : colors.primary} />
35-42: Maintain consistency with color tokens.Similar to the icon, consider using design tokens for text colors to ensure consistency throughout the application.
- isDisabled ? "text-neutral-2" : "text-[#5A32FB]", + isDisabled ? "text-neutral-2" : "text-primary",apps/expo/src/components/PhotoGrid.tsx (3)
103-103: Remove the recyclingKey prop.Based on previous learnings, the recyclingKey prop should be omitted for this component as it's designed for smaller lists of recent photos.
- recyclingKey={item.id}
51-69: Simplify the plus button press handler logic.The nested conditional logic can be simplified for better readability.
- onPress={() => { - if (hasMediaPermission && !hasFullPhotoAccess) { - ActionSheetIOS.showActionSheetWithOptions( - { - options: ["Select More Photos", "Change Settings", "Cancel"], - cancelButtonIndex: 2, - }, - (buttonIndex) => { - if (buttonIndex === 0) { - void MediaLibrary.presentPermissionsPickerAsync(); - } else if (buttonIndex === 1) { - void Linking.openSettings(); - } - }, - ); - } else { - onMorePhotos(); - } - }} + onPress={() => { + if (!hasMediaPermission || hasFullPhotoAccess) { + onMorePhotos(); + return; + } + + ActionSheetIOS.showActionSheetWithOptions( + { + options: ["Select More Photos", "Change Settings", "Cancel"], + cancelButtonIndex: 2, + }, + (buttonIndex) => { + switch (buttonIndex) { + case 0: + void MediaLibrary.presentPermissionsPickerAsync(); + break; + case 1: + void Linking.openSettings(); + break; + } + }, + ); + }}
228-232: Remove unnecessary FlatList performance optimizations.Based on previous learnings, these performance optimizations (windowSize, maxToRenderPerBatch, updateCellsBatchingPeriod, initialNumToRender, removeClippedSubviews) are not necessary for iOS implementations with a fixed/limited number of items, as iOS has better performance characteristics for FlatList rendering.
- windowSize={5} - maxToRenderPerBatch={16} - updateCellsBatchingPeriod={50} - initialNumToRender={12} - removeClippedSubviews={true}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/expo/src/app/new.tsx(3 hunks)apps/expo/src/components/CaptureEventButton.tsx(1 hunks)apps/expo/src/components/PhotoGrid.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/expo/src/app/new.tsx
🧰 Additional context used
📓 Learnings (1)
apps/expo/src/components/PhotoGrid.tsx (2)
Learnt from: jaronheard
PR: jaronheard/soonlist-turbo#375
File: apps/expo/src/components/PhotoGrid.tsx:93-171
Timestamp: 2025-01-13T03:13:45.462Z
Learning: FlatList performance optimizations (initialNumToRender, maxToRenderPerBatch, windowSize, removeClippedSubviews) are not necessary for iOS implementations with a fixed/limited number of items, as iOS has better performance characteristics for FlatList rendering.
Learnt from: jaronheard
PR: jaronheard/soonlist-turbo#375
File: apps/expo/src/components/PhotoGrid.tsx:150-160
Timestamp: 2025-01-13T03:23:22.636Z
Learning: The PhotoGrid component in the Expo app intentionally omits the recyclingKey prop for ExpoImage components as it's designed for smaller lists of recent photos, where the benefits of recycling don't outweigh potential complexities.
🔇 Additional comments (3)
apps/expo/src/components/CaptureEventButton.tsx (1)
26-33: Well-structured button implementation with proper accessibility!The Pressable component implementation follows React Native best practices with:
- Proper disabled state handling
- Consistent styling patterns using utility classes
- Good accessibility considerations
apps/expo/src/components/PhotoGrid.tsx (2)
137-143: LGTM! Effective memoization of grid data.The memoization of gridData is well implemented with appropriate dependencies.
149-149: Fix the type for _data parameter.The type for the _data parameter should be more specific.
- _data: ArrayLike<RecentPhoto & { id: string }> | null | undefined, + _: unknown[],
jaronheard
deleted the
jaronheard/tim-823-slow-photo-loading-rendering
branch
Summary by CodeRabbit
New Features
GridItemcomponent for individual photo grid items.Performance Improvements