-
Notifications
You must be signed in to change notification settings - Fork 403
Navbar is updated with proper alignment and drop shadow #2694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@KaranUnique is attempting to deploy a commit to the Vivek Prajapati's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR redesigns the user navbar layout for improved visual alignment and consistency. Changes include logo size adjustments and a restructured navbar with fixed positioning, centered desktop navigation, and reorganized action items. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/User/components/Navbar/UserNavbar.jsx (1)
263-274: Remove unused props fromMobileMenucomponent call.The
MobileMenucomponent (line 5 ofsrc/User/components/Navbar/MobileMenu.jsx) is defined to accept only{ isOpen, isLoggedIn, username, onClose }, but it is being called with additional props:searchTerm,handleSearch,handleDropdown,openDropdown, andhandleLogout. These props are not used anywhere in the component. Remove them from the call at lines 263-274 inUserNavbar.jsxunlessMobileMenuis intended to be updated to support search, dropdown, and logout functionality.
🧹 Nitpick comments (3)
src/User/components/Navbar/NavLogo.jsx (1)
6-21: Consider making the logo sizing more robust thanvh(optional).
On very short viewports,h-[5vh]can shrink the logo more than intended; consider switching toh-*(rem-based) and addingw-auto object-containto keep aspect ratio predictable.src/User/components/Navbar/UserNavbar.jsx (2)
96-105: Dropdown “outside click” ref should include the trigger too.
Right nowdropdownRefis on the panel only; clicks on the profile icon can be treated as “outside”, which can lead to flaky open/close behavior depending on event ordering. Consider putting the ref on a wrapper that contains both the icon and the panel.Also applies to: 179-222
230-250: Add basic a11y attributes to the mobile menu button.
Recommendaria-label,aria-expanded={isOpen}, andaria-controlspointing at the mobile menu container id.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/User/components/Navbar/NavLogo.jsx(1 hunks)src/User/components/Navbar/UserNavbar.jsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/User/components/Navbar/UserNavbar.jsx (3)
src/User/components/Navbar/NavLogo.jsx (1)
NavLogo(6-24)src/User/pages/Home/SearchBar.jsx (1)
SearchBar(4-28)src/User/components/Navbar/MobileMenu.jsx (1)
div(6-104)
🔇 Additional comments (2)
src/User/components/Navbar/UserNavbar.jsx (2)
126-152: Layout/structure changes look consistent and readable.
The left/center/right split + centered desktop links and action cluster is clean, and the Tailwind utilities are coherent for the stated UI goals.Also applies to: 154-174
29-58: No action required. The SearchBar prop contract is correct.The imported SearchBar component (src/User/components/SearchBar/SearchBar.jsx) expects exactly the props being passed:
searchTerm,handleSearch,searchResults, andonResultClick. These match the component's function signature and usage perfectly. There is no prop mismatch or runtime bug here.Likely an incorrect or invalid review comment.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Congratulations, Your pull request has been successfully merged 🥳🎉 Thank you for your valuable contribution to the project 🚀 Keep contributing!! ✨ 📢 Don’t forget to share your VigyBag PR on LinkedIn and tag @VigyBag and me — we’d love to celebrate your achievement with you! 🔗💡 |
Fixes Issue #2692
Issue Description :
The current navbar layout appears visually misaligned and inconsistent with the overall UI of the website. Elements such as the logo, category links, search bar, and icons do not follow a uniform structure, which affects usability and aesthetic balance.
Example: Close #2692
Changes proposed
Screenshots
Note to reviewers
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.