-
-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added new navbar design #31
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,117 +1,45 @@ | ||
import { useState, useEffect } from "react"; | ||
import React, { useState } from "react"; | ||
import Logo from "../../assets/Logo/logo.png"; | ||
import { Link, useLocation } from "react-router-dom"; | ||
|
||
const Navbar = () => { | ||
const [isScrolled, setIsScrolled] = useState(false); | ||
const [isMenuOpen, setIsMenuOpen] = useState(false); | ||
const location = useLocation(); | ||
|
||
useEffect(() => { | ||
const handleScroll = () => { | ||
const scrollPosition = window.scrollY; | ||
if (scrollPosition > 50) { | ||
setIsScrolled(true); | ||
} else { | ||
setIsScrolled(false); | ||
} | ||
}; | ||
|
||
window.addEventListener("scroll", handleScroll); | ||
|
||
return () => { | ||
window.removeEventListener("scroll", handleScroll); | ||
}; | ||
}, []); | ||
|
||
const toggleMenu = () => { | ||
setIsMenuOpen(!isMenuOpen); | ||
}; | ||
const isHomePage = location.pathname === "/"; | ||
const buttonTextClass = isScrolled | ||
? "text-gray-900" | ||
: isHomePage | ||
? "text-white" | ||
: "text-black"; | ||
|
||
const currentPath = location.pathname; | ||
|
||
const isActiveLink = (path) => currentPath === path; | ||
|
||
return ( | ||
<nav | ||
className={`w-full fixed top-0 z-50 transition duration-300 ${ | ||
isScrolled ? "bg-[#E0F0B1]" : "bg-transparent" | ||
} | ||
${isScrolled ? "text-gray-800" : "text-black"} | ||
${isScrolled ? "shadow-lg" : ""}`} | ||
> | ||
<nav className="bg-blur fixed w-full top-0 z-50"> {/* Applied glassmorphism */} | ||
<div className="max-w-7xl mx-auto px-4"> | ||
<div className="flex justify-between items-center h-16 "> | ||
<div className="flex justify-between items-center h-16"> | ||
<Link to="/"> | ||
<div className="flex-shrink-0"> | ||
<img | ||
className="w-14 h-14 bg-white rounded-full p-1 " | ||
alt="logo" | ||
src={Logo} | ||
/> | ||
<img className="w-14 h-14 bg-white rounded-full p-1" alt="logo" src={Logo} /> | ||
</div> | ||
</Link> | ||
<div className="hidden md:flex"> | ||
<ul className="ml-4 flex space-x-4 font-semibold"> | ||
<li> | ||
<a | ||
href="/" | ||
className={`hover:${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
}`} | ||
> | ||
Home | ||
</a> | ||
</li> | ||
<li> | ||
<a | ||
href="/event" | ||
className={`hover:${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
}`} | ||
> | ||
Events | ||
</a> | ||
</li> | ||
<li> | ||
<a | ||
href="/menu" | ||
className={`hover:${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
}`} | ||
> | ||
Menu | ||
</a> | ||
</li> | ||
<li> | ||
<a | ||
href="/register" | ||
className={`hover:${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
}`} | ||
> | ||
Reservation | ||
</a> | ||
</li> | ||
<li> | ||
<a | ||
href="/boardgame" | ||
className={`hover:${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
}`} | ||
> | ||
Boardgames | ||
</a> | ||
</li> | ||
{["/", "/event", "/menu", "/register", "/boardgame"].map((path, index) => ( | ||
<li key={index}> | ||
<Link | ||
to={path} | ||
className={`nav-link ${isActiveLink(path) ? "active-link" : ""}`} | ||
> | ||
{/* Add icons here if desired */} | ||
{path === "/" ? "Home" : path.replace("/", "").charAt(0).toUpperCase() + path.slice(2)} | ||
</Link> | ||
</li> | ||
))} | ||
Comment on lines
+28
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid Code Duplication by Extracting Navigation Links The navigation link mapping is duplicated in both the desktop and mobile menus. This can lead to maintenance challenges and potential inconsistencies. Consider extracting the navigation paths and link rendering into a separate component or function. Here's how you might do it:
This approach reduces duplication and makes your code more maintainable. |
||
</ul> | ||
</div> | ||
<div className="flex md:hidden"> | ||
<button | ||
onClick={toggleMenu} | ||
className={`${buttonTextClass} focus:outline-none`} | ||
> | ||
<button onClick={toggleMenu} className="text-black focus:outline-none"> | ||
<svg | ||
className="h-6 w-6" | ||
fill="none" | ||
|
@@ -132,52 +60,17 @@ const Navbar = () => { | |
</div> | ||
{/* Mobile Menu */} | ||
{isMenuOpen && ( | ||
<div | ||
className={`md:hidden ${ | ||
isScrolled ? "bg-amber-100 shadow-lg" : "bg-[#E0F0B1] shadow-lg" | ||
}`} | ||
> | ||
<div className="md:hidden mobile-menu"> | ||
<div className="px-4 pt-4 pb-4 space-y-2"> | ||
<a | ||
href="/" | ||
className={`block px-4 py-3 rounded-md text-base font-semibold transition duration-300 ${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
} hover:bg-amber-300 hover:text-black`} | ||
> | ||
Home | ||
</a> | ||
<a | ||
href="/event" | ||
className={`block px-4 py-3 rounded-md text-base font-semibold transition duration-300 ${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
} hover:bg-amber-300 hover:text-black`} | ||
> | ||
Events | ||
</a> | ||
<a | ||
href="/menu" | ||
className={`block px-4 py-3 rounded-md text-base font-semibold transition duration-300 ${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
} hover:bg-amber-300 hover:text-black`} | ||
> | ||
Menu | ||
</a> | ||
<a | ||
href="/register" | ||
className={`block px-4 py-3 rounded-md text-base font-semibold transition duration-300 ${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
} hover:bg-amber-300 hover:text-black`} | ||
> | ||
Reservation | ||
</a> | ||
<a | ||
href="/boardgame" | ||
className={`block px-4 py-3 rounded-md text-base font-semibold transition duration-300 ${ | ||
isScrolled ? "text-gray-900" : "text-gray-800" | ||
} hover:bg-amber-300 hover:text-black`} | ||
> | ||
Boardgames | ||
</a> | ||
{["/", "/event", "/menu", "/register", "/boardgame"].map((path, index) => ( | ||
<Link | ||
key={index} | ||
to={path} | ||
className={`block mobile-link ${isActiveLink(path) ? "active-link" : ""}`} | ||
> | ||
{path === "/" ? "Home" : path.replace("/", "").charAt(0).toUpperCase() + path.slice(2)} | ||
</Link> | ||
))} | ||
</div> | ||
</div> | ||
)} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug in Generating Navigation Labels
The current method for generating navigation labels may produce incorrect results for paths longer than one character. The expression:
might omit necessary characters in the labels. For example, for the path
/boardgame
, the label would be"B" + "ardgame"
, resulting in"Bardgame"
instead of"Boardgame"
.To fix this, adjust the expression to correctly capitalize the first character and include the rest of the string:
Alternatively, you can simplify the label generation: