Conversation
|
Thank you for submitting this pull request! We appreciate your contribution to the project. Before we can merge it, we need to review the changes you've made to ensure they align with our code standards and meet the requirements of the project. We'll get back to you as soon as we can with feedback. Thanks again! |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideRefines the AI-READI project page content and layout, updates external links, introduces a timeline and partners section earlier in the page, and modernizes the ProjectAbout feature grid styling and responsiveness. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thanks for making updates to your pull request. Our team will take a look and provide feedback as soon as possible. Please wait for any GitHub Actions to complete before editing your pull request. If you have any additional questions or concerns, feel free to let us know. Thank you for your contributions! |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The trailing
{}at the end ofProjectAboutrenders an extra text node with a space; it looks accidental and should be removed. - In
ProjectAbout, thegridColslogic usesfeatures.length > 3 ? md:grid-cols-2 : md:grid-cols-3; consider confirming this is intentional since more items resulting in fewer columns is a bit counterintuitive and may not match the intended layout.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The trailing `{` ` ` `}` at the end of `ProjectAbout` renders an extra text node with a space; it looks accidental and should be removed.
- In `ProjectAbout`, the `gridCols` logic uses `features.length > 3 ? md:grid-cols-2 : md:grid-cols-3`; consider confirming this is intentional since more items resulting in fewer columns is a bit counterintuitive and may not match the intended layout.
## Individual Comments
### Comment 1
<location> `src/pages/aireadi.tsx:21` </location>
<code_context>
target: `_blank`,
- ariaLabel: `AI-READI Documentation`,
+ ariaLabel: `AI-READI website`,
+ rel: `noopener`,
+ },
+ {
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider using `noopener noreferrer` for external links opened in a new tab.
For `target="_blank"` links, it’s best to use `rel="noopener noreferrer"` to prevent both `window.opener` access and referrer leakage. Since there are several such buttons, consider applying this consistently wherever referrer data isn’t required.
Suggested implementation:
```typescript
{
text: `Learn more`,
href: `https://aireadi.org/`, // update later if AI-READI gets its own docs URL
target: `_blank`,
ariaLabel: `AI-READI website`,
rel: `noopener noreferrer`,
```
```typescript
text: `Learn more`,
href: `https://aireadi.org/`, // update later if AI-READI gets its own docs URL
target: `_blank`,
ariaLabel: `AI-READI website`,
rel: `noopener noreferrer`,
```
Search the rest of `src/pages/aireadi.tsx` (and the broader codebase, if desired) for any link or button configs with `target: '_blank'` (or `target: "_blank"`). For each, add or update the `rel` property to `rel: 'noopener noreferrer'` (or backticked/quoted consistently with local code style) unless you explicitly need to send referrer data for that link.
</issue_to_address>
### Comment 2
<location> `src/components/project/about.tsx:73` </location>
<code_context>
+ height={18}
+ />
+ </span>
+ <span className="absolute -bottom-1 left-1/2 h-[2px] w-0 bg-pink-600 transition-all duration-300 group-hover:left-0 group-hover:w-full" />
+ </a>
+ </div>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Hover underline currently responds to card hover instead of link hover.
Since `group` is on the card container, the `group-hover:*` classes fire when the whole card is hovered, not just this link. If you only want the underline to animate on link hover, move `group` onto the `<a>` or create a separate grouped wrapper for the link and scope the hover to that element instead.
Suggested implementation:
```typescript
className="group/link text-primary relative inline-flex items-center gap-1 text-sm font-semibold transition-colors duration-300 hover:text-pink-700"
```
```typescript
<span className="absolute -bottom-1 left-1/2 h-[2px] w-0 bg-pink-600 transition-all duration-300 group-hover/link:left-0 group-hover/link:w-full" />
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… into project-style # Conflicts: # src/pages/aireadi.tsx
📝 Changed routes:
Commit f257504 (https://fairdataihub-website-gyhg4kgzx-fairdataihub.vercel.app). |
|
@Aydawka could you fix the sourcery comments. We can merge this in for aireadi i think |

Summary by Sourcery
Update the AI-READI project page content and layout and refine the shared ProjectAbout feature styling.
New Features:
Enhancements:
Documentation: