fix: add aria-expanded to hamburger, aria-label to mobile theme toggl…#324
Conversation
…e, fix active route highlighting for sub-routes
|
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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughNavbar.tsx receives three critical accessibility fixes from issue ChangesNavbar Accessibility and Active State Improvements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 2
🤖 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 `@client/src/components/Navbar.tsx`:
- Around line 334-340: Replace the raw HTML <button> in Navbar.tsx with the
shared Button component from client/src/components/ui/button.tsx: import Button,
use the Button in place of the element that calls handleLogout and setIsOpen
(the onClick should call handleLogout(); setIsOpen(false)), map the existing
classes/behavior to Button props (choose appropriate variant — likely "ghost" or
"danger" per style guidelines — and size "sm" and apply full-width styling via
className="w-full text-left" if needed), and remove the old border/hover classes
so the reusable Button handles styling and accessibility.
- Around line 84-87: The active-route calculation for Navbar is too permissive
and marks sibling routes as active; update the logic that sets the active
variable (the block using item.href and location.pathname) to treat a route as
active only if location.pathname === item.href OR
location.pathname.startsWith(item.href + "/") — keep the existing special-case
for item.href === "/" to match only root — so replace the startsWith(item.href)
check with a startsWith(item.href + "/") check combined with an exact equality
check to avoid false positives.
🪄 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: cd71f7e4-1e25-477f-8ee3-997df57ce1ac
📒 Files selected for processing (1)
client/src/components/Navbar.tsx
Fixes #169
Changes
1. Hamburger button — added
aria-expandedThe mobile menu toggle was missing
aria-expanded, screen readerscouldn't tell if the menu was open or closed. Fixed by binding it
to the
isOpenstate along with anaria-label.2. Mobile theme toggle — added
aria-labelThe theme toggle button on mobile was icon-only with no label.
Added
aria-labelthat updates based on the current theme,matching the desktop button behaviour.
3. Active route highlight — fixed for sub-routes
Nav items weren't getting highlighted on nested routes like
/learn/dsabecause it was using strict equality (
===). Replaced withstartsWith()with a special case for
/so Home doesn't stay always active.Files Changed
client/src/components/Navbar.tsxSummary by CodeRabbit
Release Notes
Bug Fixes
Style
Note: This PR includes accessibility fixes (aria attributes) —
open to any label the maintainer sees fit.