Skip to content

feat: add Features and About links to navbar #607

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.

Sign up for GitHub

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

Open
weiwei-gitch wants to merge 1 commit into
base: main
Choose a base branch
from
+48 −27
Open

feat: add Features and About links to navbar #607

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 48 additions & 27 deletions src/components/Navbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { NavLink, Link } from "react-router-dom";
import { NavLink, Link, useNavigate, useLocation } from "react-router-dom";
import { useState, useContext } from "react";
import { ThemeContext } from "../context/ThemeContext";
import { Moon, Sun, Menu, X, Github } from "lucide-react";
import { Moon, Sun, Menu, X } from "lucide-react";

const Navbar: React.FC = () => {
const [isOpen, setIsOpen] = useState(false);

const themeContext = useContext(ThemeContext);
const navigate = useNavigate();
const location = useLocation();

if (!themeContext) return null;

Expand All @@ -19,8 +21,28 @@ const Navbar: React.FC = () => {
: "text-slate-700 dark:text-gray-300 hover:text-blue-500"
}`;

const featureLinkStyles =
"px-4 py-2 rounded-xl text-sm lg:text-base font-semibold transition-all duration-300 text-slate-700 dark:text-gray-300 hover:text-blue-500 cursor-pointer";

const closeMenu = () => setIsOpen(false);

// Smooth scroll to #features on homepage
const handleFeaturesClick = () => {
closeMenu();
if (location.pathname === "/") {
const section = document.getElementById("features");
if (section) {
section.scrollIntoView({ behavior: "smooth" });
}
} else {
navigate("/#features");
setTimeout(() => {
const section = document.getElementById("features");
if (section) section.scrollIntoView({ behavior: "smooth" });
}, 100);
Comment on lines +38 to +42
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Avoid timeout-based scroll sequencing after route change.

Line 39’s fixed setTimeout(100) is race-prone; slower renders can miss the target section and silently fail to scroll. Trigger scrolling from route/hash change instead of a time delay.

Suggested fix 
-import { useState, useContext } from "react";
+import { useState, useContext, useEffect } from "react";
...
   const handleFeaturesClick = () => {
     closeMenu();
     if (location.pathname === "/") {
       const section = document.getElementById("features");
       if (section) {
         section.scrollIntoView({ behavior: "smooth" });
       }
     } else {
-      navigate("/#features");
-      setTimeout(() => {
-        const section = document.getElementById("features");
-        if (section) section.scrollIntoView({ behavior: "smooth" });
-      }, 100);
+      navigate("/#features");
     }
   };
+
+  useEffect(() => {
+    if (location.pathname === "/" && location.hash === "`#features`") {
+      const section = document.getElementById("features");
+      if (section) {
+        section.scrollIntoView({ behavior: "smooth" });
+      }
+    }
+  }, [location.pathname, location.hash]);
🤖 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 38 - 42, The timeout-based scroll
after calling navigate("/#features") is race-prone; instead remove the
setTimeout and move the scroll logic to run when the route/hash actually changes
(for example, in the component that renders the "features" section or a
top-level effect that watches location.hash). Detect the hash (location.hash or
listen to "hashchange") and when it equals "`#features`" call
document.getElementById("features") and section.scrollIntoView({ behavior:
"smooth" }); (or run this on mount via useEffect in the Features-containing
component) rather than relying on setTimeout in the Navbar navigate handler.

}
};

return (
<nav className="sticky top-0 z-50 bg-white dark:bg-gray-900 border-b border-gray-200 dark:border-gray-800 transition-colors duration-300 backdrop-blur">
<div className="max-w-7xl mx-auto px-6 py-4 flex items-center justify-between">
Expand All @@ -35,16 +57,24 @@ const Navbar: React.FC = () => {
alt="CRL Icon"
className="h-8 w-8 object-contain"
/>

<span>GitHub Tracker</span>
</Link>

{/* Desktop Navigation */}
<div className="hidden md:flex items-center gap-3">
<NavLink to="/" className={navLinkStyles}>
<NavLink to="/" end className={navLinkStyles}>
Home
</NavLink>

{/* Features: smooth scroll to #features section on homepage */}
<span className={featureLinkStyles} onClick={handleFeaturesClick}>
Features
</span>
Comment on lines +70 to +72
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Replace clickable <span> with semantic interactive controls.

Line 70 and Line 143 use clickable <span> elements, which are not keyboard-operable by default and miss expected navigation semantics. This is an accessibility blocker for keyboard users.

Suggested fix 
-          <span className={featureLinkStyles} onClick={handleFeaturesClick}>
+          <button
+            type="button"
+            className={`${featureLinkStyles} focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500`}
+            onClick={handleFeaturesClick}
+          >
             Features
-          </span>
+          </button>
-            <span className={featureLinkStyles} onClick={handleFeaturesClick}>
+            <button
+              type="button"
+              className={`${featureLinkStyles} text-left focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500`}
+              onClick={handleFeaturesClick}
+            >
               Features
-            </span>
+            </button>

Also applies to: 143-145

🤖 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 70 - 72, Replace the non-semantic,
non-keyboard-operable <span> elements used for interactive navigation with
proper interactive elements (e.g., <button> or <a>) so they are
keyboard-accessible and provide correct semantics; specifically update the
element using className featureLinkStyles and onClick={handleFeaturesClick} (and
the second similar span at lines ~143-145) to use a button or anchor, preserve
existing event handlers (handleFeaturesClick), add appropriate ARIA attributes
and keyboard handlers if needed, and ensure styling still applies via
featureLinkStyles so visual appearance is unchanged.


<NavLink to="/about" className={navLinkStyles}>
About
</NavLink>

<NavLink to="/track" className={navLinkStyles}>
Tracker
</NavLink>
Expand Down Expand Up @@ -73,7 +103,6 @@ const Navbar: React.FC = () => {

{/* Mobile Controls */}
<div className="md:hidden flex items-center gap-2">

{/* Theme Toggle */}
<button
onClick={toggleTheme}
Expand Down Expand Up @@ -106,36 +135,28 @@ const Navbar: React.FC = () => {
{isOpen && (
<div className="md:hidden border-t border-gray-200 dark:border-gray-800 bg-white dark:bg-gray-900">
<div className="px-6 py-5 flex flex-col gap-3">

<NavLink
to="/"
className={navLinkStyles}
onClick={closeMenu}
>
<NavLink to="/" end className={navLinkStyles} onClick={closeMenu}>
Home
</NavLink>

<NavLink
to="/track"
className={navLinkStyles}
onClick={closeMenu}
>
{/* Features: smooth scroll to #features section on homepage */}
<span className={featureLinkStyles} onClick={handleFeaturesClick}>
Features
</span>

<NavLink to="/about" className={navLinkStyles} onClick={closeMenu}>
About
</NavLink>

<NavLink to="/track" className={navLinkStyles} onClick={closeMenu}>
Tracker
</NavLink>

<NavLink
to="/contributors"
className={navLinkStyles}
onClick={closeMenu}
>
<NavLink to="/contributors" className={navLinkStyles} onClick={closeMenu}>
Contributors
</NavLink>

<NavLink
to="/login"
className={navLinkStyles}
onClick={closeMenu}
>
<NavLink to="/login" className={navLinkStyles} onClick={closeMenu}>
Login
</NavLink>
</div>
Expand All @@ -145,4 +166,4 @@ const Navbar: React.FC = () => {
);
};

export default Navbar;
export default Navbar;
Loading