feat: add Features and About links to navbar#607
Open
weiwei-gitch wants to merge 1 commit into
Open
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThe PR adds smooth-scrolling navigation to a Features section in the Navbar. It introduces routing-aware behavior, replaces the logo with text, adds a clickable Features item to both desktop and mobile menus, and ensures all mobile links close the menu on interaction. ChangesFeatures Section Navigation
Sequence Diagram(s)No sequence diagram generated. The changes represent straightforward UI event handling (onClick → scroll or navigate) without multi-component orchestration that would benefit from visualization. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🎉 Thank you @weiwei-gitch for your contribution. Please make sure your PR follows https://github.com/GitMetricsLab/github_tracker/blob/main/CONTRIBUTING.md#-pull-request-guidelines
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@src/components/Navbar.tsx`:
- Around line 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.
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+38
to
+42
| navigate("/#features"); | ||
| setTimeout(() => { | ||
| const section = document.getElementById("features"); | ||
| if (section) section.scrollIntoView({ behavior: "smooth" }); | ||
| }, 100); |
Contributor
There was a problem hiding this comment.
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.
Comment on lines
+70
to
+72
| <span className={featureLinkStyles} onClick={handleFeaturesClick}> | ||
| Features | ||
| </span> |
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Description
Added "Features" and "About" navigation links to the navbar as requested in the issue.
Added a Features link that smoothly scrolls to the #features section on the homepage. If the user is on a different page, it navigates to the homepage first and then scrolls to the section.
Added an About link that navigates to the existing /about page.
Both links are added to the desktop and mobile menus, following the existing UI theme and responsive design.
Fixed the Home NavLink active state by adding the end prop to prevent it from always appearing active.
How Has This Been Tested?
Screenshots
Type of Change
Summary by CodeRabbit
New Features
Style