ui(empty-state): make Settings link clickable in empty gallery #964
Conversation
|
|
📝 WalkthroughWalkthroughThe EmptyGalleryState component now imports Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
🤖 Fix all issues with AI Agents
In @frontend/src/components/EmptyStates/EmptyGalleryState.tsx:
- Around line 22-32: The button in EmptyGalleryState.tsx currently removes the
focus outline via the class "focus:outline-none"; replace that with a visible
focus indicator (e.g., use Tailwind focus-visible or focus classes such as
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-400" or
"focus:ring-2 focus:ring-blue-400") on the Settings button (the button with
onClick={() => navigate("/settings")}) so keyboard users get a clear, accessible
focus state while preserving hover/visual styles.
🧹 Nitpick comments (1)
frontend/src/components/EmptyStates/EmptyGalleryState.tsx (1)
24-30: Consider using theLinkcomponent for better semantics and accessibility.While the button approach works, using React Router's
Linkcomponent is more semantic for navigation and provides better accessibility (e.g., right-click to open in a new tab, visible href for screen readers).🔎 Alternative implementation using Link
First, update the import on line 2:
-import { useNavigate } from "react-router"; +import { Link } from "react-router";Then replace the button with a Link:
- <span> - Go to{" "} - <button - type="button" - onClick={() => navigate("/settings")} - className="text-blue-500 hover:underline focus:outline-none" - > - Settings - </button>{" "} - to add folders. - </span> + <span> + Go to{" "} + <Link + to="/settings" + className="text-blue-500 hover:underline focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2" + > + Settings + </Link>{" "} + to add folders. + </span>And remove the
navigatehook initialization (lines 5-6) if it's no longer needed elsewhere.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/EmptyStates/EmptyGalleryState.tsx
🔇 Additional comments (2)
frontend/src/components/EmptyStates/EmptyGalleryState.tsx (2)
5-5: LGTM!The
useNavigatehook is correctly initialized at the component level following React's Rules of Hooks.
2-2: The import statement is correct for React Router 7. In v7,useNavigateis imported from"react-router"(not"react-router-dom"as in v6).
|
Hi @rahulharpal1603 , Please let me know if this looks good or if you’d like any changes. |
|
Hi @rahulharpal1603 , I’ve updated the navigation to remove hard-coded paths and now use the centralized routes provided in the project. Please let me know if this looks good. |
1 participant
Fixes #965
🧭 Make “Settings” link clickable in empty gallery
What
Makes the “Go to Settings to add folders” text in the empty gallery state clickable and navigates directly to the Settings page.
Why
When users land on an empty gallery, adding folders is the next obvious action.
Making the Settings link clickable reduces friction and improves first-time user experience.
Scope
EmptyGalleryStateImplementation
📸 Screenshots
Before
Plain text, not clickable.
After
Settings is clickable and navigates directly to the Settings page.
Testing
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.