Fix navbar navigation links on non-homepage routes#107
Open
deepsikha-dash wants to merge 2 commits into
Open
Conversation
|
@deepsikha-dash is attempting to deploy a commit to the vishnukothakapu's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNavigation links and global styling are updated to use Next.js ChangesUI and Navigation Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/components/Navbar.tsx (1)
44-50: ⚡ Quick winUse
next/linkfor internal hash routes instead of raw anchors.Lines 44, 47, and 50 use raw
<a>tags for navigation, whileLinkfromnext/linkis already imported at the top of the file. Using<Link href="/#...">prevents full page reloads and keeps routing behavior consistent with Next.js conventions.♻️ Proposed refactor
- <a href="/#features" className={activeSection === "features" ? "text-foreground font-medium border-b-2 border-primary pb-0.5" : "text-muted-foreground hover:text-foreground hover:border-b-2 hover:border-primary hover:pb-0.5"}> + <Link href="/#features" className={activeSection === "features" ? "text-foreground font-medium border-b-2 border-primary pb-0.5" : "text-muted-foreground hover:text-foreground hover:border-b-2 hover:border-primary hover:pb-0.5"}> Features - </a> + </Link> - <a href="/#how" className={activeSection === "how" ? "text-foreground font-medium border-b-2 border-primary pb-0.5" : "text-muted-foreground hover:text-foreground hover:border-b-2 hover:border-primary hover:pb-0.5"}> + <Link href="/#how" className={activeSection === "how" ? "text-foreground font-medium border-b-2 border-primary pb-0.5" : "text-muted-foreground hover:text-foreground hover:border-b-2 hover:border-primary hover:pb-0.5"}> How it works - </a> + </Link> - <a href="/#demo" className={activeSection === "demo" ? "text-foreground font-medium border-b-2 border-primary pb-0.5" : "text-muted-foreground hover:text-foreground hover:border-b-2 hover:border-primary hover:pb-0.5"}> + <Link href="/#demo" className={activeSection === "demo" ? "text-foreground font-medium border-b-2 border-primary pb-0.5" : "text-muted-foreground hover:text-foreground hover:border-b-2 hover:border-primary hover:pb-0.5"}> Demo - </a> + </Link>🤖 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 `@app/components/Navbar.tsx` around lines 44 - 50, Replace the raw <a> anchors used for internal hash navigation with Next.js Link components to avoid full page reloads: where you currently render the three anchors (the ones checking activeSection === "features"/"how"/"demo"), use the imported Link component (Link href="/#features", Link href="/#how", Link href="/#demo") and move the className/activeSection logic onto the Link (or its child anchor) so styling and active-state checks remain identical; ensure the Link usage preserves the existing className strings and activeSection comparisons in the Navbar component.
🤖 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.
Nitpick comments:
In `@app/components/Navbar.tsx`:
- Around line 44-50: Replace the raw <a> anchors used for internal hash
navigation with Next.js Link components to avoid full page reloads: where you
currently render the three anchors (the ones checking activeSection ===
"features"/"how"/"demo"), use the imported Link component (Link
href="/#features", Link href="/#how", Link href="/#demo") and move the
className/activeSection logic onto the Link (or its child anchor) so styling and
active-state checks remain identical; ensure the Link usage preserves the
existing className strings and activeSection comparisons in the Navbar
component.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2960562c-f19a-49f0-aec6-5dcaffbc6b53
📒 Files selected for processing (1)
app/components/Navbar.tsx
5 tasks
vishnukothakapu
added
good first issue
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Anushreebasics
requested changes
May 16, 2026
Anushreebasics
left a comment
Anushreebasics
left a comment
Collaborator
There was a problem hiding this comment.
Add aria-current="page" to the active link for screen-reader clarity.
Collaborator
|
@deepsikha-dash squash commits to just 1 and make the CI green |
deepsikha-dash
force-pushed
the
fix-navbar-links
branch
from
dd59b3c to
d3d526c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@app/components/Navbar.tsx`:
- Line 45: In Navbar.tsx update the aria-current attribute used for in-page
section navigation: when activeSection === "features" (and likewise for the
other two nav links that check activeSection) set aria-current="location"
instead of "page" (or remove the attribute when not active); locate the
attribute usage inside the Navbar component where aria-current={activeSection
=== "..."} is set and change the value to "location" for all three in-page
anchor links.
- Around line 44-58: The hash-only hrefs in Navbar.tsx (the Link elements for
"Features", "How it works", and "Demo") cause navigation to preserve the current
path (e.g., /login#features); update each href to include the root path (change
"`#features`", "`#how`", "`#demo`" to "/#features", "/#how", "/#demo") so clicks
always navigate to the homepage before scrolling to the section; keep the
existing aria-current and className logic unchanged and apply the same fix to
any other hash-only Link usages in this component.
In `@app/page.tsx`:
- Around line 437-453: Replace the Next.js Link usage for the external URL with
a regular anchor element: change the JSX block that renders Link (the element
using href={`https://${url}`} and target="_blank") to an <a> tag, keep the same
classes/children (the container div, icon, label, URL span and ArrowUpRight),
and add rel="noopener noreferrer" to the anchor for security; ensure
props/variables referenced (url, label, icon, ArrowUpRight) are still used
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c0569811-e9c5-4ec1-846e-417394957eb2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
app/[username]/ProfileCard.tsxapp/[username]/not-found.tsxapp/[username]/page.tsxapp/components/Navbar.tsxapp/components/ThemeToggle.tsxapp/globals.cssapp/not-found.tsxapp/page.tsxpackage.json
💤 Files with no reviewable changes (1)
- package.json
✅ Files skipped from review due to trivial changes (4)
- app/[username]/page.tsx
- app/[username]/ProfileCard.tsx
- app/[username]/not-found.tsx
- app/not-found.tsx
deepsikha-dash
force-pushed
the
fix-navbar-links
branch
from
d3d526c to
2d819cb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/page.tsx (1)
466-473:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse regular
<a>tag for external links in FooterIcon.Same issue as
DemoRow— Next.jsLinkshould not be used for external URLs. For external links withtarget="_blank", use a regular anchor tag withrel="noopener noreferrer".🔗 Recommended fix
- <Link - href={href} - target="_blank" - aria-label={label} - className="flex h-10 w-10 items-center justify-center rounded-xl border border-violet-200/70 bg-white/60 text-zinc-500 transition-all duration-300 hover:-translate-y-0.5 hover:border-violet-300 hover:text-violet-700 hover:shadow-md dark:border-white/10 dark:bg-white/[0.04] dark:text-zinc-400 dark:hover:text-violet-200" - > + <a + href={href} + target="_blank" + rel="noopener noreferrer" + aria-label={label} + className="flex h-10 w-10 items-center justify-center rounded-xl border border-violet-200/70 bg-white/60 text-zinc-500 transition-all duration-300 hover:-translate-y-0.5 hover:border-violet-300 hover:text-violet-700 hover:shadow-md dark:border-white/10 dark:bg-white/[0.04] dark:text-zinc-400 dark:hover:text-violet-200" + > {children} - </Link> + </a>🤖 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 `@app/page.tsx` around lines 466 - 473, FooterIcon (the component rendering the Link with href, target="_blank" and aria-label) is using Next.js Link for external URLs; replace the Link with a regular anchor element (<a>) when rendering external links, preserve href, target="_blank", aria-label, className and children, and add rel="noopener noreferrer" to the anchor to prevent opener vulnerabilities; ensure the change is made in the FooterIcon render function where the Link JSX is used so internal navigation behavior remains unchanged for internal routes.
🤖 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 `@app/page.tsx`:
- Around line 488-496: The FooterColumn mapping currently always wraps entries
from the links array with Next.js Link (the <Link ...> in app/page.tsx), which
breaks external (https://...) and mailto: URLs; change the map so it checks each
href: if it's an internal path (e.g., startsWith('/')) render Next.js Link with
the same className and key, otherwise render a plain <a> element for external
and mailto links, adding target="_blank" and rel="noopener noreferrer" for
http/https links (but not for mailto). Ensure you update the branch that renders
Link vs anchor where links.map(([label, href]) => ...) is defined.
---
Duplicate comments:
In `@app/page.tsx`:
- Around line 466-473: FooterIcon (the component rendering the Link with href,
target="_blank" and aria-label) is using Next.js Link for external URLs; replace
the Link with a regular anchor element (<a>) when rendering external links,
preserve href, target="_blank", aria-label, className and children, and add
rel="noopener noreferrer" to the anchor to prevent opener vulnerabilities;
ensure the change is made in the FooterIcon render function where the Link JSX
is used so internal navigation behavior remains unchanged for internal routes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d73666f-ea64-4d5e-9335-dd17b5194816
📒 Files selected for processing (8)
app/[username]/ProfileCard.tsxapp/[username]/not-found.tsxapp/[username]/page.tsxapp/components/Navbar.tsxapp/components/ThemeToggle.tsxapp/globals.cssapp/not-found.tsxapp/page.tsx
✅ Files skipped from review due to trivial changes (2)
- app/not-found.tsx
- app/[username]/page.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- app/[username]/ProfileCard.tsx
- app/[username]/not-found.tsx
- app/components/ThemeToggle.tsx
- app/components/Navbar.tsx
deepsikha-dash
force-pushed
the
fix-navbar-links
branch
from
23da5b2 to
956791e
Compare
Owner
|
Hi @deepsikha-dash , please resolve all the review comments above and make the required changes in the code. After pushing the fixes, kindly mark the resolved review comments as resolved in the PR as well. |
Fix review comments
deepsikha-dash
force-pushed
the
fix-navbar-links
branch
from
8a42f90 to
62c0509
Compare
Owner
|
Hi @deepsikha-dash, Please resolve the conflicts, push the updated changes. |
Anushreebasics
requested changes
May 20, 2026
| @@ -0,0 +1,133 @@ | |||
| [33m956791e[m[33m ([m[1;36mHEAD[m[33m -> [m[1;32mfix-navbar-links[m[33m, [m[1;31morigin/fix-navbar-links[m[33m)[m Fix navbar navigation links on non-homepage routes | |||
Collaborator
There was a problem hiding this comment.
Please remove it from the PR.
Anushreebasics
requested changes
May 20, 2026
Anushreebasics
left a comment
Anushreebasics
left a comment
Collaborator
There was a problem hiding this comment.
The mobile navbar still uses hash-only links in [Navbar.tsx], so the fix does not apply on non-home routes for phone/tablet users. Please change the mobile items to absolute home anchors too, like href="/#features", href="/#how", and href="/#demo"
Collaborator
|
@deepsikha-dash please resolve conflicts |
vishnukothakapu
added
gssoc:approved
and removed
good first issue
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
3 participants
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.
Description
Fixed navbar navigation links on non-homepage routes.
Changes Made
/loginand/registerIssue
Closes #41
Summary by CodeRabbit
Style
New Features