Uh oh!
There was an error while loading. Please reload this page.
Add user data to open graph images - #1144
Conversation
* Add Lato fonts for image * Remove unused images * Add background SVGs as components * Update user profile and articles to account for updates
@NiallJoeMaher is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request introduces several modifications across multiple files to enhance metadata structures and improve accessibility. Key changes include updates to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File ( |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
app/(app)/[username]/page.tsx (2)
33-43: Enhanced Open Graph metadata structure.The changes to the
openGraphmetadata are well-implemented:
- The
typeis now correctly set to "profile".- The
imagesarray now contains a detailed object, which is more informative for the Open Graph protocol.- The dynamic alt text improves accessibility.
Consider adding a
usernameproperty to the Open Graph metadata, as it's specific to user profiles:openGraph: { title, description, type: "profile", + username: username, images: [ // ... ], siteName: "Codú", },
47-50: Improved Twitter card metadata, but consider consistency.The addition of the
cardproperty enhances the Twitter metadata. However, theimagesarray format is inconsistent with theopenGraphimages.For consistency, consider updating the
twitter.imagesto match theopenGraph.imagesformat:twitter: { card: "summary_large_image", title, description, - images: ["/images/og/home-og.png"],+ images: [+ {+ url: "/images/og/home-og.png",+ width: 1200,+ height: 630,+ alt: `${name || username}'s profile on Codú`,+ },+ ], },This change would provide more detailed image information for Twitter cards as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (8)
assets/Lato-Bold.ttfis excluded by!**/*.ttf,!**/*.ttfassets/Lato-Regular.ttfis excluded by!**/*.ttf,!**/*.ttfpublic/images/og/noise.pngis excluded by!**/*.png,!**/*.pngpublic/images/og/noise.svgis excluded by!**/*.svg,!**/*.svgpublic/images/og/planet.pngis excluded by!**/*.png,!**/*.pngpublic/images/og/planet.svgis excluded by!**/*.svg,!**/*.svgpublic/images/og/stars.svgis excluded by!**/*.svg,!**/*.svgpublic/images/og/waves.svgis excluded by!**/*.svg,!**/*.svg
📒 Files selected for processing (5)
- app/(app)/[username]/page.tsx (2 hunks)
- app/(app)/articles/[slug]/page.tsx (1 hunks)
- app/og/route.tsx (2 hunks)
- cdk/lambdas/uploadResize/index.js (1 hunks)
- components/background/background.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- cdk/lambdas/uploadResize/index.js
🧰 Additional context used
🪛 Biome
app/og/route.tsx
[error] 198-198: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (5)
app/(app)/[username]/page.tsx (3)
26-27: Improved metadata construction for better SEO and sharing.The changes to
titleanddescriptionenhance the metadata by providing more context and using fallbacks. This is beneficial for SEO and when sharing the profile on various platforms.
120-123: Improved accessibility with semantic heading.The addition of the
h1element with thesr-onlyclass significantly enhances the page's accessibility. This provides a clear, high-level description of the page content for screen reader users without affecting the visual layout.Line range hint
1-123: Overall assessment: Excellent improvements to metadata and accessibility.The changes in this file successfully address the PR objectives by enhancing the Open Graph metadata, improving accessibility, and refining the overall structure. The modifications to the
generateMetadatafunction and thePagecomponent's return statement are particularly noteworthy for their positive impact on SEO, social media sharing, and screen reader compatibility.A few minor suggestions were made for further improvement:
- Adding a
usernameproperty to the Open Graph metadata.- Ensuring consistency between the
openGraphandThese changes significantly enhance the user profile representation and align well with best practices for web development.
components/background/background.tsx (2)
1-64: Overall assessment: Well-structured components with room for improvementThe
StarsandWavescomponents are well-implemented, type-safe React components that provide reusable SVG graphics. They align with the PR objective of enhancing the visual identity and improving the representation of content.Positive aspects:
- Use of TypeScript for type safety.
- Flexible prop handling with spread operator.
- Reusable and easily integrable components.
Areas for improvement:
- Enhance responsiveness by allowing dynamic sizing.
- Consider simplifying the complex path in the
Wavescomponent for better performance and maintainability.These components contribute to the visual enhancement mentioned in the PR objectives. However, to fully meet the goal of incorporating article information into Open Graph images, ensure that these components are properly integrated with the article data rendering logic elsewhere in the codebase.
To confirm the integration of these components with article data, you could run:
✅ Verification successful
Integration Confirmed: Stars and Waves components are utilized in Open Graph functionality
The
StarsandWavescomponents are successfully integrated within theapp/og/route.tsxfile, ensuring they contribute to the generation of Open Graph images as intended. This alignment supports the PR objective of enhancing visual identity and improving content representation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash# Search for usage of Stars and Waves components in relation to article or Open Graph related files rg -i "Stars|Waves" --type tsx --type ts | rg -i "article|opengraph|og"Length of output: 101
Script:
#!/bin/bash# Search for usage of Stars and Waves components in relation to article or Open Graph related files rg -i "Stars|Waves" -g '*.tsx' -g '*.ts'| rg -i "article|opengraph|og"Length of output: 225
46-64: 🛠️ Refactor suggestionEnhance responsiveness and consider simplifying the Waves component
The
Wavescomponent is well-structured but has some areas for improvement:
Fixed dimensions may limit responsiveness.
The single path element is quite complex, which might affect performance and maintainability.
Improve responsiveness:
Similar to theStarscomponent, consider accepting width and height as optional props and use viewBox for scaling. For example:exportconstWaves=({ width =1200, height =638, ...props}: React.SVGProps<SVGSVGElement>&{width?: number;height?: number})=>(<svgxmlns="http://www.w3.org/2000/svg"width={width}height={height}viewBox="0 0 1200 638"preserveAspectRatio="xMidYMid meet"fill="none"{...props}>{/* ... rest of the SVG content ... */}</svg>);Consider path complexity:
The single path element is very complex. If possible, consider breaking it down into multiple simpler paths or using a different approach to create the wave effect. This could improve performance and make the component easier to maintain and modify in the future.To assess the impact of the complex path on performance, you could run a performance profiling tool. Here's a script to check if there are any performance-related comments in the codebase:
| images: [ | ||
| `/og?title=${encodeURIComponent( | ||
| post.title, | ||
| )}&readTime=${post.readTimeMins}&author=${encodeURIComponent( | ||
| post.user.name, | ||
| )}&date=${post.updatedAt}`, | ||
| ], |
There was a problem hiding this comment.
💡 Codebase verification
readTimeMins is properly defined and usage is safe.
The readTimeMins property is consistently defined in the codebase, ensuring that its usage does not require additional null checks. This addresses and resolves the concern about potential undefined errors.
However, there is still an inconsistency between the openGraph.images and twitter.images properties that needs to be addressed to maintain metadata consistency across platforms.
🔗 Analysis chain
Improved Open Graph metadata with article details.
The addition of readTime, author, and date to the Open Graph image URL enhances the metadata, aligning with the PR objective. Good use of encodeURIComponent for proper URL encoding.
However, there are two points to consider:
- The
twitter.imagesproperty hasn't been updated to match the new format used inopenGraph.images. This could lead to inconsistency in metadata across platforms.
Consider updating the twitter.images property to match:
twitter: {description: post.excerpt,images: [`/og?title=${encodeURIComponent(post.title)}&readTime=${post.readTimeMins}&author=${encodeURIComponent(post.user.name)}&date=${post.updatedAt}`],},- The
readTimeMinsproperty is used without checking if it exists, which could potentially lead to errors if the property is undefined.
Consider adding a null check or providing a default value:
readTime=${post.readTimeMins??0}Let's verify the existence and type of the readTimeMins property:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash# Description: Check for the existence and usage of readTimeMins property in the codebase# Test 1: Check if readTimeMins is defined in any type definitionsecho"Checking for readTimeMins in type definitions:"
rg --type typescript "readTimeMins.*:" -g "!*.test.ts" -g "!*.spec.ts"# Test 2: Check for usage of readTimeMins in the codebaseecho"\nChecking for usage of readTimeMins:"
rg --type typescript "\.readTimeMins" -g "!*.test.ts" -g "!*.spec.ts"Length of output: 402
Script:
#!/bin/bash# Description: Check for the existence and usage of readTimeMins property in the codebase# Test 1: Check if readTimeMins is defined in any type definitionsecho"Checking for readTimeMins in type definitions:"
rg "readTimeMins.*:" -g "!*.test.ts" -g "!*.spec.ts"# Test 2: Check for usage of readTimeMins in the codebaseecho"\nChecking for usage of readTimeMins:"
rg "\.readTimeMins" -g "!*.test.ts" -g "!*.spec.ts"Length of output: 1935
| export const Stars = ( | ||
| props: React.JSX.IntrinsicAttributes & React.SVGProps<SVGSVGElement>, | ||
| ) => ( | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={1131} | ||
| height={601} | ||
| fill="none" | ||
| {...props} | ||
| > | ||
| <path | ||
| fill="#fff" | ||
| fillOpacity={0.2} | ||
| d="M104.227 331.67a4.949 4.949 0 1 1-9.896 0 4.949 4.949 0 1 1 9.896 0ZM866.23 36.452a2.25 2.25 0 1 1-4.5.001 2.25 2.25 0 0 1 4.5 0ZM151.009 220.963a2.25 2.25 0 1 1-4.5.002 2.25 2.25 0 0 1 4.5-.002ZM1110.04 374.873c0 1.242-1.01 2.25-2.25 2.25a2.246 2.246 0 0 1-2.25-2.25c0-1.243 1-2.25 2.25-2.25 1.24 0 2.25 1.007 2.25 2.25ZM358.828 85.055a2.25 2.25 0 1 1-4.5.001 2.25 2.25 0 0 1 4.5 0ZM1111.83 21.151a2.245 2.245 0 1 1-4.49 0 2.245 2.245 0 1 1 4.49 0ZM1096.54 149.859c0 1.243-1.01 2.25-2.25 2.25a2.251 2.251 0 0 1-2.25-2.25c0-1.243 1.01-2.25 2.25-2.25s2.25 1.007 2.25 2.25ZM729.484 111.157a2.25 2.25 0 1 1-4.5 0 2.25 2.25 0 0 1 4.5 0ZM20.56 2.25a2.25 2.25 0 1 1-4.5.001 2.25 2.25 0 0 1 4.5 0Z" | ||
| /> | ||
| <path | ||
| fill="#fff" | ||
| fillOpacity={0.27} | ||
| d="M31.565 330.348a2.251 2.251 0 0 1 .761-4.436 2.25 2.25 0 0 1-.76 4.436ZM34.12 230.333a2.25 2.25 0 0 1 .761-4.436 2.25 2.25 0 0 1-.76 4.436ZM63.181 215.232a2.25 2.25 0 1 1 .762-4.434 2.25 2.25 0 0 1-.761 4.434ZM30.719 260.798a2.25 2.25 0 0 1 .76-4.436 2.251 2.251 0 0 1-.76 4.436ZM59.623 156.175a2.25 2.25 0 1 1 .762-4.434 2.25 2.25 0 0 1-.762 4.434ZM97.22 149.847a2.25 2.25 0 0 1 .762-4.436 2.25 2.25 0 0 1-.761 4.436ZM1077.05 141.685c.69-.572.78-1.588.2-2.27a1.6 1.6 0 0 0-2.26-.201 1.598 1.598 0 0 0-.2 2.271 1.6 1.6 0 0 0 2.26.2ZM1039.43 80.706c.69-.571.78-1.588.2-2.27a1.61 1.61 0 1 0-2.47 2.07c.58.682 1.59.772 2.27.2ZM1016.01 81.843a1.612 1.612 0 0 0-2.07-2.47 1.606 1.606 0 0 0-.2 2.27 1.61 1.61 0 0 0 2.27.2ZM1052.52 98.335a1.611 1.611 0 1 0-2.07-2.472 1.611 1.611 0 0 0 2.07 2.472ZM996.929 44.008a1.612 1.612 0 0 0-2.07-2.47 1.611 1.611 0 1 0 2.07 2.47ZM971.378 53.651a1.611 1.611 0 1 0-2.068-2.471 1.611 1.611 0 0 0 2.068 2.471Z" | ||
| /> | ||
| <path | ||
| fill="#fff" | ||
| fillOpacity={0.2} | ||
| d="M4.498 444.841a2.25 2.25 0 1 1-4.5 0 2.25 2.25 0 0 1 4.5 0Z" | ||
| /> | ||
| <path | ||
| fill="#fff" | ||
| fillOpacity={0.27} | ||
| d="M1128.37 438.118a2.256 2.256 0 0 1-1.84-2.598 2.255 2.255 0 0 1 2.6-1.838 2.25 2.25 0 1 1-.76 4.436Z" | ||
| /> | ||
| <path | ||
| fill="#fff" | ||
| fillOpacity={0.2} | ||
| d="M831.276 591.549a4.95 4.95 0 0 1-4.948 4.951 4.95 4.95 0 0 1-4.948-4.951 4.949 4.949 0 1 1 9.896 0ZM36.886 525.845a4.95 4.95 0 0 1-4.948 4.951 4.95 4.95 0 0 1-4.948-4.951 4.95 4.95 0 0 1 4.948-4.95 4.95 4.95 0 0 1 4.948 4.95ZM1030.1 580.749c0 1.242-1.01 2.25-2.25 2.25a2.252 2.252 0 0 1-2.25-2.25c0-1.243 1.01-2.25 2.25-2.25s2.25 1.007 2.25 2.25ZM599.166 562.748a2.25 2.25 0 1 1-4.499.001 2.25 2.25 0 0 1 4.499-.001Z" | ||
| /> | ||
| <path | ||
| fill="#fff" | ||
| fillOpacity={0.27} | ||
| d="M462.42 584.349a2.25 2.25 0 1 1-4.499.001 2.25 2.25 0 0 1 4.499-.001ZM363.458 598.75a2.25 2.25 0 1 1-4.499.001 2.25 2.25 0 0 1 4.499-.001ZM343.666 572.648a2.25 2.25 0 1 1-4.499.001 2.25 2.25 0 0 1 4.499-.001ZM394.046 596.95a2.25 2.25 0 1 1-4.499.001 2.25 2.25 0 0 1 4.499-.001ZM286.088 586.149a2.249 2.249 0 1 1-4.498 0 2.25 2.25 0 1 1 4.498 0ZM273.493 550.147a2.25 2.25 0 1 1-4.499.001 2.25 2.25 0 0 1 4.499-.001Z" | ||
| /> | ||
| </svg> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving responsiveness of the Stars component
The Stars component is well-structured and uses TypeScript for type safety. However, the fixed dimensions (width: 1131, height: 601) might limit its responsiveness across different screen sizes.
Consider making the component more responsive by:
- Accepting width and height as optional props with default values.
- Using percentage values or viewBox for scaling.
Here's an example of how you could modify the component:
exportconstStars=({
width =1131,
height =601,
...props}: React.SVGProps<SVGSVGElement>&{width?: number;height?: number})=>(<svgxmlns="http://www.w3.org/2000/svg"width={width}height={height}viewBox="0 0 1131 601"preserveAspectRatio="xMidYMid meet"fill="none"{...props}>{/* ... rest of the SVG content ... */}</svg>);This change allows the component to be more flexible while maintaining its aspect ratio.
| const regularFontData = await fetch( | ||
| new URL("@/assets/Lato-Regular.ttf", import.meta.url), | ||
| ).then((res) => res.arrayBuffer()); | ||
| const boldFontData = await fetch( | ||
| new URL("@/assets/Lato-Bold.ttf", import.meta.url), | ||
| ).then((res) => res.arrayBuffer()); | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Suggestion: Fetch font data in parallel to improve performance
Currently, the font data for regular and bold fonts is fetched sequentially, which can lead to increased response times. Fetching both fonts in parallel will optimize performance by reducing the total fetch time.
Apply this diff to fetch font data in parallel:
- const regularFontData = await fetch(- new URL("@/assets/Lato-Regular.ttf", import.meta.url),- ).then((res) => res.arrayBuffer());-- const boldFontData = await fetch(- new URL("@/assets/Lato-Bold.ttf", import.meta.url),- ).then((res) => res.arrayBuffer());+ const [regularFontData, boldFontData] = await Promise.all([+ fetch(new URL("@/assets/Lato-Regular.ttf", import.meta.url)).then((res) => res.arrayBuffer()),+ fetch(new URL("@/assets/Lato-Bold.ttf", import.meta.url)).then((res) => res.arrayBuffer()),+ ]);📝 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.
| constregularFontData=awaitfetch( | |
| newURL("@/assets/Lato-Regular.ttf",import.meta.url), | |
| ).then((res)=>res.arrayBuffer()); | |
| constboldFontData=awaitfetch( | |
| newURL("@/assets/Lato-Bold.ttf",import.meta.url), | |
| ).then((res)=>res.arrayBuffer()); | |
| const[regularFontData,boldFontData]=awaitPromise.all([ | |
| fetch(newURL("@/assets/Lato-Regular.ttf",import.meta.url)).then((res)=>res.arrayBuffer()), | |
| fetch(newURL("@/assets/Lato-Bold.ttf",import.meta.url)).then((res)=>res.arrayBuffer()), | |
| ]); |
| if (isNaN(date.getTime())) { | ||
| throw new Error("Invalid date"); | ||
| } |
There was a problem hiding this comment.
Replace unsafe isNaN with Number.isNaN
Using the global isNaN function is unsafe because it performs type coercion, which may lead to unexpected results. The Number.isNaN method provides a more reliable check by ensuring the value is of the Number type and is actually NaN.
Apply this diff to fix the issue:
- if (isNaN(date.getTime())) {+ if (Number.isNaN(date.getTime())) {📝 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.
| if(isNaN(date.getTime())){ | |
| thrownewError("Invalid date"); | |
| } | |
| if(Number.isNaN(date.getTime())){ | |
| thrownewError("Invalid date"); | |
| } |
🧰 Tools
🪛 Biome
[error] 198-198: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
✨ Codu Pull Request 💻
Pull Request details
Any Breaking changes
Associated Screenshots