-
Notifications
You must be signed in to change notification settings - Fork 322
fix: Make project links clickable and open in new tab (Fixes #441) #502
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
base: main
Are you sure you want to change the base?
Conversation
|
@Riyanshverma is attempting to deploy a commit to the AOSSIE Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughUpdated the projects page to convert a project link container from a paragraph element to a semantic anchor element. The anchor element uses the project's href attribute, includes proper security attributes for external links, and applies updated styling with hover effects and dark mode support. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5–10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/pages/projects.jsx (1)
86-95: LGTM with suggestions for accessibility and defensive coding.The anchor implementation correctly uses
target="_blank"andrel="noopener noreferrer"for security. However, consider these improvements:
- Accessibility: Add screen reader indication that the link opens in a new tab:
<a href={project.link.href} target="_blank" rel="noopener noreferrer" + aria-label={`${project.link.label} (opens in new tab)`} className="relative z-10 mt-6 flex text-md font-semibold font-mono text-zinc-600 transition hover:text-[#00843D] dark:hover:text-yellow-400 dark:text-zinc-200 items-center" style={{ textDecoration: 'none' }} >
- Defensive coding: Add validation to prevent runtime errors if data is incomplete:
+ {project.link?.href && ( <a href={project.link.href} target="_blank" rel="noopener noreferrer" className="relative z-10 mt-6 flex text-md font-semibold font-mono text-zinc-600 transition hover:text-[#00843D] dark:hover:text-yellow-400 dark:text-zinc-200 items-center" style={{ textDecoration: 'none' }} > <LinkIcon className="h-6 w-6 flex-none scale-110" /> <span className="ml-2">{project.link.label}</span> </a> + )}
| <a | ||
| href={project.link.href} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="relative z-10 mt-6 flex text-md font-semibold font-mono text-zinc-600 transition hover:text-[#00843D] dark:hover:text-yellow-400 dark:text-zinc-200 items-center" | ||
| style={{ textDecoration: 'none' }} | ||
| > | ||
| <LinkIcon className="h-6 w-6 flex-none scale-110" /> | ||
| <span className="ml-2">{project.link.label}</span> | ||
| </p> | ||
| </a> |
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.
Critical: PR references wrong issue number.
The PR title claims to fix issue #441, but according to the PR objectives, issue #441 is about implementing a footer section with navigation links, social media handles, and contact information. This PR actually fixes non-clickable project links on the Projects page, which is unrelated to footer implementation. Please update the PR to reference the correct issue number.
🤖 Prompt for AI Agents
In src/pages/projects.jsx around lines 86 to 95 the PR incorrectly references
issue #441 in its title/body while the change actually fixes non-clickable
project links on the Projects page; update the PR metadata to reference the
correct issue for this bug (locate the issue that tracks "non-clickable project
links" or create one if missing), change the PR title and description to mention
that issue number, update any closing keywords (e.g., "Fixes #<correct-number>")
so GitHub links it properly, and amend the commit message or changelog entry if
necessary to reflect the correct issue reference.
Overview
Details
Related Issue
Closes #433
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.