add FAQ section to nav bar #124
Conversation
|
@ssscharan149 is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
@ssscharan149 hey, is it mobile responsive? |
|
No sir. |
WalkthroughAdded Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Btn as Menu Button
participant State as React State (isOpen)
participant Nav as Navbar UI
participant Doc as Document (Escape)
User->>Btn: click
Btn->>State: toggle isOpen
State->>Nav: re-render
alt isOpen == true
Nav-->>User: show mobile menu (X icon) with nav items
else isOpen == false
Nav-->>User: hide mobile menu (Menu icon)
end
User->>Nav: click nav item (e.g., FAQ / Get Started / Contribute)
Nav->>State: set isOpen = false
State->>Nav: re-render (menu closes)
Doc->>State: Escape key pressed
State->>Nav: set isOpen = false & blur active element
Nav-->>User: menu closed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (3)
apps/web/src/components/faq/FaqSection.tsx (1)
12-12: Remove spaces around the equals sign in JSX attribute.The
idattribute has spaces around the=operator, which is not idiomatic in JSX. While it works functionally, it's inconsistent with standard JSX formatting.Apply this diff to fix the formatting:
- <div className="flex flex-col border-b border-[#252525]" id = "faq"> + <div className="flex flex-col border-b border-[#252525]" id="faq">apps/web/src/components/landing-sections/navbar.tsx (2)
16-16: Add space after comma for consistency.The state declaration is missing a space after the comma, which is inconsistent with standard JavaScript formatting conventions.
Apply this diff:
- const [isOpen,setIsOpen] = useState(false); + const [isOpen, setIsOpen] = useState(false);
46-50: Consider removing the unnecessary wrapper div.The button is wrapped in a
<div>that serves no styling or layout purpose. You can simplify by removing it.Apply this diff:
- <div> - <button className="md:hidden text-white" onClick={()=> setIsOpen(!isOpen)}> + <button className="md:hidden text-white" onClick={() => setIsOpen(!isOpen)}> - {isOpen ? <X size={28}/> : <Menu size={28}/>} - </button> - </div> + {isOpen ? <X size={28} /> : <Menu size={28} />} + </button>Note: Also adds spaces before
/>for consistency with JSX formatting conventions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/faq/FaqSection.tsx(1 hunks)apps/web/src/components/landing-sections/navbar.tsx(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/components/landing-sections/navbar.tsx (1)
apps/web/src/components/icons/icons.tsx (2)
Github(195-197)Terminal(1-5)
🔇 Additional comments (2)
apps/web/src/components/landing-sections/navbar.tsx (2)
6-6: LGTM!The additional icon imports (
MenuandX) are correctly added for the mobile menu toggle functionality.
79-79: LGTM!Correctly hides the desktop navigation buttons on mobile, as they are now included in the mobile menu.
| {isOpen && ( | ||
| <motion.div | ||
| initial={{opacity:0,y:-10}} | ||
| animate={{opacity:1,y:0}} | ||
| transition={{duration:0.25}} | ||
| className="absolute top-full left-0 w-full bg-neutral-900/90 backdrop-blur-xl border-t border-white/10 md:hidden flex flex-col items-center py-5 space-y-4 z-50" | ||
| > | ||
| {links.map((link,index)=>( | ||
| <Link key={index} href={link.href} onClick={()=>setIsOpen(false)} className="text-white hover:text-gray-300 text-lg">{link.name} | ||
| </Link> | ||
|
|
||
| ))} | ||
|
|
||
| <Link | ||
| href="https://github.com/apsinghdev/opensox" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| onClick={()=>setIsOpen(false)} | ||
| className="flex items-center gap-2 px-4 py-2 bg-[0#d1117] hover:bg-[#161b22] rounded-lg border border-[#30363d] text-white transition-colors" | ||
| > | ||
| <Github className="w-5 h-5" /> | ||
| <span className="text-sm font-medium">Contribute</span> | ||
| </Link> | ||
|
|
||
| <Link href="/dashboard/home" onClick={()=> setIsOpen(false)} className="cursor-pointer z-30"> | ||
| <PrimaryButtom> | ||
| <Terminal /> | ||
| Get Started | ||
| </PrimaryButtom> | ||
| </Link> | ||
|
|
||
|
|
||
| </motion.div>)} |
There was a problem hiding this comment.
Critical: Fix typo in background color class.
Line 115 has a typo in the Tailwind class: bg-[0#d1117] should be bg-[#0d1117]. The current value is invalid and won't apply the intended background color.
Apply this diff:
- className="flex items-center gap-2 px-4 py-2 bg-[0#d1117] hover:bg-[#161b22] rounded-lg border border-[#30363d] text-white transition-colors"
+ className="flex items-center gap-2 px-4 py-2 bg-[#0d1117] hover:bg-[#161b22] rounded-lg border border-[#30363d] text-white transition-colors"📝 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.
| {isOpen && ( | |
| <motion.div | |
| initial={{opacity:0,y:-10}} | |
| animate={{opacity:1,y:0}} | |
| transition={{duration:0.25}} | |
| className="absolute top-full left-0 w-full bg-neutral-900/90 backdrop-blur-xl border-t border-white/10 md:hidden flex flex-col items-center py-5 space-y-4 z-50" | |
| > | |
| {links.map((link,index)=>( | |
| <Link key={index} href={link.href} onClick={()=>setIsOpen(false)} className="text-white hover:text-gray-300 text-lg">{link.name} | |
| </Link> | |
| ))} | |
| <Link | |
| href="https://github.com/apsinghdev/opensox" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| onClick={()=>setIsOpen(false)} | |
| className="flex items-center gap-2 px-4 py-2 bg-[0#d1117] hover:bg-[#161b22] rounded-lg border border-[#30363d] text-white transition-colors" | |
| > | |
| <Github className="w-5 h-5" /> | |
| <span className="text-sm font-medium">Contribute</span> | |
| </Link> | |
| <Link href="/dashboard/home" onClick={()=> setIsOpen(false)} className="cursor-pointer z-30"> | |
| <PrimaryButtom> | |
| <Terminal /> | |
| Get Started | |
| </PrimaryButtom> | |
| </Link> | |
| </motion.div>)} | |
| {isOpen && ( | |
| <motion.div | |
| initial={{opacity:0,y:-10}} | |
| animate={{opacity:1,y:0}} | |
| transition={{duration:0.25}} | |
| className="absolute top-full left-0 w-full bg-neutral-900/90 backdrop-blur-xl border-t border-white/10 md:hidden flex flex-col items-center py-5 space-y-4 z-50" | |
| > | |
| {links.map((link,index)=>( | |
| <Link key={index} href={link.href} onClick={()=>setIsOpen(false)} className="text-white hover:text-gray-300 text-lg">{link.name} | |
| </Link> | |
| ))} | |
| <Link | |
| href="https://github.com/apsinghdev/opensox" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| onClick={()=>setIsOpen(false)} | |
| className="flex items-center gap-2 px-4 py-2 bg-[#0d1117] hover:bg-[#161b22] rounded-lg border border-[#30363d] text-white transition-colors" | |
| > | |
| <Github className="w-5 h-5" /> | |
| <span className="text-sm font-medium">Contribute</span> | |
| </Link> | |
| <Link href="/dashboard/home" onClick={()=> setIsOpen(false)} className="cursor-pointer z-30"> | |
| <PrimaryButtom> | |
| <Terminal /> | |
| Get Started | |
| </PrimaryButtom> | |
| </Link> | |
| </motion.div>)} |
🤖 Prompt for AI Agents
In apps/web/src/components/landing-sections/navbar.tsx around lines 97 to 129,
there is a typo in the Tailwind background class on line ~115: replace the
invalid class `bg-[0#d1117]` with the correct `bg-[#0d1117]` so the intended
background color is applied; update the className string accordingly and run a
quick dev build to verify Tailwind recognizes the corrected utility.
✅ Addressed in commit b134511
|
Added mobile responsiveness to the navbar. Also fixed a small typo mistake with a new commit . nav-mobile-responsive.mp4new-nav.mp4 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/web/src/components/landing-sections/navbar.tsx (4)
6-6: Polish the import statement spacing.The import has inconsistent spacing around commas: missing space after
Github,and extra space in,Menu ,.Apply this diff:
-import { Terminal, Github,Menu ,X } from "lucide-react"; +import { Terminal, Github, Menu, X } from "lucide-react";
31-31: Add space before closing brace for consistency.The FAQ link is missing a space before the closing brace, making it inconsistent with the other link objects in the array.
Apply this diff:
- { name: "FAQ", href: "/#faq"} + { name: "FAQ", href: "/#faq" }
96-123: Consider adding Escape key support to close the mobile menu.The mobile menu currently only closes via the toggle button or clicking a link. Adding Escape key support would improve the user experience and align with common accessibility patterns.
Add this effect after the
isOpenstate declaration:React.useEffect(() => { const handleEscape = (e: KeyboardEvent) => { if (e.key === 'Escape' && isOpen) { setIsOpen(false); } }; document.addEventListener('keydown', handleEscape); return () => document.removeEventListener('keydown', handleEscape); }, [isOpen]);
97-117: Refine spacing in object literals and arrow functions.There are several formatting inconsistencies with missing spaces in object literals and arrow functions throughout the mobile menu code.
Apply this diff to improve consistency:
<motion.div - initial={{opacity:0,y:-10}} - animate={{opacity:1,y:0}} - transition={{duration:0.25}} + initial={{ opacity: 0, y: -10 }} + animate={{ opacity: 1, y: 0 }} + transition={{ duration: 0.25 }} className="absolute top-full left-0 w-full bg-neutral-900/90 backdrop-blur-xl border-t border-white/10 md:hidden flex flex-col items-center py-5 space-y-4 z-50" > - {links.map((link,index)=>( - <Link key={index} href={link.href} onClick={()=>setIsOpen(false)} className="text-white hover:text-gray-300 text-lg">{link.name} + {links.map((link, index) => ( + <Link key={index} href={link.href} onClick={() => setIsOpen(false)} className="text-white hover:text-gray-300 text-lg">{link.name} </Link> ))} <Link href="https://github.com/apsinghdev/opensox" target="_blank" rel="noopener noreferrer" - onClick={()=>setIsOpen(false)} + onClick={() => setIsOpen(false)} className="flex items-center gap-2 px-4 py-2 bg-[#0d1117] hover:bg-[#161b22] rounded-lg border border-[#30363d] text-white transition-colors" > <Github className="w-5 h-5" /> <span className="text-sm font-medium">Contribute</span> </Link> - <Link href="/dashboard/home" onClick={()=> setIsOpen(false)} className="cursor-pointer z-30"> + <Link href="/dashboard/home" onClick={() => setIsOpen(false)} className="cursor-pointer z-30"> <PrimaryButtom> <Terminal /> Get Started
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/faq/FaqSection.tsx(1 hunks)apps/web/src/components/landing-sections/navbar.tsx(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/components/faq/FaqSection.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/components/landing-sections/navbar.tsx (1)
apps/web/src/components/icons/icons.tsx (2)
Github(195-197)Terminal(1-5)
🔇 Additional comments (1)
apps/web/src/components/landing-sections/navbar.tsx (1)
11-123: Great work implementing mobile responsiveness!The mobile menu implementation is solid and functional. The collapsible navigation with smooth animations using framer-motion works well, and the responsive breakpoints are correctly applied. The FAQ link is now accessible on both desktop and mobile views, achieving the PR objectives.
The main improvement needed is adding accessibility attributes to the toggle button (flagged separately), while the formatting suggestions are optional polish items.
Nice job on your first open-source contribution! 🎉
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/src/components/landing-sections/navbar.tsx (4)
18-28: Good accessibility addition, but consider improving focus management.The Escape key handler correctly closes the mobile menu, which is good for keyboard navigation. However, blurring the active element may leave keyboard users without a clear focus indicator.
Consider moving focus back to the toggle button instead of blurring:
React.useEffect(() => { const handleEscape = (e: KeyboardEvent) => { if (e.key === 'Escape' && isOpen) { setIsOpen(false); - (document.activeElement as HTMLElement)?.blur(); } }; document.addEventListener('keydown', handleEscape); return () => document.removeEventListener('keydown', handleEscape); }, [isOpen]);This way, keyboard users know where they are in the document after closing the menu.
58-65: Consider repositioning the mobile menu toggle to the right.The accessibility attributes (
aria-labelandaria-expanded) are correctly implemented. However, the toggle button is positioned on the left side of the navbar, which is unconventional. Most mobile UIs place the menu toggle on the right for better thumb accessibility and user expectations.Consider reordering the navbar children or using flexbox order utilities:
- <button - className="md:hidden text-white" + <button + className="md:hidden text-white order-last" onClick={() => setIsOpen(!isOpen)} aria-label="Toggle navigation menu" aria-expanded={isOpen} >Also consider adding
aria-controlsto link the button to the menu:<button className="md:hidden text-white order-last" onClick={() => setIsOpen(!isOpen)} aria-label="Toggle navigation menu" aria-expanded={isOpen} + aria-controls="mobile-menu" >Then add
id="mobile-menu"to the mobile menu container.
111-138: Add exit animation for better UX.The mobile menu has an entrance animation but no exit animation—it will abruptly disappear when closed. Wrap the menu in
AnimatePresencefrom framer-motion to enable smooth exit transitions.+import { motion, useScroll, useMotionValueEvent, AnimatePresence } from "framer-motion";- {isOpen && ( - <motion.div + <AnimatePresence> + {isOpen && ( + <motion.div - initial={{ opacity: 0, y: -10 }} - animate={{ opacity: 1, y: 0}} - transition={{ duration: 0.25 }} + initial={{ opacity: 0, y: -10 }} + animate={{ opacity: 1, y: 0}} + exit={{ opacity: 0, y: -10 }} + transition={{ duration: 0.25 }} - className="absolute top-full left-0 w-full bg-neutral-900/90 backdrop-blur-xl border-t border-white/10 md:hidden flex flex-col items-center py-5 space-y-4 z-50" - > + className="absolute top-full left-0 w-full bg-neutral-900/90 backdrop-blur-xl border-t border-white/10 md:hidden flex flex-col items-center py-5 space-y-4 z-50" + > - {links.map((link, index)=>( + {links.map((link, index)=>( - <Link key={index} href={link.href} onClick={() => setIsOpen(false)} className="text-white hover:text-gray-300 text-lg">{link.name} - </Link> - ))} + <Link key={index} href={link.href} onClick={() => setIsOpen(false)} className="text-white hover:text-gray-300 text-lg">{link.name} + </Link> + ))} - <Link - href="https://github.com/apsinghdev/opensox" - target="_blank" - rel="noopener noreferrer" - onClick={() => setIsOpen(false)} - className="flex items-center gap-2 px-4 py-2 bg-[#0d1117] hover:bg-[#161b22] rounded-lg border border-[#30363d] text-white transition-colors" - > - <Github className="w-5 h-5" /> - <span className="text-sm font-medium">Contribute</span> - </Link> + <Link + href="https://github.com/apsinghdev/opensox" + target="_blank" + rel="noopener noreferrer" + onClick={() => setIsOpen(false)} + className="flex items-center gap-2 px-4 py-2 bg-[#0d1117] hover:bg-[#161b22] rounded-lg border border-[#30363d] text-white transition-colors" + > + <Github className="w-5 h-5" /> + <span className="text-sm font-medium">Contribute</span> + </Link> - <Link href="/dashboard/home" onClick={() => setIsOpen(false)} className="cursor-pointer z-30"> - <PrimaryButtom> - <Terminal /> - Get Started - </PrimaryButtom> - </Link> - </motion.div>)} + <Link href="/dashboard/home" onClick={() => setIsOpen(false)} className="cursor-pointer"> + <PrimaryButtom> + <Terminal /> + Get Started + </PrimaryButtom> + </Link> + </motion.div> + )} + </AnimatePresence>Note: The
z-30on the Get Started link is redundant since it's already inside az-50container.
111-138: Consider adding click-outside-to-close functionality.Many users expect to be able to close a mobile menu by tapping outside of it. You can implement this by adding a click handler on a backdrop overlay or by using a ref with a click-outside hook.
Example using a backdrop:
{isOpen && ( <> <div className="fixed inset-0 bg-black/20 z-40 md:hidden" onClick={() => setIsOpen(false)} aria-hidden="true" /> <motion.div initial={{ opacity: 0, y: -10 }} animate={{ opacity: 1, y: 0}} transition={{ duration: 0.25 }} className="absolute top-full left-0 w-full bg-neutral-900/90 backdrop-blur-xl border-t border-white/10 md:hidden flex flex-col items-center py-5 space-y-4 z-50" > {/* menu content */} </motion.div> </> )}This improves the mobile UX by providing an intuitive way to dismiss the menu.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/components/landing-sections/navbar.tsx(6 hunks)
🔇 Additional comments (4)
apps/web/src/components/landing-sections/navbar.tsx (4)
6-6: LGTM: Mobile menu icons imported correctly.The Menu and X icons from lucide-react are appropriate choices for the mobile navigation toggle.
16-16: LGTM: Mobile menu state initialized properly.The
isOpenstate correctly manages mobile menu visibility with an appropriate initial value.
43-43: LGTM: FAQ link added correctly.The FAQ navigation entry is properly formatted and consistent with other links in the array.
94-94: LGTM: Desktop actions correctly hidden on mobile.The responsive utility properly hides desktop action buttons on mobile, with mobile versions provided in the dropdown menu.
|
@ssscharan149 you did a good work but you've pushed changes directly to main branch. please create a new branch each time you push any changes to remote repo. ive tranferred your commits to this pr #129 |
|
Sorry sir, I dont know that I pushed it directly, but I have created a feature branch say nav-change in my local repo, |
2 participants
New users can directly navigate to the FAQ section, can see the frequently asked questions.
In other words, updated navigation bar with FAQ, making it look more complete that is, provides navigation across the whole landing page.
Changes done in 2 files.
apps/web/src/components/faq/FaqSection.tsx
apps/web/src/components/landing-sections/navbar.tsx
Recording.2025-10-22.162252.mp4
Summary by CodeRabbit
New Features
Chores