[UI/UX] Refactor Navbar for Improved Visual Hierarchy and Responsive Design#73
Conversation
🚀 PR Received SuccessfullyHello @Aish-kul16, Thank you for taking the initiative to contribute to this project. Please ensure that your PR follows all project guidelines properly before requesting review.
|
|
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)
📝 WalkthroughWalkthroughNavbar.jsx is refactored to componentize the Tools mega menu (Tag, Chevron, MegaMenuPanel), consolidate state/keyboard/hover handlers, update user identity chip, and restyle/rewire desktop and mobile navbar rendering and interactions. ChangesNavbar Mega Menu & Layout Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@frontend/src/components/shared/Navbar.jsx`:
- Around line 108-213: MegaMenuPanel lacks keyboard focus management: when
opened focus stays on the trigger and keyboard users can't reach the items.
Update MegaMenuPanel to accept a triggerRef prop (the trigger button ref), and
in a useEffect watch megaRef to detect when the panel becomes visible; when
visible, querySelector the first focusable element inside megaRef (e.g., first
Link from MEGA_MENU_ITEMS) and call .focus(); attach a keydown handler on the
panel that (a) closes and returns focus to triggerRef.current when Escape is
pressed by calling onClose(), (b) implements a simple focus-trap for
Tab/Shift+Tab by computing all tabbable elements inside megaRef and cycling
focus so Tab/Shift+Tab never escape the panel; ensure existing interactive
elements (Links) remain focusable and clean up event listeners on unmount/close.
- Around line 272-278: The mega menu leave timer (megaLeaveTimer) can fire after
unmount and call setMegaOpen on an unmounted component; add a cleanup to
clearTimeout(megaLeaveTimer.current) when the component unmounts (or when
relevant effect dependencies change). Locate the handlers handleMegaMouseEnter
and handleMegaMouseLeave and add a useEffect cleanup that checks
megaLeaveTimer.current and calls clearTimeout to prevent the pending timeout
from running after unmount.
🪄 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: 66a8c1d6-5a97-4327-a234-c44fc74ad125
📒 Files selected for processing (1)
frontend/src/components/shared/Navbar.jsx
| function MegaMenuPanel({ megaRef, onMouseEnter, onMouseLeave, onClose }) { | ||
| return ( | ||
| <div | ||
| id="tools-mega-menu" | ||
| role="region" | ||
| aria-label="Tools menu" | ||
| ref={megaRef} | ||
| onMouseEnter={onMouseEnter} | ||
| onMouseLeave={onMouseLeave} | ||
| className="absolute top-full left-1/2 mt-4 w-[620px] bg-white border border-zinc-200 shadow-2xl shadow-black/8 z-50 rounded-sm" | ||
| style={{ transform: "translateX(-50%)" }} | ||
| > | ||
| {/* Single thin top accent — replaces aggressive border-4 */} | ||
| <div className="h-0.5 w-full bg-black rounded-t-sm" /> | ||
|
|
||
| <div className="p-5"> | ||
| {/* Section label */} | ||
| <p className="text-[10px] font-semibold uppercase tracking-[0.18em] text-zinc-400 mb-4 pb-3 border-b border-zinc-100"> | ||
| AI-Powered Tools — GSSoC '26 | ||
| </p> | ||
|
|
||
| {/* Item grid — gap-px with bg-zinc-100 creates razor-thin dividers */} | ||
| <div className="grid grid-cols-2 gap-px bg-zinc-100 rounded-[2px] overflow-hidden"> | ||
| {MEGA_MENU_ITEMS.map((item) => { | ||
| const hasSubmenu = item.submenu && item.submenu.length > 0; | ||
|
|
||
| // ── Contest Arsenal: full-width with nested submenu ──────────── | ||
| if (hasSubmenu) { | ||
| return ( | ||
| <div key={item.label} className="col-span-2"> | ||
| {/* Parent header */} | ||
| <div className="bg-zinc-900 text-white px-4 py-3"> | ||
| <div className="flex items-center gap-2"> | ||
| <span className="text-sm leading-none opacity-70">{item.icon}</span> | ||
| <span className="text-xs font-bold uppercase tracking-[0.1em]">{item.label}</span> | ||
| <Tag label={item.tag} /> | ||
| </div> | ||
| <p className="text-[11px] text-zinc-400 mt-1 pl-5 leading-snug font-normal"> | ||
| {item.desc} | ||
| </p> | ||
| </div> | ||
| {/* Submenu grid */} | ||
| <div className="grid grid-cols-2 gap-px bg-zinc-100"> | ||
| {item.submenu.map((sub) => ( | ||
| <Link | ||
| key={sub.label} | ||
| to={sub.to} | ||
| onClick={onClose} | ||
| className="group bg-white px-4 py-3 hover:bg-zinc-900 hover:text-white transition-colors duration-150" | ||
| > | ||
| <div className="flex items-center gap-2 mb-0.5"> | ||
| <span className="text-sm leading-none">{sub.icon}</span> | ||
| <span className="text-xs font-semibold uppercase tracking-[0.09em] group-hover:text-white"> | ||
| {sub.label} | ||
| </span> | ||
| </div> | ||
| <p className="text-[11px] text-zinc-400 pl-5 leading-snug group-hover:text-zinc-300 font-normal"> | ||
| {sub.desc} | ||
| </p> | ||
| </Link> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| // ── Standard grid item ───────────────────────────────────────── | ||
| const Wrapper = item.to ? Link : "div"; | ||
| const wrapperProps = item.to ? { to: item.to, onClick: onClose } : {}; | ||
|
|
||
| return ( | ||
| <Wrapper | ||
| key={item.label} | ||
| {...wrapperProps} | ||
| className={`bg-white px-4 py-3 transition-colors duration-150 ${ | ||
| item.to | ||
| ? "cursor-pointer hover:bg-zinc-900 hover:text-white group" | ||
| : "cursor-default opacity-50" | ||
| }`} | ||
| > | ||
| <div className="flex items-center gap-2 mb-0.5"> | ||
| <span className="text-sm leading-none opacity-70">{item.icon}</span> | ||
| <span className="text-xs font-semibold uppercase tracking-[0.09em]">{item.label}</span> | ||
| <Tag label={item.tag} /> | ||
| </div> | ||
| <p className="text-[11px] text-zinc-400 pl-5 leading-snug group-hover:text-zinc-300 font-normal"> | ||
| {item.desc} | ||
| </p> | ||
| </Wrapper> | ||
| ); | ||
| })} | ||
| </div> | ||
|
|
||
| {/* Footer strip */} | ||
| <div className="mt-4 pt-3 border-t border-zinc-100 flex items-center justify-between"> | ||
| <span className="text-[10px] font-medium uppercase tracking-[0.12em] text-zinc-400"> | ||
| More tools shipping soon | ||
| </span> | ||
| <span className="text-[10px] font-semibold uppercase tracking-[0.1em] text-zinc-700 underline underline-offset-2 hover:text-black cursor-pointer transition-colors"> | ||
| View All → | ||
| </span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Add keyboard navigation support for mega menu items.
The MegaMenuPanel renders interactive Links but provides no keyboard navigation pattern. When a keyboard user opens the mega menu (via Enter/Space on the trigger), focus remains on the trigger button and users cannot Tab into the panel to navigate the items. This creates an accessibility gap for keyboard-only users who cannot reach the mega menu content.
Consider implementing focus management:
- Move focus to the first menu item when the panel opens
- Enable Tab/Shift+Tab navigation within the panel
- Return focus to the trigger when the panel closes via Escape
🤖 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 `@frontend/src/components/shared/Navbar.jsx` around lines 108 - 213,
MegaMenuPanel lacks keyboard focus management: when opened focus stays on the
trigger and keyboard users can't reach the items. Update MegaMenuPanel to accept
a triggerRef prop (the trigger button ref), and in a useEffect watch megaRef to
detect when the panel becomes visible; when visible, querySelector the first
focusable element inside megaRef (e.g., first Link from MEGA_MENU_ITEMS) and
call .focus(); attach a keydown handler on the panel that (a) closes and returns
focus to triggerRef.current when Escape is pressed by calling onClose(), (b)
implements a simple focus-trap for Tab/Shift+Tab by computing all tabbable
elements inside megaRef and cycling focus so Tab/Shift+Tab never escape the
panel; ensure existing interactive elements (Links) remain focusable and clean
up event listeners on unmount/close.
|
Hi @kunalverma2512 , I’ve completed the navbar UI/UX refactor and opened this PR for review. The refactor focuses on improving:
Looking forward to your feedback. Thanks! |
|
@Aish-kul16 kindly look on coderabbitai and provide the reasoning whether its suggestion good or not also attach a video with good presentation about your work |
|
@Aish-kul16 check technical discussion if not seen |
|
Hi @kunalverma2512 , Thanks for pointing this out. The suggestion is valid from an accessibility perspective since keyboard users currently cannot properly navigate into the mega menu after opening it. I agree that basic focus management should be improved here. I’m planning to implement lightweight keyboard accessibility support (focus transfer + escape handling) without overcomplicating the current navbar architecture or UX scope. |
|
@Aish-kul16 go ahead |
|
@Aish-kul16 coderabbitai suggestions are valid and worth implementing. CodeRabbit sometimes gives optional improvements too, but in this PR both points are meaningful from accessibility and React lifecycle perspectives. Go ahead with:
Keep the implementation clean and lightweight without overengineering the navbar architecture. Once finalized, attach the demo video properly and make sure to explain things whatever you done and then we can move ahead. |
|
Hi @kunalverma2512 , I’ve implemented the CodeRabbit suggestions and pushed the accessibility + cleanup fixes to the PR.
kindly review it, thank you . |
|
@coderabbitai please perform a full review of this PR and verify whether the implementation, code quality, accessibility, responsiveness, and overall architecture are properly handled. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me read the full 🏁 Script executed: Length of output: 27950 PR description images:
✅ What's Working Well
🔴 Issues Found1. Focus Is Hijacked on Mouse Hover (High — Accessibility Bug)In const handleMegaMouseEnter = () => {
clearTimeout(megaLeaveTimer.current);
setMegaOpen(true);
setTimeout(() => {
firstMenuItemRef.current?.focus(); // ← fires on every mouse hover
}, 0);
};This means when a mouse user simply moves their cursor over "Tools", the browser's focus jumps into the mega menu. This is disruptive — it can trigger scroll jumps, break screen reader announcements, and violate WCAG 2.1 guideline 3.2.1 (On Focus). Focus should only be moved programmatically on keyboard activation. Fix: Move focus transfer exclusively into the keyboard handler on the trigger button, where it already exists: const handleMegaMouseEnter = () => {
clearTimeout(megaLeaveTimer.current);
setMegaOpen(true);
// ← do NOT call focus() here
};The trigger's if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
setMegaOpen((v) => !v);
setTimeout(() => {
firstMenuItemRef.current?.focus(); // ← only on keyboard, correct
}, 0);
}2. Mega Menu Stays Open When User Tabs Past It (Medium — Accessibility)There is no handler to close the menu when keyboard focus leaves the panel entirely. A user who opens the menu with the keyboard and then Tabs through all the links will land on whatever comes after the mega menu in the tab order, with the mega menu still visually open. Fix: Add a // In MegaMenuPanel
onBlur={(e) => {
if (
!e.currentTarget.contains(e.relatedTarget) &&
!megaTriggerRef.current?.contains(e.relatedTarget)
) {
onClose();
}
}}3. "View All →" Is Not Keyboard Accessible (Medium — Accessibility)In the mega menu footer: <span
className="... cursor-pointer ... hover:text-black"
>
View All →
</span>This element has hover styles and Fix: Either make it a proper // Option A: make it navigable
<Link to="/tools" onClick={onClose} className="...">View All →</Link>
// Option B: mark it as coming soon / non-interactive
<span className="text-[10px] font-medium ... text-zinc-400 cursor-default">
View All — Coming Soon
</span>4. Mobile Tools Accordion Missing
|
| # | Issue | Severity | Category |
|---|---|---|---|
| 1 | Focus jumps on mouse hover in handleMegaMouseEnter |
🔴 High | Accessibility |
| 2 | Mega menu doesn't close on Tab-out | 🟠 Medium | Accessibility |
| 3 | "View All →" not keyboard/screen-reader accessible | 🟠 Medium | Accessibility |
| 4 | Mobile accordion missing aria-controls |
🟠 Medium | Accessibility |
| 5 | shadow-black/8 non-standard Tailwind value |
🟠 Medium | Rendering |
| 6 | Cleanup useEffect indentation |
🟡 Low | Code Quality |
Issues 1–3 are direct accessibility concerns worth fixing before merge. Issue 1 in particular partially negates the keyboard accessibility work already done. Issues 4–6 are straightforward cleanup items.
The visual redesign itself — the spacing, typography scale, hierarchy, and brutalist language — is a genuine improvement over the original. Once these items are addressed and the demo video is attached, this should be good to go.
kunalverma2512
added
enhancement
Thank you so much for the review and guidance throughout the PR process @kunalverma2512. I also went through this remaining CodeRabbit suggestions carefully. Do you want me to work on those accessibility improvements as well in a separate follow-up PR? Do let me know . |
|
@Aish-kul16 thanks |
2 participants
Refactored the navbar UI/UX to improve visual hierarchy, spacing consistency, readability, and responsive behavior while preserving the project's brutalist design language and all existing functionality.
Screenshots
Before
-After
Closes #57
Summary by CodeRabbit
Style
New Features
Bug Fixes / Improvements