feat: add glassmorphism navbar with theme support#260
Conversation
❌ Deploy Preview for github-spy failed.
|
There was a problem hiding this comment.
🎉 Thank you @Kartikey-Pathak for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. 📝 WalkthroughWalkthroughNavbar styling is simplified to a white/gray bordered sticky header; desktop links use plain ChangesUI Styling Updates
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Navbar
participant App
Browser->>Navbar: render header and navigation
Navbar->>App: none (layout sibling)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Navbar.tsx`:
- Around line 33-57: The nav Link and button hover color (currently using
hover:text-gray-300) is too low-contrast on the light glass navbar; update those
classNames in the Navbar component (the Link elements for Home, Tracker,
Contributors, Login and the toggleTheme button) to use theme-aware hover classes
such as hover:text-gray-700 dark:hover:text-gray-300 (or equivalent light/dark
explicit classes) so the hover color is dark on light backgrounds and stays
light in dark mode; ensure you update every occurrence of hover:text-gray-300 in
those elements to the new paired classes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 63d626db-5614-4ed8-a1b4-51860f4e9d39
📒 Files selected for processing (2)
src/App.tsxsrc/components/Navbar.tsx
|
@Kartikey-Pathak : pls resolve conflicts |
f4d9c80 to
7564e45
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Navbar.tsx (1)
91-140:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winResolve broken JSX structure in mobile menu block (compile blocker).
Line 92–Line 140 mixes two different mobile-menu implementations and has unbalanced tags (notably around Line 119), which causes the parser failure reported for the
<nav>tree. Please keep one mobile-menu implementation and rebalance wrappers so all opened elements are closed exactly once.Proposed minimal cleanup
- {/* Mobile Links */} - {isOpen && ( - <div className="md:hidden bg-white dark:bg-gray-800 text-black dark:text-white"> - <div className="space-y-4 px-6 py-4"> - <Link - to="/" - className="block text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" - onClick={() => setIsOpen(false)} - > - Home - </Link> - </div> - - <div className="lg:hidden flex items-center space-x-3"> - <button onClick={toggleTheme} className="p-2 text-slate-500 dark:text-gray-400" aria-label="Toggle theme"> - {mode === "dark" ? <Sun className="h-6 w-6" /> : <Moon className="h-6 w-6" />} - </button> - - <button - onClick={() => setIsOpen(!isOpen)} - className="p-2.5 rounded-2xl bg-white/80 dark:bg-gray-800 text-slate-900 dark:text-white" - aria-label="Toggle navigation menu" - aria-expanded={isOpen} - > - {isOpen ? <X className="h-7 w-7" /> : <Menu className="h-7 w-7" />} - </button> - </div> - </div> - </div> + {/* Mobile Links */} + {isOpen && ( + <div className="md:hidden bg-white dark:bg-gray-800 text-black dark:text-white"> + <div className="space-y-4 px-6 py-4"> + <Link + to="/" + className="block text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" + onClick={() => setIsOpen(false)} + > + Home + </Link> + </div> + + <div className="lg:hidden flex items-center space-x-3"> + <button onClick={toggleTheme} className="p-2 text-slate-500 dark:text-gray-400" aria-label="Toggle theme"> + {mode === "dark" ? <Sun className="h-6 w-6" /> : <Moon className="h-6 w-6" />} + </button> + </div> + </div> + )} - {/* Mobile Menu Panel */} + {/* Mobile Menu Panel */} <div className={`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Navbar.tsx` around lines 91 - 140, The JSX has two overlapping mobile-menu implementations that leave tags unbalanced around the isOpen block—remove the duplicate/older mobile links block and keep only the intended collapsible panel (the div whose className toggles with isOpen), ensure every opened div/nav is closed exactly once, and wire the toggle controls (buttons using setIsOpen, toggleTheme and icons Menu/X/Sun/Moon) to the remaining menu; specifically keep the MobileNavLink entries and the login/signup grid inside the single responsive panel and delete the extra md:hidden block so the <nav> tree is balanced.src/App.tsx (1)
17-48:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix nested wrapper merge artifact causing invalid JSX and duplicate progress bar.
Line 17 introduced a new root
<div>but Line 19 kept the old one, and only one gets closed before</ThemeWrapper>. This is a JSX parse blocker and also duplicatesScrollProgressBarrendering (Line 18 + Line 20). Keep a single root wrapper.Proposed minimal fix
- <ThemeWrapper> - <div className="relative dark:bg-gray-800 flex flex-col min-h-screen"> - <ScrollProgressBar /> - <div className="relative flex flex-col min-h-screen"> + <ThemeWrapper> + <div className="relative flex flex-col min-h-screen dark:bg-gray-800"> {!isFullscreen && <ScrollProgressBar />} @@ - </div> + </div> </ThemeWrapper>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.tsx` around lines 17 - 48, Remove the nested/duplicated root <div> wrapper introduced around the app so JSX is valid and the ScrollProgressBar isn't rendered twice: keep a single root container (the one with className "relative flex flex-col min-h-screen" or merge the class lists) and remove the extra wrapper and its duplicate <ScrollProgressBar />; ensure the conditional rendering using isFullscreen (for ScrollProgressBar, Navbar, Footer) remains only once, that <main> with Router and the Toaster remain inside the single wrapper, and that the wrapper is properly closed before </ThemeWrapper>.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/App.tsx`:
- Around line 17-48: Remove the nested/duplicated root <div> wrapper introduced
around the app so JSX is valid and the ScrollProgressBar isn't rendered twice:
keep a single root container (the one with className "relative flex flex-col
min-h-screen" or merge the class lists) and remove the extra wrapper and its
duplicate <ScrollProgressBar />; ensure the conditional rendering using
isFullscreen (for ScrollProgressBar, Navbar, Footer) remains only once, that
<main> with Router and the Toaster remain inside the single wrapper, and that
the wrapper is properly closed before </ThemeWrapper>.
In `@src/components/Navbar.tsx`:
- Around line 91-140: The JSX has two overlapping mobile-menu implementations
that leave tags unbalanced around the isOpen block—remove the duplicate/older
mobile links block and keep only the intended collapsible panel (the div whose
className toggles with isOpen), ensure every opened div/nav is closed exactly
once, and wire the toggle controls (buttons using setIsOpen, toggleTheme and
icons Menu/X/Sun/Moon) to the remaining menu; specifically keep the
MobileNavLink entries and the login/signup grid inside the single responsive
panel and delete the extra md:hidden block so the <nav> tree is balanced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7b9ad8e-09db-458f-aacd-e2217e4afd0d
📒 Files selected for processing (2)
src/App.tsxsrc/components/Navbar.tsx
|
Closing and reopening PR due to persistent GitHub merge conflict state after syncing branch with latest |
Related Issue
Description
This PR enhances the Navbar UI by introducing a modern glassmorphism effect with proper support for both light and dark themes.
Previously, the navbar had a flat background which did not visually integrate well with the overall UI design system.
This update improves visual depth, consistency, and modern UI aesthetics while ensuring readability is maintained across all themes.
No functional logic has been changed.
How Has This Been Tested?
Verified in light mode ☀️
Verified in dark mode 🌙
Checked responsiveness on different screen sizes
Ensured navbar content remains readable over all backgrounds
Tested across multiple routes/pages
Screenshots (if applicable)
Type of Change
Summary by CodeRabbit