Skip to content

Refactor GitHub link handling in Header component for improved structure and mobile responsiveness #293

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
wants to merge 3 commits into
base: main
Choose a base branch
from
+98 −54
Open

Refactor GitHub link handling in Header component for improved structure and mobile responsiveness #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a54b5c8
Refactor GitHub link handling in Header component for improved struct…
sarans-h Oct 7, 2025
3a1c4de
Merge branch 'devvsakib:main' into issue-268
sarans-h Oct 8, 2025
ffea295
Enhance Header component with mobile menu functionality and dynamic s…
sarans-h Oct 8, 2025
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
150 changes: 97 additions & 53 deletions src/components/Header/Header.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ function Header({notice }) {
.catch((error) => console.error("Error fetching GitHub stars:", error));
}, []);

// Handle window resize to close mobile menu when switching to desktop
useEffect(() => {
const handleResize = () => {
if (window.innerWidth >= 768 && open) { // 768px is md breakpoint
setOpen(false);
}
};

window.addEventListener('resize', handleResize);
return () => window.removeEventListener('resize', handleResize);
}, [open]);

const navLink = [
{
name: "Home",
Expand All @@ -46,17 +58,39 @@ function Header({notice }) {
link: "/resources",
icon: <MdStore size="1.2rem" />,
},
{
name: "Github",
link: "https://github.com/devvsakib/github-error-solve",
icon: <AiFillGithub size="1.2rem" />,
isExternalURL: true,
},
];

const githubLink = {
name: "Github",
link: "https://github.com/devvsakib/github-error-solve",
icon: <AiFillGithub size="1.2rem" />,
isExternalURL: true,
};

// Dynamic styles for cleaner code
const mobileMenuClasses = `
md:flex md:items-center md:gap-6 md:static md:opacity-100 md:visible md:bg-transparent md:flex-row md:justify-end md:h-auto md:w-auto
${open ?
'fixed inset-0 w-full h-screen flex flex-col items-center justify-center gap-8 z-40 backdrop-blur-md opacity-100 visible' :
'fixed inset-0 w-full h-screen flex flex-col items-center justify-center gap-8 z-40 opacity-0 invisible pointer-events-none'
}
transition-opacity duration-500 ease-in-out
md:pointer-events-auto md:relative md:inset-auto
`;

const mobileMenuStyle = {
backgroundColor: open ? (theme === "dark" ? "rgba(2, 0, 14, 0.95)" : "rgba(209, 203, 244, 0.95)") : "transparent",
backgroundImage: open ? (theme === "dark"
? "radial-gradient(rgba(133, 138, 227, 0.1) 2px, transparent 0)"
: "radial-gradient(rgba(25, 25, 31, 0.1) 2px, transparent 0)"
) : "none",
backgroundSize: "30px 30px",
backgroundPosition: "-5px -5px"
};

return (
<header className="p-4 shadow-lg backdrop-blur-sm z-50">
<div className="w-full md:w-5/6 mx-auto flex flex-row md:flex-row justify-between items-center">
<header className="fixed top-0 left-0 right-0 w-full h-16 p-4 shadow-lg backdrop-blur-md dark:bg-gray-900/20 border-b border-white/20 dark:border-gray-800/30 z-50">
<div className="w-full md:w-5/6 mx-auto flex justify-between items-center h-full relative z-50">
{/* Logo */}
<Link to={"/"}>
<img
Expand All @@ -66,56 +100,66 @@ function Header({notice }) {
/>
</Link>

{/* Menu icon */}
<div
onClick={() => setOpen((val) => !val)}
className="cursor-pointer md:hidden"
>
{open ? <MdClose size="1.2rem" /> : <MdMenu size="1.2rem" />}
{/* GitHub, theme toggle and menu icon for mobile */}
<div className="flex items-center gap-3 md:hidden relative z-50">
<a target="_blank" href={githubLink.link}>
<div className="bg-blue-600/50 shadow font-semibold flex gap-1 p-1 px-2 items-center rounded-full text-sm">
<span className="githubBtn">
{githubLink.icon}
</span>
{
countStar && (
<div className="flex items-center gap-1">
{countStar}
</div>
)
}
</div>
</a>
Comment on lines +105 to +118
Copy link

@coderabbitai coderabbitai bot Oct 7, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add rel="noopener noreferrer" to external link.

Opening links with target="_blank" without rel="noopener noreferrer" exposes users to potential security risks (tabnabbing attacks) and performance issues. The opened page can access window.opener and potentially redirect the original page to a malicious site.

Apply this diff to fix the security issue:

-<a target="_blank" href={githubLink.link}>
+<a target="_blank" rel="noopener noreferrer" href={githubLink.link}>
📝 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.

Suggested change
<a target="_blank" href={githubLink.link}>
<div className="bg-blue-600/50 shadow font-semibold flex gap-1 p-1 px-2 items-center rounded-full text-sm">
<span className="githubBtn">
{githubLink.icon}
</span>
{
countStar && (
<div className="flex items-center gap-1">
{countStar}
</div>
)
}
</div>
</a>
<a target="_blank" rel="noopener noreferrer" href={githubLink.link}>
<div className="bg-blue-600/50 shadow font-semibold flex gap-1 p-1 px-2 items-center rounded-full text-sm">
<span className="githubBtn">
{githubLink.icon}
</span>
{
countStar && (
<div className="flex items-center gap-1">
{countStar}
</div>
)
}
</div>
</a>
🤖 Prompt for AI Agents 
In src/components/Header/Header.jsx around lines 72 to 85, the external anchor
uses target="_blank" without rel="noopener noreferrer"; update the <a> element
to include rel="noopener noreferrer" (i.e., set rel to "noopener noreferrer") so
the opened page cannot access window.opener and to mitigate tabnabbing and
performance/security issues.

⚠️ Potential issue | 🟡 Minor

Add accessible label to GitHub link.

The GitHub link lacks an aria-label, which impacts screen reader users who may not understand the purpose of the link from the icon alone.

Apply this diff to improve accessibility:

-<a target="_blank" href={githubLink.link}>
+<a target="_blank" rel="noopener noreferrer" href={githubLink.link} aria-label="Star us on GitHub">
📝 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.

Suggested change
<a target="_blank" href={githubLink.link}>
<div className="bg-blue-600/50 shadow font-semibold flex gap-1 p-1 px-2 items-center rounded-full text-sm">
<span className="githubBtn">
{githubLink.icon}
</span>
{
countStar && (
<div className="flex items-center gap-1">
{countStar}
</div>
)
}
</div>
</a>
<a target="_blank" rel="noopener noreferrer" href={githubLink.link} aria-label="Star us on GitHub">
<div className="bg-blue-600/50 shadow font-semibold flex gap-1 p-1 px-2 items-center rounded-full text-sm">
<span className="githubBtn">
{githubLink.icon}
</span>
{
countStar && (
<div className="flex items-center gap-1">
{countStar}
</div>
)
}
</div>
</a>
🤖 Prompt for AI Agents 
In src/components/Header/Header.jsx around lines 72 to 85, the anchor for the
GitHub link lacks an aria-label which makes it inaccessible to screen readers;
add an aria-label attribute to the <a> element with a meaningful descriptive
string (e.g., use githubLink.label if available or fallback to "View on GitHub")
so screen readers can convey the link purpose, and while updating the anchor
also include rel="noopener noreferrer" when using target="_blank" for security.

<div className="text-lg cursor-pointer" onClick={toggleTheme}>
<HiMoon className="dark:hidden" />
<HiSun className="hidden dark:inline" />
</div>
Comment on lines +119 to +122
Copy link

@coderabbitai coderabbitai bot Oct 7, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add accessible label to theme toggle button.

The theme toggle lacks semantic button markup and an aria-label, making it difficult for screen reader users to understand its purpose.

Apply this diff to improve accessibility:

-<div className="text-lg cursor-pointer" onClick={toggleTheme}>
+<button className="text-lg cursor-pointer bg-transparent border-0" onClick={toggleTheme} aria-label="Toggle theme">
   <HiMoon className="dark:hidden" />
   <HiSun className="hidden dark:inline" />
-</div>
+</button>
📝 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.

Suggested change
<div className="text-lg cursor-pointer" onClick={toggleTheme}>
<HiMoon className="dark:hidden" />
<HiSun className="hidden dark:inline" />
</div>
<button className="text-lg cursor-pointer bg-transparent border-0" onClick={toggleTheme} aria-label="Toggle theme">
<HiMoon className="dark:hidden" />
<HiSun className="hidden dark:inline" />
</button>
🤖 Prompt for AI Agents 
In src/components/Header/Header.jsx around lines 86 to 89, the theme toggle is a
non-semantic div without an aria-label; replace it with a <button type="button">
preserving the className and onClick (toggleTheme), add an appropriate
aria-label (preferably dynamic: "Switch to dark mode" or "Switch to light mode"
based on current theme state) and consider adding aria-pressed or visually
hidden text if you want screen readers to know the current state; keep the
HiMoon and HiSun icons as children so keyboard and screen-reader users can
activate the control.

<div
onClick={() => setOpen((val) => !val)}
className="cursor-pointer relative z-50 p-2"
>
{open ? <MdClose size="1.2rem" /> : <MdMenu size="1.2rem" />}
</div>
</div>

{/* Nav link items */}
<div
className={`
md:flex md:items-center
md:pb-0 md:gap-6
md:static md:z-auto
md:w-auto md:pl-0
md:bg-transparent
grid gap-2 absolute
z-[-1] left-0 w-full py-2 pl-8
transition-all duration-500 ease-in
${open ? (theme === "dark" ? "bg-dark/90" : "bg-white/90") : ""}
${open ? "top-14" : "top-[-490px]"}`}
>
{navLink.map((link, index) => {
return (
<div key={`${link.name}-${index}`}>
{link?.isExternalURL ? (
<a target="_blank" href={link.link}>
<div className="bg-blue-600/50 shadow font-semibold flex gap-1 p-1 px-2 items-center rounded-full">
<span className="githubBtn">
{link.icon}
</span>
{
countStar && (
<div className="flex items-center gap-1">
{countStar}
</div>
)
}
</div>
</a>
) : (
<Link className="flex items-center gap-1" to={link.link}>
{/* {link.icon} */}
{link.name}
</Link>
<div className={mobileMenuClasses} style={mobileMenuStyle}>
{navLink.map((link, index) => (
<div key={`${link.name}-${index}`}>
<Link
className="flex items-center gap-1 text-3xl md:text-base font-medium
text-[#1E1E1F] dark:text-white hover:text-[#858AE3]
transition-all duration-300 md:transform-none transform hover:scale-110
relative group"
to={link.link}
onClick={() => setOpen(false)}
>
<span className="relative z-10">{link.name}</span>
<div className="absolute inset-0 rounded-lg bg-gradient-to-r from-[#9912d8] to-[#858AE3] opacity-0 group-hover:opacity-10 transition-opacity duration-300 -z-10 md:hidden"></div>
</Link>
</div>
))}

{/* GitHub link for desktop */}
<div className="hidden md:block">
<a target="_blank" href={githubLink.link}>
<div className="bg-blue-600/50 shadow font-semibold flex gap-1 p-1 px-2 items-center rounded-full">
<span className="githubBtn">{githubLink.icon}</span>
{countStar && (
<div className="flex items-center gap-1">{countStar}</div>
)}
</div>
);
})}
<div className="text-lg cursor-pointer" onClick={toggleTheme}>
</a>
</div>

{/* Theme toggle for desktop */}
<div className="text-lg cursor-pointer hidden md:block" onClick={toggleTheme}>
<HiMoon className="dark:hidden" />
<HiSun className="hidden dark:inline" />
</div>
Comment on lines +162 to 165
Copy link

@coderabbitai coderabbitai bot Oct 7, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add accessible label to theme toggle button.

Same accessibility issue as the mobile theme toggle (Lines 86-89). The theme toggle should use semantic button markup with an aria-label.

Apply this diff to improve accessibility:

-<div className="text-lg cursor-pointer hidden md:block" onClick={toggleTheme}>
+<button className="text-lg cursor-pointer bg-transparent border-0 hidden md:block" onClick={toggleTheme} aria-label="Toggle theme">
   <HiMoon className="dark:hidden" />
   <HiSun className="hidden dark:inline" />
-</div>
+</button>
📝 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.

Suggested change
<div className="text-lg cursor-pointer hidden md:block" onClick={toggleTheme}>
<HiMoon className="dark:hidden" />
<HiSun className="hidden dark:inline" />
</div>
<button
className="text-lg cursor-pointer bg-transparent border-0 hidden md:block"
onClick={toggleTheme}
aria-label="Toggle theme"
>
<HiMoon className="dark:hidden" />
<HiSun className="hidden dark:inline" />
</button>
🤖 Prompt for AI Agents 
In src/components/Header/Header.jsx around lines 138 to 141, the theme toggle is
a non-semantic div without an accessible name; change it to a <button> element
(preserve the existing classes and onClick handler), add type="button" and an
appropriate aria-label (e.g., "Toggle theme" or "Switch to dark/light mode"),
and mark the decorative icon elements as aria-hidden="true" so screen readers
don't announce them; ensure keyboard focus styles remain and functionality is
unchanged.

Expand Down
2 changes: 1 addition & 1 deletion src/components/Layout/Layout.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const Layout = ({ stars, children }) => {
<Header
notice={"Under Construction"}
/>
<div className='relative'>
<div className='relative pt-16'>
<div className="container mx-auto p-4 min-h-screen">
{children}
</div>
Expand Down