fix: improve navbar hover contrast readability#282
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR updates the Navbar component's hover styling to improve navigation link readability. Logo, desktop navigation, and mobile navigation links are restyled with consistent bordered, rounded corners, and theme-aware hover text colors to address contrast issues during interaction. ChangesNavigation Link Hover Styling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @pari-dubey1 for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
127-135: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winMobile theme toggle button has the same hover contrast issue.
Like the desktop theme toggle button, the mobile version also uses
hover:text-gray-300without a dark mode variant. This creates the same light mode contrast issue and should be updated to match the navigation links pattern.🎨 Proposed fix to match navigation link hover pattern
<button onClick={() => { toggleTheme(); setIsOpen(false); }} - className="text-sm font-semibold px-3 py-1 rounded border border-gray-500 hover:text-gray-300 hover:border-gray-300 transition duration-200 w-full text-left" + className="text-sm font-semibold px-3 py-1 rounded border border-gray-500 hover:text-gray-700 dark:hover:text-gray-300 hover:border-gray-400 transition duration-200 w-full text-left" >🤖 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 127 - 135, The mobile theme toggle button uses a plain hover class (hover:text-gray-300) that lacks a dark-mode variant, causing poor contrast; update the button's hover classes to match the navigation links pattern by adding the appropriate dark-mode hover class (e.g., dark:hover:text-...) so the element rendered by the onClick handler (toggleTheme + setIsOpen) and its label ({mode}) uses the same hover behavior in dark mode as the desktop nav links.
55-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTheme toggle buttons fail WCAG AA contrast in light mode.
The desktop and mobile theme toggle buttons (lines 57 and 132) use
hover:text-gray-300 hover:border-gray-300, which produces only 1.47:1 contrast on a white background—failing WCAG AA standards (requires 4.5:1 minimum for normal text).The navigation links already use the correct pattern:
hover:text-gray-700 dark:hover:text-gray-300 hover:border-gray-400, which achieves 10.31:1 contrast in light mode. Apply the same styling to both theme toggle buttons for consistency and accessibility compliance.Suggested fix
<button onClick={toggleTheme} - className="text-sm font-semibold px-3 py-1 rounded border border-gray-500 hover:text-gray-300 hover:border-gray-300 transition duration-200" + className="text-sm font-semibold px-3 py-1 rounded border border-gray-500 hover:text-gray-700 dark:hover:text-gray-300 hover:border-gray-400 transition duration-200" >🤖 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 55 - 60, The theme toggle buttons (the button using toggleTheme and rendering Sun/Moon based on mode) use low-contrast classes; update their className to match the nav link pattern by replacing hover:text-gray-300 hover:border-gray-300 with hover:text-gray-700 dark:hover:text-gray-300 hover:border-gray-400 so both the desktop and mobile toggle buttons use the accessible hover styles and pass WCAG AA.
🧹 Nitpick comments (2)
src/components/Navbar.tsx (2)
23-23: ⚡ Quick winConsider adding consistent hover border styling to the logo link.
The logo link has the same hover text colors as the navigation links but lacks the border and rounded corner styling (
px-2 py-1 border border-transparent hover:border-gray-400 rounded) that was added to other nav links. This creates inconsistent hover feedback across navigation elements.For visual consistency, consider applying the same bordered hover effect to the logo link.
✨ Suggested styling update for consistency
- className="text-2xl font-bold hover:text-gray-700 dark:hover:text-gray-300 cursor-pointer flex items-center" + className="text-2xl font-bold hover:text-gray-700 dark:hover:text-gray-300 cursor-pointer flex items-center px-2 py-1 border border-transparent hover:border-gray-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 23, The logo link in the Navbar component currently uses the className "text-2xl font-bold hover:text-gray-700 dark:hover:text-gray-300 cursor-pointer flex items-center" and lacks the bordered hover styling applied to other nav links; update the logo link's className in src/components/Navbar.tsx (the element that renders the logo) to include "px-2 py-1 border border-transparent hover:border-gray-400 rounded" in addition to the existing classes so its hover feedback matches the other navigation links.
33-54: ⚡ Quick winConsider extracting repeated className to a constant.
The same className string is repeated across all desktop navigation links (lines 33, 39, 45, 51):
"text-lg font-medium hover:text-gray-700 dark:hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded"Extracting this to a constant would improve maintainability and ensure consistency across future updates.
♻️ Refactor suggestion to reduce duplication
const Navbar: React.FC = () => { const [isOpen, setIsOpen] = useState<boolean>(false); const themeContext = useContext(ThemeContext); + + const navLinkClasses = "text-lg font-medium hover:text-gray-700 dark:hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded"; if (!themeContext)Then use it in the links:
<Link to="/" - className="text-lg font-medium hover:text-gray-700 dark:hover:text-gray-300 transition-all px-2 py-1 border border-transparent hover:border-gray-400 rounded" + className={navLinkClasses} >Apply the same pattern to the other desktop nav links and mobile nav links (with
block ${navLinkClasses}for mobile).🤖 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 33 - 54, The repeated className string used in the Navbar component's Link elements should be extracted to a single constant (e.g., const navLinkClasses) and then referenced in each desktop Link (currently the Home, Tracker, Contributors, Login Link elements) to eliminate duplication; update mobile link usages to use `block ${navLinkClasses}` or equivalent to preserve layout and ensure consistency across both desktop and mobile navigation.
🤖 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/components/Navbar.tsx`:
- Around line 127-135: The mobile theme toggle button uses a plain hover class
(hover:text-gray-300) that lacks a dark-mode variant, causing poor contrast;
update the button's hover classes to match the navigation links pattern by
adding the appropriate dark-mode hover class (e.g., dark:hover:text-...) so the
element rendered by the onClick handler (toggleTheme + setIsOpen) and its label
({mode}) uses the same hover behavior in dark mode as the desktop nav links.
- Around line 55-60: The theme toggle buttons (the button using toggleTheme and
rendering Sun/Moon based on mode) use low-contrast classes; update their
className to match the nav link pattern by replacing hover:text-gray-300
hover:border-gray-300 with hover:text-gray-700 dark:hover:text-gray-300
hover:border-gray-400 so both the desktop and mobile toggle buttons use the
accessible hover styles and pass WCAG AA.
---
Nitpick comments:
In `@src/components/Navbar.tsx`:
- Line 23: The logo link in the Navbar component currently uses the className
"text-2xl font-bold hover:text-gray-700 dark:hover:text-gray-300 cursor-pointer
flex items-center" and lacks the bordered hover styling applied to other nav
links; update the logo link's className in src/components/Navbar.tsx (the
element that renders the logo) to include "px-2 py-1 border border-transparent
hover:border-gray-400 rounded" in addition to the existing classes so its hover
feedback matches the other navigation links.
- Around line 33-54: The repeated className string used in the Navbar
component's Link elements should be extracted to a single constant (e.g., const
navLinkClasses) and then referenced in each desktop Link (currently the Home,
Tracker, Contributors, Login Link elements) to eliminate duplication; update
mobile link usages to use `block ${navLinkClasses}` or equivalent to preserve
layout and ensure consistency across both desktop and mobile navigation.
|
🎉🎉 Thank you for your contribution! Your PR #282 has been merged! 🎉🎉 |
2 participants
Related Issue
Description
Fixed the navbar hover contrast issue where navigation links became difficult to read on hover due to low text visibility.
Updated hover text colors to improve readability and maintain proper contrast in both light and dark themes.
Changes made:
How Has This Been Tested?
Screenshots
Before:
After:
Type of Change
Summary by CodeRabbit