-
-
Notifications
You must be signed in to change notification settings - Fork 0
Add YouTube video embed support to article reader #32
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: master
Are you sure you want to change the base?
Conversation
- Replace YouTube thumbnail links with functional video players - Support multiple YouTube URL formats (embed, watch, youtu.be) - Add responsive iframe styling with proper aspect ratios - Enhance reader experience with in-line video playback 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Summary by CodeRabbit
WalkthroughYouTube video embedding support was added to the article reader by detecting YouTube links in markdown and rendering them as embedded iframes. CSS rules were updated to style iframes consistently, with specific enhancements for YouTube embeds, including minimum and maximum heights and a box-shadow for emphasis. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ArticleContent
participant MarkdownRenderer
participant YouTube
User->>ArticleContent: Loads article with markdown
ArticleContent->>MarkdownRenderer: Render markdown content
MarkdownRenderer->>MarkdownRenderer: Detect link
alt Link is YouTube video
MarkdownRenderer->>YouTube: Embed video via iframe
YouTube-->>MarkdownRenderer: Serve video
else Not a YouTube link
MarkdownRenderer->>MarkdownRenderer: Render as standard link
end
MarkdownRenderer-->>ArticleContent: Rendered content (links/embeds)
ArticleContent-->>User: Displays article with embedded YouTube videos
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/Feed/ArticleReader/ArticleReader.tsxOops! Something went wrong! :( ESLint: 9.21.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.2.1_eslint@9.21.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. components/Feed/ArticleReader/ArticleReader.cssOops! Something went wrong! :( ESLint: 9.21.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.2.1_eslint@9.21.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/Feed/ArticleReader/ArticleReader.css
(2 hunks)components/Feed/ArticleReader/ArticleReader.tsx
(2 hunks)
🔇 Additional comments (2)
components/Feed/ArticleReader/ArticleReader.css (2)
120-122
: Good addition to ensure consistent iframe styling.Adding
.reader-view-article iframe
to the selector ensures that YouTube iframes embedded through the new functionality receive consistent styling with other embedded content.
133-138
: Well-designed YouTube embed styling.The styling choices are excellent:
- 315px minimum height aligns with YouTube's recommended embed dimensions
- 500px maximum height prevents oversized videos
- Subtle box-shadow provides visual emphasis without distraction
const isYouTubeEmbedUrl = (url: string): boolean => { | ||
return url.includes('youtube.com/embed/') || url.includes('youtu.be/'); | ||
}; |
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.
🛠️ Refactor suggestion
Function name and logic inconsistency with YouTube URL detection.
The function name isYouTubeEmbedUrl
suggests it only detects embed URLs, but it's missing the youtube.com/watch?v=
format that extractYouTubeVideoId
handles. This creates inconsistency where some YouTube URLs may pass the extraction but fail the detection.
Rename the function to be more accurate and consistent:
-const isYouTubeEmbedUrl = (url: string): boolean => {
- return url.includes('youtube.com/embed/') || url.includes('youtu.be/');
+const isYouTubeUrl = (url: string): boolean => {
+ return url.includes('youtube.com/embed/') ||
+ url.includes('youtu.be/') ||
+ url.includes('youtube.com/watch?v=');
And update the usage accordingly in line 719.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/Feed/ArticleReader/ArticleReader.tsx around lines 26 to 28, the
function named isYouTubeEmbedUrl does not accurately reflect its logic because
it misses the youtube.com/watch?v= URL format handled by extractYouTubeVideoId.
Rename this function to something like isYouTubeUrl to better represent its
purpose and update its logic to include the youtube.com/watch?v= pattern. Also,
update all usages of this function accordingly, including the one at line 719,
to maintain consistency.
a: ({ href, children, ...props }: { href?: string; children?: React.ReactNode; [key: string]: unknown }) => { | ||
// Check if this is a YouTube embed link | ||
if (href && isYouTubeEmbedUrl(href)) { | ||
const videoId = extractYouTubeVideoId(href); | ||
if (videoId) { | ||
// Return the iframe directly - the nesting warning can be safely ignored | ||
// as this provides better user experience than showing just a link | ||
return ( | ||
<iframe | ||
src={`https://www.youtube.com/embed/${videoId}`} | ||
title="YouTube video player" | ||
className="w-full aspect-video rounded-lg my-8 block" | ||
frameBorder="0" | ||
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share" | ||
allowFullScreen | ||
loading="lazy" | ||
style={{ display: 'block', margin: '2rem auto' }} | ||
/> | ||
); | ||
} | ||
} | ||
|
||
return ( | ||
<a | ||
href={href} | ||
className="text-primary hover:text-primary/80 underline decoration-primary/30 hover:decoration-primary/60 transition-colors" | ||
target={href?.startsWith('http') ? '_blank' : undefined} | ||
rel={href?.startsWith('http') ? 'noopener noreferrer' : undefined} | ||
{...props} | ||
> | ||
{children} | ||
</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.
🧹 Nitpick (assertive)
Excellent YouTube embed implementation with minor security enhancement opportunities.
The implementation correctly handles YouTube URL detection, video ID extraction, and fallback behavior. The accessibility and performance considerations (lazy loading, proper titles) are well implemented.
Consider adding sandbox attributes for enhanced security:
return (
<iframe
src={`https://www.youtube.com/embed/${videoId}`}
title="YouTube video player"
className="w-full aspect-video rounded-lg my-8 block"
frameBorder="0"
+ sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share"
allowFullScreen
loading="lazy"
style={{ display: 'block', margin: '2rem auto' }}
/>
);
The sandbox attribute provides an additional layer of security by restricting iframe capabilities to only what's necessary for YouTube embeds.
📝 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.
a: ({ href, children, ...props }: { href?: string; children?: React.ReactNode; [key: string]: unknown }) => { | |
// Check if this is a YouTube embed link | |
if (href && isYouTubeEmbedUrl(href)) { | |
const videoId = extractYouTubeVideoId(href); | |
if (videoId) { | |
// Return the iframe directly - the nesting warning can be safely ignored | |
// as this provides better user experience than showing just a link | |
return ( | |
<iframe | |
src={`https://www.youtube.com/embed/${videoId}`} | |
title="YouTube video player" | |
className="w-full aspect-video rounded-lg my-8 block" | |
frameBorder="0" | |
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share" | |
allowFullScreen | |
loading="lazy" | |
style={{ display: 'block', margin: '2rem auto' }} | |
/> | |
); | |
} | |
} | |
return ( | |
<a | |
href={href} | |
className="text-primary hover:text-primary/80 underline decoration-primary/30 hover:decoration-primary/60 transition-colors" | |
target={href?.startsWith('http') ? '_blank' : undefined} | |
rel={href?.startsWith('http') ? 'noopener noreferrer' : undefined} | |
{...props} | |
> | |
{children} | |
</a> | |
); | |
}, | |
a: ({ href, children, ...props }: { href?: string; children?: React.ReactNode; [key: string]: unknown }) => { | |
// Check if this is a YouTube embed link | |
if (href && isYouTubeEmbedUrl(href)) { | |
const videoId = extractYouTubeVideoId(href); | |
if (videoId) { | |
// Return the iframe directly - the nesting warning can be safely ignored | |
// as this provides better user experience than showing just a link | |
return ( | |
<iframe | |
src={`https://www.youtube.com/embed/${videoId}`} | |
title="YouTube video player" | |
className="w-full aspect-video rounded-lg my-8 block" | |
frameBorder="0" | |
+ sandbox="allow-scripts allow-same-origin allow-presentation" | |
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share" | |
allowFullScreen | |
loading="lazy" | |
style={{ display: 'block', margin: '2rem auto' }} | |
/> | |
); | |
} | |
} | |
return ( | |
<a | |
href={href} | |
className="text-primary hover:text-primary/80 underline decoration-primary/30 hover:decoration-primary/60 transition-colors" | |
target={href?.startsWith('http') ? '_blank' : undefined} | |
rel={href?.startsWith('http') ? 'noopener noreferrer' : undefined} | |
{...props} | |
> | |
{children} | |
</a> | |
); | |
}, |
🤖 Prompt for AI Agents
In components/Feed/ArticleReader/ArticleReader.tsx around lines 717 to 750, the
YouTube iframe embed is correctly implemented but lacks sandbox attributes for
enhanced security. Add a sandbox attribute to the iframe element with
appropriate values such as "allow-scripts allow-same-origin allow-presentation"
to restrict iframe capabilities while still allowing necessary functionality.
This will improve security by limiting what the embedded content can do.
const extractYouTubeVideoId = (url: string): string | null => { | ||
try { | ||
const urlObj = new URL(url); | ||
|
||
// Handle youtube.com/embed/VIDEO_ID format | ||
if (urlObj.hostname === 'www.youtube.com' || urlObj.hostname === 'youtube.com') { | ||
const embedMatch = urlObj.pathname.match(/^\/embed\/([a-zA-Z0-9_-]+)/); | ||
if (embedMatch) { | ||
return embedMatch[1]; | ||
} | ||
|
||
// Handle youtube.com/watch?v=VIDEO_ID format | ||
const watchMatch = urlObj.searchParams.get('v'); | ||
if (watchMatch) { | ||
return watchMatch; | ||
} | ||
} | ||
|
||
// Handle youtu.be/VIDEO_ID format | ||
if (urlObj.hostname === 'youtu.be') { | ||
const shortMatch = urlObj.pathname.match(/^\/([a-zA-Z0-9_-]+)/); | ||
if (shortMatch) { | ||
return shortMatch[1]; | ||
} | ||
} | ||
|
||
return null; | ||
} catch { | ||
return null; | ||
} | ||
}; |
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.
🧹 Nitpick (assertive)
Consider enhancing video ID validation for better accuracy.
The function correctly handles multiple YouTube URL formats and error cases. However, YouTube video IDs are consistently 11 characters long, which could be used for additional validation.
Consider adding video ID length validation:
const extractYouTubeVideoId = (url: string): string | null => {
try {
const urlObj = new URL(url);
// Handle youtube.com/embed/VIDEO_ID format
if (urlObj.hostname === 'www.youtube.com' || urlObj.hostname === 'youtube.com') {
- const embedMatch = urlObj.pathname.match(/^\/embed\/([a-zA-Z0-9_-]+)/);
+ const embedMatch = urlObj.pathname.match(/^\/embed\/([a-zA-Z0-9_-]{11})/);
if (embedMatch) {
return embedMatch[1];
}
// Handle youtube.com/watch?v=VIDEO_ID format
const watchMatch = urlObj.searchParams.get('v');
- if (watchMatch) {
+ if (watchMatch && watchMatch.length === 11) {
return watchMatch;
}
}
// Handle youtu.be/VIDEO_ID format
if (urlObj.hostname === 'youtu.be') {
- const shortMatch = urlObj.pathname.match(/^\/([a-zA-Z0-9_-]+)/);
+ const shortMatch = urlObj.pathname.match(/^\/([a-zA-Z0-9_-]{11})/);
if (shortMatch) {
return shortMatch[1];
}
}
return null;
} catch {
return null;
}
};
📝 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 extractYouTubeVideoId = (url: string): string | null => { | |
try { | |
const urlObj = new URL(url); | |
// Handle youtube.com/embed/VIDEO_ID format | |
if (urlObj.hostname === 'www.youtube.com' || urlObj.hostname === 'youtube.com') { | |
const embedMatch = urlObj.pathname.match(/^\/embed\/([a-zA-Z0-9_-]+)/); | |
if (embedMatch) { | |
return embedMatch[1]; | |
} | |
// Handle youtube.com/watch?v=VIDEO_ID format | |
const watchMatch = urlObj.searchParams.get('v'); | |
if (watchMatch) { | |
return watchMatch; | |
} | |
} | |
// Handle youtu.be/VIDEO_ID format | |
if (urlObj.hostname === 'youtu.be') { | |
const shortMatch = urlObj.pathname.match(/^\/([a-zA-Z0-9_-]+)/); | |
if (shortMatch) { | |
return shortMatch[1]; | |
} | |
} | |
return null; | |
} catch { | |
return null; | |
} | |
}; | |
const extractYouTubeVideoId = (url: string): string | null => { | |
try { | |
const urlObj = new URL(url); | |
// Handle youtube.com/embed/VIDEO_ID format | |
if (urlObj.hostname === 'www.youtube.com' || urlObj.hostname === 'youtube.com') { | |
const embedMatch = urlObj.pathname.match(/^\/embed\/([a-zA-Z0-9_-]{11})/); | |
if (embedMatch) { | |
return embedMatch[1]; | |
} | |
// Handle youtube.com/watch?v=VIDEO_ID format | |
const watchMatch = urlObj.searchParams.get('v'); | |
if (watchMatch && watchMatch.length === 11) { | |
return watchMatch; | |
} | |
} | |
// Handle youtu.be/VIDEO_ID format | |
if (urlObj.hostname === 'youtu.be') { | |
const shortMatch = urlObj.pathname.match(/^\/([a-zA-Z0-9_-]{11})/); | |
if (shortMatch) { | |
return shortMatch[1]; | |
} | |
} | |
return null; | |
} catch { | |
return null; | |
} | |
}; |
🤖 Prompt for AI Agents
In components/Feed/ArticleReader/ArticleReader.tsx between lines 30 and 60, the
extractYouTubeVideoId function extracts video IDs but does not validate their
length. To improve accuracy, add a check to ensure the extracted video ID is
exactly 11 characters long before returning it. If the ID length is incorrect,
return null instead.
Summary
Enhanced the article reader to display functional YouTube video players instead of static thumbnail images with external links.
Changes Made
Technical Details
youtube.com/embed/
,youtube.com/watch?v=
, andyoutu.be/
Test Plan
🤖 Generated with Claude Code