Add empty state placeholders for AI tagging and home pages #549
Conversation
|
Warning Rate limit exceeded@rahulharpal1603 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughTwo gallery pages now conditionally render empty-state components when no images exist. New Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/pages/Home/Home.tsx (1)
68-92: Consider extracting empty state into a shared component.Both
Home.tsxandAITagging.tsxuse nearly identical empty state patterns with only minor variations in icons and text. Consider creating a reusableEmptyStatecomponent to reduce duplication and improve maintainability.Example component signature:
// components/EmptyState/EmptyState.tsx interface EmptyStateProps { icon: React.ComponentType<{ className?: string }>; title: string; description: string; items?: Array<{ icon: React.ComponentType<{ className?: string }>; text: string; }>; } export function EmptyState({ icon: Icon, title, description, items }: EmptyStateProps) { // Implementation with the shared structure }This would allow both pages to use:
<EmptyState icon={FolderOpen} title="No Images to Display" description="Your gallery is empty..." items={[...]} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/pages/AITagging/AITagging.tsx(2 hunks)frontend/src/pages/Home/Home.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/pages/Home/Home.tsx (1)
frontend/src/components/Media/ImageCard.tsx (1)
ImageCard(20-111)
frontend/src/pages/AITagging/AITagging.tsx (1)
frontend/src/components/Media/ImageCard.tsx (1)
ImageCard(20-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
🔇 Additional comments (6)
frontend/src/pages/AITagging/AITagging.tsx (3)
15-15: LGTM!The lucide-react icon imports follow best practices by importing specific icons for tree-shaking. The alias
ImageIconappropriately avoids naming conflict with theImagetype imported on line 6.
86-98: LGTM!The conditional rendering correctly prevents the empty grid from displaying. The grid now renders only when images are present, improving UX by ensuring users see either the empty state or the populated grid, never both.
80-80: Confirm BMP support or update the listed formats. I don’t see anybmpreferences in the frontend accept attributes or image‐processing code—please verify that the backend upload pipeline genuinely handles BMP files; if not, remove BMP from the “PNG, JPG, JPEG, GIF, BMP” claim.frontend/src/pages/Home/Home.tsx (3)
13-13: LGTM!The lucide-react icon imports follow best practices with specific icon imports for tree-shaking. The
ImageIconalias appropriately avoids conflict with theImagetype.
84-84: Verify supported image formats claim.Ensure the listed formats (PNG, JPG, JPEG, GIF, BMP) match actual backend capabilities. This is the same claim made in AITagging.tsx line 80.
The verification script from AITagging.tsx will check format references across both files.
95-106: LGTM!The conditional rendering correctly prevents empty grid rendering, ensuring users see either the empty state or the populated grid.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/src/components/EmptyStates/EmptyAITaggingState.tsx (1)
16-25: Use semantic list markup for accessibilityThese two bullet-style rows are currently just nested
<div>elements, so screen readers won’t treat them as a list. Wrap them in a<ul>/<li>structure to provide proper semantics without changing the visual layout.- <div className="flex flex-col gap-2 text-sm text-gray-400 dark:text-gray-500"> - <div className="flex items-center gap-2"> + <ul className="flex flex-col gap-2 text-sm text-gray-400 dark:text-gray-500"> + <li className="flex items-center gap-2"> <Tags className="h-4 w-4" /> <span>AI will automatically detect objects, people, and scenes</span> - </div> - <div className="flex items-center gap-2"> + </li> + <li className="flex items-center gap-2"> <ImageIcon className="h-4 w-4" /> <span>Supports PNG, JPG, JPEG, GIF image formats</span> - </div> - </div> + </li> + </ul>frontend/src/components/EmptyStates/EmptyGalleryState.tsx (1)
16-24: Adopt semantic list elements for guidance rowsLike the AI tagging empty state, these two guidance items are visually list bullets but aren’t announced as a list. Switching to
<ul>/<li>keeps styling intact while improving accessibility.- <div className="flex flex-col gap-2 text-sm text-gray-400 dark:text-gray-500"> - <div className="flex items-center gap-2"> + <ul className="flex flex-col gap-2 text-sm text-gray-400 dark:text-gray-500"> + <li className="flex items-center gap-2"> <ImageIcon className="h-4 w-4" /> <span>Supported formats: PNG, JPG, JPEG, GIF</span> - </div> - <div className="flex items-center gap-2"> + </li> + <li className="flex items-center gap-2"> <FolderOpen className="h-4 w-4" /> <span>Go to Settings to add folders</span> - </div> - </div> + </li> + </ul>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/EmptyStates/EmptyAITaggingState.tsx(1 hunks)frontend/src/components/EmptyStates/EmptyGalleryState.tsx(1 hunks)frontend/src/pages/AITagging/AITagging.tsx(2 hunks)frontend/src/pages/Home/Home.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/pages/Home/Home.tsx
- frontend/src/pages/AITagging/AITagging.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Backend Tests
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/pages/AITagging/AITagging.tsx (2)
52-55: Fix broken scrollableRef attachment.The
scrollableRefis defined but the ref is never attached to the scrollable container div (line 54). This breaks theTimelineScrollbarfunctionality that depends on this ref to perform scroll operations.Apply this diff to reattach the ref:
<div ref={scrollableRef} className="hide-scrollbar flex-1 overflow-x-hidden overflow-y-auto" >Note: The ref attribute appears to already be present on line 54 in the provided code. If this is the case and the static analysis is incorrect, you can safely disregard this comment. However, please verify that the ref is properly attached.
26-26: TimelineScrollbar functionality is broken.The
monthMarkersstate is initialized but never populated after removing theChronologicalGallerycomponent. TheTimelineScrollbaron lines 88-94 depends on this state, rendering it non-functional.Consider one of these solutions:
- If timeline scrolling is no longer needed, remove the
TimelineScrollbarand related state:- const [monthMarkers, setMonthMarkers] = useState<MonthMarker[]>([]);- {monthMarkers.length > 0 && ( - <TimelineScrollbar - scrollableRef={scrollableRef} - monthMarkers={monthMarkers} - className="absolute top-0 right-0 h-full w-4" - /> - )}
- If timeline scrolling should be preserved, populate
monthMarkersfrom the image data or restore theChronologicalGalleryintegration.Also applies to: 88-94
frontend/src/pages/Home/Home.tsx (1)
23-24: TimelineScrollbar setup is broken.The
scrollableRefis defined (line 23) but is no longer attached to any DOM element after removing the scrollable container structure. ThemonthMarkersstate (line 24) is also never populated. This makes theTimelineScrollbaron lines 90-97 non-functional.Consider one of these solutions:
- If timeline scrolling is no longer needed, remove the related code:
- const scrollableRef = useRef<HTMLDivElement>(null); - const [monthMarkers, setMonthMarkers] = useState<MonthMarker[]>([]);- {/* Timeline Scrollbar */} - {monthMarkers.length > 0 && ( - <TimelineScrollbar - scrollableRef={scrollableRef} - monthMarkers={monthMarkers} - className="absolute top-0 right-0 h-full w-4" - /> - )}
- If timeline scrolling should be preserved, wrap the content in a scrollable container and reattach the ref:
<div className="p-6"> + <div ref={scrollableRef} className="overflow-y-auto"> <h1 className="mb-6 text-2xl font-bold">{title}</h1> {/* ... rest of content ... */} + </div>Then populate
monthMarkersfrom the image data.Also applies to: 90-97
🧹 Nitpick comments (2)
frontend/src/pages/AITagging/AITagging.tsx (1)
16-18: Remove unused ChronologicalGallery import.The
ChronologicalGalleryandMonthMarkerimports are no longer used after switching to the manual grid layout.Apply this diff to remove the unused imports:
-import { - ChronologicalGallery, - MonthMarker, -} from '@/components/Media/ChronologicalGallery';frontend/src/pages/Home/Home.tsx (1)
4-6: Remove unused ChronologicalGallery import.The
ChronologicalGalleryandMonthMarkerimports are no longer used after switching to the manual grid layout.Apply this diff to remove the unused imports:
-import { - ChronologicalGallery, - MonthMarker, -} from '@/components/Media/ChronologicalGallery';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/pages/AITagging/AITagging.tsx(2 hunks)frontend/src/pages/Home/Home.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/pages/Home/Home.tsx (2)
frontend/src/components/EmptyStates/EmptyGalleryState.tsx (1)
EmptyGalleryState(3-28)frontend/src/components/Media/ImageCard.tsx (1)
ImageCard(20-111)
frontend/src/pages/AITagging/AITagging.tsx (2)
frontend/src/components/EmptyStates/EmptyAITaggingState.tsx (1)
EmptyAITaggingState(3-28)frontend/src/components/Media/ImageCard.tsx (1)
ImageCard(20-111)
🪛 GitHub Check: Linting
frontend/src/pages/Home/Home.tsx
[failure] 80-80:
'ImageCard' is not defined
frontend/src/pages/AITagging/AITagging.tsx
[failure] 74-74:
'ImageCard' is not defined
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Backend Tests
🔇 Additional comments (4)
frontend/src/pages/AITagging/AITagging.tsx (2)
67-68: LGTM: Empty state conditional rendering is correct.The empty state is correctly shown only when there are no tagged images and loading is complete.
71-82: LGTM: Image grid implementation is correct.The responsive grid layout with conditional rendering is implemented correctly and will provide a good user experience.
frontend/src/pages/Home/Home.tsx (2)
73-74: LGTM: Empty state conditional rendering is correct.The empty state is correctly shown only when there are no images and loading is complete.
77-88: LGTM: Image grid implementation is correct.The responsive grid layout with conditional rendering is implemented correctly and maintains proper indexing for the MediaView modal.
…and add empty state handling
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/pages/AITagging/AITagging.tsx(2 hunks)frontend/src/pages/Home/Home.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/AITagging/AITagging.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/pages/Home/Home.tsx (2)
frontend/src/components/Media/ChronologicalGallery.tsx (1)
ChronologicalGallery(26-173)frontend/src/components/EmptyStates/EmptyGalleryState.tsx (1)
EmptyGalleryState(3-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tauri Build Check (windows-latest)
🔇 Additional comments (1)
frontend/src/pages/Home/Home.tsx (1)
17-17: LGTM!The import statement is correct and the EmptyGalleryState component is properly imported.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/components/EmptyStates/EmptySearchState.tsx (1)
1-28: Clean implementation; addaria-hiddento decorative icons.The component structure is clear and provides helpful guidance for zero search results. However, the decorative icons should be marked
aria-hidden="true"to prevent screen readers from announcing them redundantly.Apply this diff to improve accessibility:
export const EmptySearchState = () => ( <div className="flex flex-col items-center justify-center py-16 text-center"> <div className="mb-6 rounded-full bg-gray-100 p-4 dark:bg-gray-800"> - <Search className="h-16 w-16 text-gray-400" /> + <Search className="h-16 w-16 text-gray-400" aria-hidden="true" /> </div> <h2 className="mb-2 text-xl font-semibold text-gray-700 dark:text-gray-300"> No Search Results </h2> <p className="mb-6 max-w-md text-gray-500 dark:text-gray-400"> No images match your search criteria. Try adjusting your search or try a different face image. </p> <div className="flex flex-col gap-2 text-sm text-gray-400 dark:text-gray-500"> <div className="flex items-center gap-2"> - <Search className="h-4 w-4" /> + <Search className="h-4 w-4" aria-hidden="true" /> <span>Try adjusting the face image selection</span> </div> <div className="flex items-center gap-2"> - <Search className="h-4 w-4" /> + <Search className="h-4 w-4" aria-hidden="true" /> <span> Make sure the face is clearly visible in the reference image </span> </div> </div> </div> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/EmptyStates/EmptyGalleryState.tsx(1 hunks)frontend/src/components/EmptyStates/EmptySearchState.tsx(1 hunks)frontend/src/pages/Home/Home.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/components/EmptyStates/EmptyGalleryState.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/pages/Home/Home.tsx (3)
frontend/src/components/Media/ChronologicalGallery.tsx (1)
ChronologicalGallery(26-173)frontend/src/components/EmptyStates/EmptySearchState.tsx (1)
EmptySearchState(3-28)frontend/src/components/EmptyStates/EmptyGalleryState.tsx (1)
EmptyGalleryState(3-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Backend Tests
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
🔇 Additional comments (1)
frontend/src/pages/Home/Home.tsx (1)
17-18: Excellent fix for the empty state logic!The imports and conditional rendering now correctly distinguish between an empty gallery and zero search results. When a search is active and returns no matches, users see EmptySearchState with appropriate guidance instead of the misleading "Your gallery is empty" message. This resolves the previous major issue.
Also applies to: 77-89
Removed EmptySearchState component from Home page rendering logic.
Updated text to clarify AI tagging functionality and modified the Bot component's stroke width.
Updated the EmptyGalleryState component to improve text clarity and adjusted the stroke width of the FolderOpen icon.
rahulharpal1603
left a comment
rahulharpal1603
left a comment
There was a problem hiding this comment.
Thanks @Hemil36 !
2 participants
This pull request enhances the user experience in the
HomeandAITaggingpages by introducing visually appealing and informative empty state placeholders. These placeholders guide users when there are no images to display, providing clear instructions and context-aware suggestions. Additionally, relevant icon imports have been added to support these UI updates.UI/UX Improvements:
Homepage when no images are available, including icons, helpful text, and action prompts for users to add folders or images.AITaggingpage for cases when no images have been AI-tagged yet, informing users about AI tagging features and supported formats.Conditional Rendering:
Summary by CodeRabbit