Fixed navbar hover contrast issue#235
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates Navbar styling: logo hover changed to blue, desktop navigation link classes adjusted (including a distinct Tracker hover border), and the mobile menu was reworked to add ChangesNavbar styling and mobile menu restructuring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
🎉 Thank you @SiddhiMohite20 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
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 (2)
src/components/Navbar.tsx (2)
43-54:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncomplete update: Contributors and Login links still use old gray-based hover styling.
The PR aims to improve navbar hover contrast, but only the Home and Tracker links were updated to blue-based hover colors. The Contributors and Login links still use the old
hover:text-gray-300andhover:border-gray-400, creating inconsistency across the navigation bar.♻️ Proposed fix to update remaining links
<Link to="/contributors" - className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" + className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" > Contributors </Link> <Link to="/login" - className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" + className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" > Login </Link>🤖 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 43 - 54, Update the two Link components for "Contributors" and "Login" in Navbar (the Link elements around the "Contributors" and "Login" text) to use the same blue-based hover utility classes as the other nav items: replace the current hover classes `hover:text-gray-300` and `hover:border-gray-400` with the blue variants used elsewhere (e.g., `hover:text-blue-300` and `hover:border-blue-400`) so all nav links share consistent hover text and border styles.
30-54:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd missing About and Contact routes to desktop navigation.
The mobile menu includes
/aboutand/contactlinks (lines 100–112), but these are absent from the desktop navigation (lines 30–54). Both route components exist in the application (src/pages/About/About.tsxandsrc/pages/Contact/Contact.tsx), creating an inconsistent experience where mobile users can access routes that desktop users cannot.Proposed fix
<div className="hidden md:flex space-x-6"> <Link to="/" className="text-lg font-medium hover:text-blue-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" > Home </Link> <Link to="/track" className="text-lg font-medium text-black hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" > Tracker </Link> + <Link + to="/about" + className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" + > + About + </Link> + <Link + to="/contact" + className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" + > + Contact + </Link> <Link to="/contributors" className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" > Contributors </Link>🤖 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 30 - 54, The desktop navigation (the div with className "hidden md:flex space-x-6") is missing Link entries for the About and Contact routes; add two Link components for to="/about" (text "About") and to="/contact" (text "Contact") inside that same div, using the same styling pattern/classes as the existing Link elements (e.g., mirror the hover/text/border classes used for Contributors or Tracker) so desktop and mobile menus contain the same routes.
🧹 Nitpick comments (1)
src/components/Navbar.tsx (1)
92-126: ⚡ Quick winMobile menu:
hover:border-blackmay have poor contrast in dark mode.All mobile navigation links use
hover:border-black, which will render as a black border on thedark:bg-gray-800background in dark mode. This creates low contrast and may not be as visible as intended.♻️ Proposed fix for better dark mode border visibility
<Link to="/" - className="block text-lg font-medium hover:text-blue-500 transition-all px-2 py-1 border border-transparent hover:border-black rounded" + className="block text-lg font-medium hover:text-blue-500 transition-all px-2 py-1 border border-transparent hover:border-black dark:hover:border-gray-400 rounded" onClick={() => setIsOpen(false)} > Home </Link>Apply the same change to all mobile navigation links (About, Contact, Contributors, and Login) to add
dark:hover:border-gray-400for better visibility in dark mode.🤖 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 92 - 126, The mobile nav Link elements in Navbar (the Link components for Home, About, Contact, Contributors, Login) use hover:border-black which has poor contrast in dark mode; update each Link's className to include dark:hover:border-gray-400 (in addition to the existing hover:border-black) so the hover border is visible on dark backgrounds and keep the onClick={() => setIsOpen(false)} behavior unchanged.
🤖 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 31-42: The Home and Tracker Link elements in Navbar.tsx have
inconsistent hover styles (Home uses hover:text-blue-300; Tracker uses
hover:text-blue-600 and hover:border-blue-400) and the Tracker also forces
text-black; update the Link for "/" (Home) to match the Tracker hover behavior
by using hover:text-blue-600 and hover:border-blue-400 and remove the explicit
text-black from the Link for "/track" so both links share the same hover/font
behavior; locate the Link elements with to="/" and to="/track" in the Navbar
component and make these style changes.
- Line 39: The Tracker link's className in the Navbar component includes an
explicit "text-black" which overrides dark mode; remove "text-black" (and the
extra space) from the className string on the Tracker link so the parent's
"dark:text-white" can take effect, leaving the other utility classes
(hover:text-blue-600, transition-all, px-2, py-1, border border-transparent,
hover:border-blue-400, rounded) intact.
---
Outside diff comments:
In `@src/components/Navbar.tsx`:
- Around line 43-54: Update the two Link components for "Contributors" and
"Login" in Navbar (the Link elements around the "Contributors" and "Login" text)
to use the same blue-based hover utility classes as the other nav items: replace
the current hover classes `hover:text-gray-300` and `hover:border-gray-400` with
the blue variants used elsewhere (e.g., `hover:text-blue-300` and
`hover:border-blue-400`) so all nav links share consistent hover text and border
styles.
- Around line 30-54: The desktop navigation (the div with className "hidden
md:flex space-x-6") is missing Link entries for the About and Contact routes;
add two Link components for to="/about" (text "About") and to="/contact" (text
"Contact") inside that same div, using the same styling pattern/classes as the
existing Link elements (e.g., mirror the hover/text/border classes used for
Contributors or Tracker) so desktop and mobile menus contain the same routes.
---
Nitpick comments:
In `@src/components/Navbar.tsx`:
- Around line 92-126: The mobile nav Link elements in Navbar (the Link
components for Home, About, Contact, Contributors, Login) use hover:border-black
which has poor contrast in dark mode; update each Link's className to include
dark:hover:border-gray-400 (in addition to the existing hover:border-black) so
the hover border is visible on dark backgrounds and keep the onClick={() =>
setIsOpen(false)} behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| <Link | ||
| to="/" | ||
| className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||
| className="text-lg font-medium hover:text-blue-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||
| > | ||
| Home | ||
| </Link> | ||
| <Link | ||
| to="/track" | ||
| className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||
| className="text-lg font-medium text-black hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" | ||
| > | ||
| Tracker | ||
| </Link> |
There was a problem hiding this comment.
Inconsistent hover styling between Home and Tracker links.
The Home link uses hover:text-blue-300 while the Tracker link uses hover:text-blue-600. This inconsistency creates a confusing visual experience where similar navigation items behave differently on hover.
♻️ Proposed fix for consistent styling
<Link
to="/"
- className="text-lg font-medium hover:text-blue-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded"
+ className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded"
>
Home
</Link>
<Link
to="/track"
- className="text-lg font-medium text-black hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded"
+ className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded"
>
Tracker
</Link>This unifies both links to use hover:text-blue-600 and hover:border-blue-400, and removes the problematic explicit text-black.
📝 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.
| <Link | |
| to="/" | |
| className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | |
| className="text-lg font-medium hover:text-blue-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | |
| > | |
| Home | |
| </Link> | |
| <Link | |
| to="/track" | |
| className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | |
| className="text-lg font-medium text-black hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" | |
| > | |
| Tracker | |
| </Link> | |
| <Link | |
| to="/" | |
| className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" | |
| > | |
| Home | |
| </Link> | |
| <Link | |
| to="/track" | |
| className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" | |
| > | |
| Tracker | |
| </Link> |
🤖 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 31 - 42, The Home and Tracker Link
elements in Navbar.tsx have inconsistent hover styles (Home uses
hover:text-blue-300; Tracker uses hover:text-blue-600 and hover:border-blue-400)
and the Tracker also forces text-black; update the Link for "/" (Home) to match
the Tracker hover behavior by using hover:text-blue-600 and
hover:border-blue-400 and remove the explicit text-black from the Link for
"/track" so both links share the same hover/font behavior; locate the Link
elements with to="/" and to="/track" in the Navbar component and make these
style changes.
| <Link | ||
| to="/track" | ||
| className="text-lg font-medium hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" | ||
| className="text-lg font-medium text-black hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" |
There was a problem hiding this comment.
Critical: Explicit text-black breaks dark mode visibility.
The Tracker link specifies text-black without a dark mode variant. This will override the parent's dark:text-white class, rendering the link invisible (or extremely low contrast) in dark mode.
🐛 Proposed fix
- className="text-lg font-medium text-black hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded"
+ className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded"Remove the explicit text-black to allow the parent's text-black dark:text-white to apply correctly. Also fixes the double space.
📝 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.
| className="text-lg font-medium text-black hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" | |
| className="text-lg font-medium hover:text-blue-600 transition-all px-2 py-1 border border-transparent hover:border-blue-400 rounded" |
🤖 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` at line 39, The Tracker link's className in the
Navbar component includes an explicit "text-black" which overrides dark mode;
remove "text-black" (and the extra space) from the className string on the
Tracker link so the parent's "dark:text-white" can take effect, leaving the
other utility classes (hover:text-blue-600, transition-all, px-2, py-1, border
border-transparent, hover:border-blue-400, rounded) intact.
Related Issue
--- #233
Description
--- Improved Navbar hover text visibility by updating hover colors for better readability and accessibility on light background.
How Has This Been Tested?
--- 1.Ran the project locally using Vite
2.Tested navbar hover interactions on Chrome browser
3.Verified improved text visibility on hover state
Type of Change
Summary by CodeRabbit
New Features
Style