feat(projects): add /projects page with clickable social icons, responsive grid, and share links #58
Conversation
…nsive grid, and share links while preserving visual design
WalkthroughA new client-side Next.js page ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
app/projects/page.tsx (2)
95-96: Consider using the Facebook icon from lucide-react.The comment mentions using
Share2to avoid adding another icon library, butlucide-react(already imported) includes aApply this diff to use the proper Facebook icon:
+import { Github, Twitter, Globe, Share2, Linkedin, Copy, Facebook } from "lucide-react"Then replace:
- {/* Using Share2 icon to represent Facebook share without adding another icon lib */} - <Share2 className="w-4 h-4" /> + <Facebook className="w-4 h-4" />
120-126: Add error handling for Image component.The Next.js
Imagecomponent could fail to load if the logo path is invalid or the file is missing. Consider adding theonErrorprop or using a placeholder.Apply this diff to add a fallback:
<Image src={logoSrc} alt={`${project.name} logo`} width={64} height={64} className="w-16 h-16 object-contain" + onError={(e) => { + e.currentTarget.src = "/placeholder-logo.svg" + }} />Alternatively, use the
placeholderandblurDataURLprops for a better loading experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/projects/page.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/projects/page.tsx (1)
lib/metadata.ts (1)
getAbsoluteUrl(13-19)
🔇 Additional comments (3)
app/projects/page.tsx (3)
9-19: LGTM!The
Projectinterface is well-structured with appropriate optional fields for flexibility.
21-47: Temporary data acknowledged.The hardcoded project data is appropriate for initial development. As noted in the PR objectives, this is intended to be wired to JSON/API later.
185-216: Overall structure is well-implemented.The page layout is clean with:
- Semantic HTML elements (header, main, article)
- Responsive grid that adapts to screen sizes (1→2→3 columns)
- Good visual hierarchy and consistent styling
- Accessible navigation with the "Back to Articles" link
| @@ -0,0 +1,216 @@ | |||
| "use client" | |||
There was a problem hiding this comment.
Consider Server Component for better SEO and performance.
Using "use client" makes the entire page client-side rendered, which may hurt SEO since project content won't be in the initial HTML. With Next.js 15, consider refactoring to:
- Keep the page as a Server Component for static content (header, project data)
- Extract interactive features (ShareLinks, clipboard functionality) into separate Client Components
This approach would improve SEO, reduce JavaScript bundle size, and maintain interactivity where needed.
🤖 Prompt for AI Agents
In app/projects/page.tsx around line 1, the file is marked with "use client"
making the whole page a Client Component which harms SEO and increases bundle
size; refactor by removing "use client" so the page becomes a Server Component
for static content (headers, project data fetching and rendering), and move any
interactive pieces (ShareLinks, clipboard handlers, event-driven UI) into
separate Client Components that import "use client" at their top; ensure data
fetching uses server-side APIs or getServerSideProps/async server component
patterns and pass necessary props to the isolated client components to restore
interactivity without converting the whole page to a client bundle.
| ] | ||
|
|
||
| function ShareLinks({ project }: { project: Project }) { | ||
| const shareTarget = project.links.website || project.links.github || getAbsoluteUrl("/projects") |
There was a problem hiding this comment.
Fix circular reference in fallback logic.
The fallback getAbsoluteUrl("/projects") creates a confusing UX where sharing a project without a website or GitHub link shares the projects listing page itself, rather than something meaningful about the specific project.
Consider these alternatives:
- Omit the share buttons entirely if no website/GitHub link exists
- Use a project-specific anchor:
getAbsoluteUrl(\/projects#${project.id}`)` - Show a disabled state with a tooltip explaining why sharing is unavailable
Apply this diff to hide share functionality when no valid URL exists:
function ShareLinks({ project }: { project: Project }) {
const shareTarget = project.links.website || project.links.github || getAbsoluteUrl("/projects")
+
+ // Don't show share links if no meaningful URL exists
+ if (!project.links.website && !project.links.github) {
+ return null
+ }
+
const shareText = encodeURIComponent(`${project.name}`)📝 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.
| const shareTarget = project.links.website || project.links.github || getAbsoluteUrl("/projects") | |
| function ShareLinks({ project }: { project: Project }) { | |
| const shareTarget = project.links.website || project.links.github || getAbsoluteUrl("/projects") | |
| // Don't show share links if no meaningful URL exists | |
| if (!project.links.website && !project.links.github) { | |
| return null | |
| } | |
| const shareText = encodeURIComponent(`${project.name}`) |
🤖 Prompt for AI Agents
In app/projects/page.tsx around line 50, the fallback to
getAbsoluteUrl("/projects") creates a circular/share-unhelpful URL; change the
logic to only set shareTarget when project.links.website or project.links.github
exist and otherwise treat sharing as unavailable—update component to omit or
render a disabled share UI (with tooltip) when shareTarget is undefined, ensure
shareTarget is computed as project.links.website || project.links.github ||
undefined, and adjust downstream rendering to check for undefined before showing
share buttons so no projects listing URL is used as a fallback.
| const copyLink = async () => { | ||
| try { | ||
| await navigator.clipboard.writeText(shareTarget) | ||
| // Optional: You could integrate a toast here, but we'll keep it minimal and accessible. | ||
| alert("Link copied to clipboard") | ||
| } catch (e) { | ||
| alert("Unable to copy link") | ||
| } | ||
| } |
There was a problem hiding this comment.
Add clipboard API availability check and improve UX feedback.
The code has two issues:
navigator.clipboardrequires HTTPS and may not be available in all contexts (e.g., older browsers, insecure contexts)- Using
alert()is disruptive and not recommended for modern web UX
Apply this diff to add availability check:
const copyLink = async () => {
+ if (!navigator.clipboard) {
+ alert("Clipboard access not available")
+ return
+ }
try {
await navigator.clipboard.writeText(shareTarget)For better UX, consider integrating a toast notification library (like sonner or react-hot-toast) instead of alert(). This would provide non-blocking feedback that's more consistent with modern UI patterns.
📝 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.
| const copyLink = async () => { | |
| try { | |
| await navigator.clipboard.writeText(shareTarget) | |
| // Optional: You could integrate a toast here, but we'll keep it minimal and accessible. | |
| alert("Link copied to clipboard") | |
| } catch (e) { | |
| alert("Unable to copy link") | |
| } | |
| } | |
| const copyLink = async () => { | |
| if (!navigator.clipboard) { | |
| alert("Clipboard access not available") | |
| return | |
| } | |
| try { | |
| await navigator.clipboard.writeText(shareTarget) | |
| // Optional: You could integrate a toast here, but we'll keep it minimal and accessible. | |
| alert("Link copied to clipboard") | |
| } catch (e) { | |
| alert("Unable to copy link") | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/projects/page.tsx around lines 58 to 66, the copyLink handler assumes
navigator.clipboard is always available and uses blocking alert() calls; update
it to first check for clipboard API availability (e.g., if (navigator.clipboard
&& navigator.clipboard.writeText)) and handle the unsupported case gracefully
(show a non-blocking fallback such as copying via a hidden input/select fallback
or disabling the copy action), replace alert() with a toast/notification
mechanism (integrate a toast library like sonner or react-hot-toast, or use a
small custom non-blocking message component) to show success and error messages,
and ensure errors are caught and logged for debugging while presenting an
accessible, non-disruptive message to the user.
| const logoSrc = useMemo(() => { | ||
| if (!project.logo) return "/placeholder-logo.svg" | ||
| return project.logo.startsWith("/") ? project.logo : project.logo | ||
| }, [project.logo]) |
There was a problem hiding this comment.
Simplify redundant useMemo logic.
The ternary operator doesn't perform any meaningful transformation—it returns project.logo unchanged in both branches:
return project.logo.startsWith("/") ? project.logo : project.logoApply this diff to simplify:
const logoSrc = useMemo(() => {
- if (!project.logo) return "/placeholder-logo.svg"
- return project.logo.startsWith("/") ? project.logo : project.logo
+ return project.logo || "/placeholder-logo.svg"
}, [project.logo])If you intended to prepend "/" to relative paths, the logic should be:
return project.logo.startsWith("/") ? project.logo : `/${project.logo}`🤖 Prompt for AI Agents
In app/projects/page.tsx around lines 111-114, the useMemo contains a redundant
ternary that returns project.logo unchanged; replace that logic so that if
project.logo is falsy you return the placeholder, otherwise either return
project.logo directly (to keep current behavior) or, if you intended to
normalize relative paths, return project.logo prefixed with a leading slash when
it does not start with one (i.e. check startsWith("/") and prepend "/" only for
relative paths). Ensure the dependency array remains [project.logo].
| }, [project.logo]) | ||
|
|
||
| return ( | ||
| <article className="flex flex-col h-full bg-white shadow-lg overflow-hidden transition-all duration-300 hover:shadow-2xl hover:-translate-y-2 border border-gradient-to-r from-[#228B22]/10 to-[#FFBF00]/10 relative rounded-lg"> |
There was a problem hiding this comment.
Invalid Tailwind CSS class will not render correctly.
The className contains border-gradient-to-r from-[#228B22]/10 to-[#FFBF00]/10, but border-gradient-to-r is not a valid Tailwind class. Tailwind doesn't support gradient borders directly via utility classes.
To achieve a gradient border effect, consider one of these approaches:
Option 1: Use a wrapper div with gradient background (recommended)
- <article className="flex flex-col h-full bg-white shadow-lg overflow-hidden transition-all duration-300 hover:shadow-2xl hover:-translate-y-2 border border-gradient-to-r from-[#228B22]/10 to-[#FFBF00]/10 relative rounded-lg">
+ <article className="flex flex-col h-full bg-white shadow-lg overflow-hidden transition-all duration-300 hover:shadow-2xl hover:-translate-y-2 border border-gray-200 relative rounded-lg">Option 2: Add gradient as a pseudo-element if gradient border is essential
This requires custom CSS or using a wrapper div with padding and gradient background.
📝 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.
| <article className="flex flex-col h-full bg-white shadow-lg overflow-hidden transition-all duration-300 hover:shadow-2xl hover:-translate-y-2 border border-gradient-to-r from-[#228B22]/10 to-[#FFBF00]/10 relative rounded-lg"> | |
| <article className="flex flex-col h-full bg-white shadow-lg overflow-hidden transition-all duration-300 hover:shadow-2xl hover:-translate-y-2 border border-gray-200 relative rounded-lg"> |
🤖 Prompt for AI Agents
In app/projects/page.tsx around line 117, the className uses an invalid Tailwind
class `border-gradient-to-r from-[#228B22]/10 to-[#FFBF00]/10`; remove that
invalid class and implement a proper gradient-border pattern instead — wrap this
article in a thin outer wrapper div that has the gradient background (using
`bg-gradient-to-r from-... to-...`) and padding, then move the current white
background, shadow and rounded corners to an inner element so the inner element
shows the white card while the outer wrapper provides the visible gradient
border; alternatively, if you prefer CSS, replace the invalid utility with a
custom class that creates a gradient border via a pseudo-element or
background-clip and apply that class to the wrapper.
| return ( | ||
| <div className="min-h-screen bg-gradient-to-br from-green-50 via-yellow-50 to-[#FFC517]/10"> | ||
| {/* Header */} | ||
| <header className="border-b border-gradient-to-r from-[#228B22]/20 to-[#FFBF00]/20 bg-white/90 backdrop-blur-sm sticky top-0 z-50 shadow-sm"> |
There was a problem hiding this comment.
Invalid Tailwind CSS class in header border.
Similar to line 117, the className contains border-gradient-to-r from-[#228B22]/20 to-[#FFBF00]/20, which is not valid Tailwind syntax. Gradient borders require custom CSS or alternative approaches.
Apply this diff to use a standard border:
- <header className="border-b border-gradient-to-r from-[#228B22]/20 to-[#FFBF00]/20 bg-white/90 backdrop-blur-sm sticky top-0 z-50 shadow-sm">
+ <header className="border-b border-gray-200 bg-white/90 backdrop-blur-sm sticky top-0 z-50 shadow-sm">If you need a gradient border, consider using a box-shadow or a pseudo-element with a gradient background.
📝 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.
| <header className="border-b border-gradient-to-r from-[#228B22]/20 to-[#FFBF00]/20 bg-white/90 backdrop-blur-sm sticky top-0 z-50 shadow-sm"> | |
| <header className="border-b border-gray-200 bg-white/90 backdrop-blur-sm sticky top-0 z-50 shadow-sm"> |
🤖 Prompt for AI Agents
In app/projects/page.tsx around line 189, the header's className uses invalid
Tailwind classes "border-gradient-to-r from-[#228B22]/20 to-[#FFBF00]/20";
replace those with a valid standard border utility (for example a solid border
color like border-b border-gray-200 or border-b border-[rgba(34,139,34,0.2)]
using arbitrary color syntax) and remove the gradient border utilities; if you
require a gradient border instead, implement it via a pseudo-element or wrapper
with a gradient background clipped to the border (or use box-shadow) rather than
using the invalid Tailwind gradient border classes.
1 participant
metadata.ts.page.tsxSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.