Fix: resolve accessibility issue (#379) #513
Conversation
|
@rishu772 is attempting to deploy a commit to the AOSSIE Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe project link rendering in the Cards component was refactored from a paragraph-based structure to a dedicated anchor element with Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| <CardActions sx={{ justifyContent: 'center' }}> | ||
| <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 hover:text-[#00843D] dark:hover:text-yellow-400" | ||
| > | ||
| <LinkIcon className="h-6 w-6 flex-none scale-110" /> | ||
| <span className="ml-2">{project.link.label}</span> | ||
| </a> | ||
| </CardActions> | ||
|
|
||
|
|
There was a problem hiding this comment.
Add accessible link context and new-tab indication.
The anchor element implementation has a few accessibility and defensive coding concerns:
-
Link context: If
project.link.labelis generic (e.g., "Learn More" or "Visit Project"), screen reader users navigating by links won't have sufficient context about which project the link refers to without the surrounding card content. -
New tab indication: Links that open in new tabs should indicate this behavior to users. This is a WCAG best practice (3.2.5 Level AAA) to avoid disorienting users, especially those using assistive technologies.
-
Missing defensive check: Direct access to
project.link.hrefwithout checking ifproject.linkexists could cause a runtime error.
🔎 Proposed improvements
<CardActions sx={{ justifyContent: 'center' }}>
- <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 hover:text-[#00843D] dark:hover:text-yellow-400"
- >
- <LinkIcon className="h-6 w-6 flex-none scale-110" />
- <span className="ml-2">{project.link.label}</span>
- </a>
+ {project.link && (
+ <a
+ href={project.link.href}
+ target="_blank"
+ rel="noopener noreferrer"
+ aria-label={`${project.link.label} for ${project.name} (opens in new tab)`}
+ className="relative z-10 mt-6 flex text-md font-semibold font-mono text-zinc-600 hover:text-[#00843D] dark:hover:text-yellow-400"
+ >
+ <LinkIcon className="h-6 w-6 flex-none scale-110" />
+ <span className="ml-2">{project.link.label}</span>
+ <span className="sr-only"> (opens in new tab)</span>
+ </a>
+ )}
</CardActions>Note: Add the following to your Tailwind config or global CSS for the screen-reader-only class:
.sr-only {
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
clip: rect(0, 0, 0, 0);
white-space: nowrap;
border-width: 0;
}📝 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.
| <CardActions sx={{ justifyContent: 'center' }}> | |
| <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 hover:text-[#00843D] dark:hover:text-yellow-400" | |
| > | |
| <LinkIcon className="h-6 w-6 flex-none scale-110" /> | |
| <span className="ml-2">{project.link.label}</span> | |
| </a> | |
| </CardActions> | |
| <CardActions sx={{ justifyContent: 'center' }}> | |
| {project.link && ( | |
| <a | |
| href={project.link.href} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| aria-label={`${project.link.label} for ${project.name} (opens in new tab)`} | |
| className="relative z-10 mt-6 flex text-md font-semibold font-mono text-zinc-600 hover:text-[#00843D] dark:hover:text-yellow-400" | |
| > | |
| <LinkIcon className="h-6 w-6 flex-none scale-110" /> | |
| <span className="ml-2">{project.link.label}</span> | |
| <span className="sr-only"> (opens in new tab)</span> | |
| </a> | |
| )} | |
| </CardActions> |
🤖 Prompt for AI Agents
In src/pages/projects.jsx around lines 85 to 97, the anchor uses
project.link.href and project.link.label directly which can throw if
project.link is missing, provides poor context to screen readers, and doesn't
indicate it opens a new tab; fix by defensively checking that project.link and
project.link.href exist before rendering the anchor (otherwise omit or render a
disabled/fallback UI), make the visible label more descriptive or append a
visually-hidden span with the project title (e.g., “Visit {project.title}”) so
link text is unique in context, and add a visually-hidden note like “(opens in
new tab)” or include the same text in aria-label to announce the new-tab
behavior while keeping target="_blank" and rel attributes as-is.
2 participants
This pull request resolves Issue #379 by fixing the incorrect card link behavior in the Projects section.
What was fixed
Corrected link handling so project links behave as expected
Ensured links are properly clickable and structured
-Cleaned up related UI logic without affecting existing design
Verification
Tested locally using
npm run devConfirmed no breaking changes to other components
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.