131 changes: 131 additions & 0 deletions app/(app)/courses/[slug]/[id]/_client.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
"use client";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { CircleCheck, SquarePlay } from "lucide-react";
import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { mockContentList } from "../../mock";

interface ContentProps {
session: Session | null;
}
Comment on lines +8 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove unused session prop

The session prop is defined in the interface but is not used anywhere within the Content component implementation. The component only uses feature flag checks and renders child components without any session-dependent functionality.

  • Remove the session prop from the ContentProps interface
  • Remove the session parameter from the Content component definition
🔗 Analysis chain

Verify usage of session prop

The session prop is defined in the interface but doesn't appear to be used within the Content component. Either remove the unused prop or implement the intended session-dependent functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash# Search for session usage in the component
ast-grep --pattern 'session.$$$'

Length of output: 10367


Script:

#!/bin/bash# Let's search for the Content component implementation and session usage
ast-grep --pattern 'const Content = ({ session }: ContentProps) => { $$$}'

Length of output: 1434


const Content = ({ session }: ContentProps) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);

if (!flagEnabled) {
notFound();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible stupid question on this one Carolina. Can you put featureflag the route? Then you can just let nexts router deal with the notFound ?

Reason this might be a stupid queston is I have no idea how nexts router works 🤣 I am basing this comment from my knowledge of react router

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we could but that's a good pattern that we could have. We will figure it out


return (
<div className="overflow-auto">
<div className="w-full divide-x divide-gray-700 lg:grid lg:grid-cols-12">
<VideoPlayer />
<Sidebar contentList={mockContentList} />
</div>
<div className="mx-auto max-w-5xl">
<ContentList contentList={mockContentList} />
</div>
</div>
);
};
Comment on lines +12 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error boundaries and loading states

The component should handle potential errors from mock data and add loading states for better UX. Consider implementing:

  1. Error boundaries to catch rendering errors
  2. Loading states while checking feature flags
  3. Proper error handling for missing or malformed mock data

Here's a suggested implementation:

 const Content = ({ session }: ContentProps) => {
+ const [isLoading, setIsLoading] = useState(true);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);
if (!flagEnabled) {
notFound();
}
+ if (isLoading) {+ return <div>Loading...</div>;+ }++ if (!mockContentList?.length) {+ return <div>No content available</div>;+ }+
return (
<div className="overflow-auto">

Committable suggestion was skipped due to low confidence.


const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
Comment on lines +32 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance iframe security and performance

The YouTube iframe implementation needs security and performance improvements:

Apply these security and performance enhancements:

 const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
+ loading="lazy"+ sandbox="allow-scripts allow-same-origin allow-presentation"+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"+ referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);

Also consider:

  1. Making the video URL configurable through props
  2. Adding error handling for failed video loads
  3. Implementing a loading placeholder
📝 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.

Suggested change
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
loading="lazy"
sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);


interface SidebarProps {
contentList: typeof mockContentList;
}

const Sidebar = ({ contentList }: SidebarProps) => (
<div className="col-span-3 flex w-full flex-col overflow-auto">
<ul className="divide-y divide-gray-700">
{contentList && contentList.length > 0 ? (
contentList.map((item, index) => (
<SidebarItem key={index} item={item} />
))
) : (
<li className="text-center text-gray-500">No content available</li>
)}
</ul>
</div>
);

interface SidebarItemProps {
item: (typeof mockContentList)[0];
}

const SidebarItem = ({ item }: SidebarItemProps) => (
<li className="flex flex-row items-center justify-between px-2 py-2">
<div className="flex flex-row items-center">
<SquarePlay
className="mr-4 h-5 w-5 text-white group-hover:text-white"
aria-hidden="true"
/>
<p>{item.title}</p>
</div>
<CircleCheck
className={`h-6 w-6 ${
item.watched ? "mr-2 h-5 w-5 text-pink-600" : "mr-2 h-5 w-5 text-white"
}`}
aria-hidden="true"
/>
</li>
);
Comment on lines +44 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve accessibility and type definitions

The sidebar implementation needs accessibility enhancements and better type definitions:

  1. Replace mock data type with proper interface:
interfaceContentItem{id: string;title: string;watched: boolean;// ... other properties}interfaceSidebarProps{contentList: ContentItem[];}
  1. Enhance accessibility:
-<ul className="divide-y divide-gray-700">+<ul + className="divide-y divide-gray-700"+ role="list"+ aria-label="Course content"+>-<li className="flex flex-row items-center justify-between px-2 py-2">+<li + className="flex flex-row items-center justify-between px-2 py-2"+ tabIndex={0}+ role="listitem"+>


interface ContentListProps {
contentList: typeof mockContentList;
}

const ContentList = ({ contentList }: ContentListProps) => {
return (
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<li key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
Comment on lines +88 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML structure and accessibility issues

The ContentList component has several structural and accessibility issues:

Apply these fixes:

 const ContentList = ({ contentList }: ContentListProps) => {
return (
- <>+ <div className="content-list" role="region" aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
- <li key={author} className="flex justify-between gap-x-6 py-5">+ <div key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
- alt=""+ alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
- </li>+ </div>- <div>{description}</div>- <div>{longDescription}</div>+ <div role="article">+ <h3 className="sr-only">Course description</h3>+ <p>{description}</p>+ <p>{longDescription}</p>+ </div>
</div>
),
)}
- </>+ </div>
);
};
📝 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.

Suggested change
constContentList=({ contentList }: ContentListProps)=>{
return(
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<likey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
constContentList=({ contentList }: ContentListProps)=>{
return(
<divclassName="content-list"role="region"aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<divkey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</div>
<divrole="article">
<h3className="sr-only">Course description</h3>
<p>{description}</p>
<p>{longDescription}</p>
</div>
</div>
),
)}
</div>
);
};


export default Content;
13 changes: 13 additions & 0 deletions app/(app)/courses/[slug]/[id]/page.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
import { getServerAuthSession } from "@/server/auth";
import Content from "./_client";
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding loading and error states.

The page lacks loading and error handling components which are important for better UX.

Add loading.tsx and error.tsx files in the same directory:

// loading.tsxexportdefaultfunctionLoading(){return<div>Loadingcoursecontent...</div>;}// error.tsxexportdefaultfunctionError({
error,
reset,}: {error: Error;reset: ()=>void;}){return(<div><h2>Somethingwentwrong!</h2><buttononClick={reset}>Tryagain</button></div>);}


export const metadata = {
title: "Video Course",
};
Comment on lines +4 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance metadata with dynamic course information.

The current metadata title is static. Consider utilizing the route parameters to display the actual course title.

export const metadata = {
- title: "Video Course",+ title: async function generateTitle({ params }) {+ const { slug } = params;+ return `${slug.replace('-', ' ')} | Video Course`;+ },
};

Committable suggestion was skipped due to low confidence.


export default async function Page() {
// Example of grabbing session in case it is needed
const session = await getServerAuthSession();

return <Content session={session} />;
}
Comment on lines +8 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize route parameters and add error handling.

The page component doesn't use the URL parameters ([slug] and [id]) which are essential for fetching the correct course content.

Consider implementing proper error handling and course data fetching:

export default async function Page({
+ params,+ }: {+ params: { slug: string; id: string };
}) {
const session = await getServerAuthSession();
++ // Fetch course data using params.slug and params.id+ const course = await getCourse(params.slug, params.id);++ if (!course) {+ notFound();+ }- return <Content session={session} />;+ return <Content session={session} course={course} />;
}

Committable suggestion was skipped due to low confidence.

130 changes: 130 additions & 0 deletions app/(app)/courses/mock.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
// Mock video source (this would typically be a path to a video file)
const mockVideoSrc = "https://youtu.be/Ln4KSN0rchI?t=12";

// Mock content list
const mockContentList = [
{
id: 1,
title: "Introduction to React",
longDescription:
"In this video, we will explore the fundamentals of React, including its component-based architecture, virtual DOM, and how to get started with your first React application. This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description: "Learn the basics of React and its core concepts",
watched: false,
publishedDate: "2023-01-15",
author: "John Doe",
imageLink: "https://example.com/images/react-introduction.jpg",
},
{
id: 2,
title: "Setting Up Your Development Environment",
longDescription:
"This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description:
"Install and configure the necessary tools for React development",
watched: true,
publishedDate: "2023-01-22",
author: "Jane Smith",
},
{
id: 3,
title: "Components and Props",
longDescription:
"Dive deep into React's core concepts of components and props. Learn how to create reusable components and pass data between them using props.",
description: "Understand the building blocks of React applications",
watched: true,
publishedDate: "2023-01-29",
author: "John Doe",
},
{
id: 4,
title: "State and Lifecycle",
longDescription:
"Explore how to manage component state and lifecycle methods to build dynamic and interactive applications with React.",
description: "Manage component state and lifecycle methods",
watched: true,
publishedDate: "2023-02-05",
author: "Jane Smith",
},
{
id: 5,
title: "Handling Events",
longDescription:
"Learn how to handle user interactions in React, including events like clicks, form submissions, and more.",
description: "Learn how to handle user interactions in React",
watched: true,
publishedDate: "2023-02-12",
author: "John Doe",
},
{
id: 6,
title: "Conditional Rendering",
longDescription:
"Discover how to display different UI elements based on specific conditions in your React applications.",
description: "Display different UI based on conditions",
watched: false,
publishedDate: "2023-02-19",
author: "Jane Smith",
},
{
id: 7,
title: "Lists and Keys",
longDescription:
"Learn the best practices for rendering lists of components in React and how to efficiently manage them with unique keys.",
description: "Render multiple components efficiently",
watched: false,
publishedDate: "2023-02-26",
author: "John Doe",
},
{
id: 8,
title: "Forms in React",
longDescription:
"This video covers how to create and manage forms in React applications, including controlled and uncontrolled components.",
description: "Create and handle form inputs in React applications",
watched: false,
publishedDate: "2023-03-05",
author: "Jane Smith",
},
{
id: 9,
title: "Hooks: useState and useEffect",
longDescription:
"An introduction to React Hooks, focusing on useState and useEffect to manage state and side effects in functional components.",
description: "Understand and use basic React Hooks",
watched: false,
publishedDate: "2023-03-12",
author: "John Doe",
},
{
id: 10,
title: "Custom Hooks",
longDescription:
"Learn how to create custom hooks in React to encapsulate and reuse logic across components.",
description: "Create reusable logic with custom Hooks",
watched: false,
publishedDate: "2023-03-19",
author: "Jane Smith",
},
{
id: 11,
title: "Context API",
longDescription:
"Explore the Context API to manage global state in your React applications without the need for prop drilling.",
description: "Manage global state in your React application",
watched: false,
publishedDate: "2023-03-26",
author: "John Doe",
},
{
id: 12,
title: "React Router",
longDescription:
"Learn how to implement navigation in your single-page applications using React Router, including dynamic routing and nested routes.",
description: "Implement navigation in your single-page application",
watched: false,
publishedDate: "2023-04-02",
author: "Jane Smith",
},
];

export { mockVideoSrc, mockContentList };
1 change: 1 addition & 0 deletions utils/flags.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
COURSE_VIDEO: "course-video",
JOBS: "jobs",
// Add more feature flags as needed
} as const;
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
131 changes: 131 additions & 0 deletions app/(app)/courses/[slug]/[id]/_client.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
"use client";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { CircleCheck, SquarePlay } from "lucide-react";
import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { mockContentList } from "../../mock";

interface ContentProps {
session: Session | null;
}
Comment on lines +8 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove unused session prop

The session prop is defined in the interface but is not used anywhere within the Content component implementation. The component only uses feature flag checks and renders child components without any session-dependent functionality.

  • Remove the session prop from the ContentProps interface
  • Remove the session parameter from the Content component definition
🔗 Analysis chain

Verify usage of session prop

The session prop is defined in the interface but doesn't appear to be used within the Content component. Either remove the unused prop or implement the intended session-dependent functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash# Search for session usage in the component
ast-grep --pattern 'session.$$$'

Length of output: 10367


Script:

#!/bin/bash# Let's search for the Content component implementation and session usage
ast-grep --pattern 'const Content = ({ session }: ContentProps) => { $$$}'

Length of output: 1434


const Content = ({ session }: ContentProps) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);

if (!flagEnabled) {
notFound();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible stupid question on this one Carolina. Can you put featureflag the route? Then you can just let nexts router deal with the notFound ?

Reason this might be a stupid queston is I have no idea how nexts router works 🤣 I am basing this comment from my knowledge of react router

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we could but that's a good pattern that we could have. We will figure it out


return (
<div className="overflow-auto">
<div className="w-full divide-x divide-gray-700 lg:grid lg:grid-cols-12">
<VideoPlayer />
<Sidebar contentList={mockContentList} />
</div>
<div className="mx-auto max-w-5xl">
<ContentList contentList={mockContentList} />
</div>
</div>
);
};
Comment on lines +12 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error boundaries and loading states

The component should handle potential errors from mock data and add loading states for better UX. Consider implementing:

  1. Error boundaries to catch rendering errors
  2. Loading states while checking feature flags
  3. Proper error handling for missing or malformed mock data

Here's a suggested implementation:

 const Content = ({ session }: ContentProps) => {
+ const [isLoading, setIsLoading] = useState(true);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);
if (!flagEnabled) {
notFound();
}
+ if (isLoading) {+ return <div>Loading...</div>;+ }++ if (!mockContentList?.length) {+ return <div>No content available</div>;+ }+
return (
<div className="overflow-auto">

Committable suggestion was skipped due to low confidence.


const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
Comment on lines +32 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance iframe security and performance

The YouTube iframe implementation needs security and performance improvements:

Apply these security and performance enhancements:

 const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
+ loading="lazy"+ sandbox="allow-scripts allow-same-origin allow-presentation"+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"+ referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);

Also consider:

  1. Making the video URL configurable through props
  2. Adding error handling for failed video loads
  3. Implementing a loading placeholder
📝 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.

Suggested change
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
loading="lazy"
sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);


interface SidebarProps {
contentList: typeof mockContentList;
}

const Sidebar = ({ contentList }: SidebarProps) => (
<div className="col-span-3 flex w-full flex-col overflow-auto">
<ul className="divide-y divide-gray-700">
{contentList && contentList.length > 0 ? (
contentList.map((item, index) => (
<SidebarItem key={index} item={item} />
))
) : (
<li className="text-center text-gray-500">No content available</li>
)}
</ul>
</div>
);

interface SidebarItemProps {
item: (typeof mockContentList)[0];
}

const SidebarItem = ({ item }: SidebarItemProps) => (
<li className="flex flex-row items-center justify-between px-2 py-2">
<div className="flex flex-row items-center">
<SquarePlay
className="mr-4 h-5 w-5 text-white group-hover:text-white"
aria-hidden="true"
/>
<p>{item.title}</p>
</div>
<CircleCheck
className={`h-6 w-6 ${
item.watched ? "mr-2 h-5 w-5 text-pink-600" : "mr-2 h-5 w-5 text-white"
}`}
aria-hidden="true"
/>
</li>
);
Comment on lines +44 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve accessibility and type definitions

The sidebar implementation needs accessibility enhancements and better type definitions:

  1. Replace mock data type with proper interface:
interfaceContentItem{id: string;title: string;watched: boolean;// ... other properties}interfaceSidebarProps{contentList: ContentItem[];}
  1. Enhance accessibility:
-<ul className="divide-y divide-gray-700">+<ul + className="divide-y divide-gray-700"+ role="list"+ aria-label="Course content"+>-<li className="flex flex-row items-center justify-between px-2 py-2">+<li + className="flex flex-row items-center justify-between px-2 py-2"+ tabIndex={0}+ role="listitem"+>


interface ContentListProps {
contentList: typeof mockContentList;
}

const ContentList = ({ contentList }: ContentListProps) => {
return (
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<li key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
Comment on lines +88 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML structure and accessibility issues

The ContentList component has several structural and accessibility issues:

Apply these fixes:

 const ContentList = ({ contentList }: ContentListProps) => {
return (
- <>+ <div className="content-list" role="region" aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
- <li key={author} className="flex justify-between gap-x-6 py-5">+ <div key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
- alt=""+ alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
- </li>+ </div>- <div>{description}</div>- <div>{longDescription}</div>+ <div role="article">+ <h3 className="sr-only">Course description</h3>+ <p>{description}</p>+ <p>{longDescription}</p>+ </div>
</div>
),
)}
- </>+ </div>
);
};
📝 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.

Suggested change
constContentList=({ contentList }: ContentListProps)=>{
return(
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<likey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
constContentList=({ contentList }: ContentListProps)=>{
return(
<divclassName="content-list"role="region"aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<divkey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</div>
<divrole="article">
<h3className="sr-only">Course description</h3>
<p>{description}</p>
<p>{longDescription}</p>
</div>
</div>
),
)}
</div>
);
};


export default Content;
13 changes: 13 additions & 0 deletions app/(app)/courses/[slug]/[id]/page.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
import { getServerAuthSession } from "@/server/auth";
import Content from "./_client";
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding loading and error states.

The page lacks loading and error handling components which are important for better UX.

Add loading.tsx and error.tsx files in the same directory:

// loading.tsxexportdefaultfunctionLoading(){return<div>Loadingcoursecontent...</div>;}// error.tsxexportdefaultfunctionError({
error,
reset,}: {error: Error;reset: ()=>void;}){return(<div><h2>Somethingwentwrong!</h2><buttononClick={reset}>Tryagain</button></div>);}


export const metadata = {
title: "Video Course",
};
Comment on lines +4 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance metadata with dynamic course information.

The current metadata title is static. Consider utilizing the route parameters to display the actual course title.

export const metadata = {
- title: "Video Course",+ title: async function generateTitle({ params }) {+ const { slug } = params;+ return `${slug.replace('-', ' ')} | Video Course`;+ },
};

Committable suggestion was skipped due to low confidence.


export default async function Page() {
// Example of grabbing session in case it is needed
const session = await getServerAuthSession();

return <Content session={session} />;
}
Comment on lines +8 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize route parameters and add error handling.

The page component doesn't use the URL parameters ([slug] and [id]) which are essential for fetching the correct course content.

Consider implementing proper error handling and course data fetching:

export default async function Page({
+ params,+ }: {+ params: { slug: string; id: string };
}) {
const session = await getServerAuthSession();
++ // Fetch course data using params.slug and params.id+ const course = await getCourse(params.slug, params.id);++ if (!course) {+ notFound();+ }- return <Content session={session} />;+ return <Content session={session} course={course} />;
}

Committable suggestion was skipped due to low confidence.

130 changes: 130 additions & 0 deletions app/(app)/courses/mock.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
// Mock video source (this would typically be a path to a video file)
const mockVideoSrc = "https://youtu.be/Ln4KSN0rchI?t=12";

// Mock content list
const mockContentList = [
{
id: 1,
title: "Introduction to React",
longDescription:
"In this video, we will explore the fundamentals of React, including its component-based architecture, virtual DOM, and how to get started with your first React application. This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description: "Learn the basics of React and its core concepts",
watched: false,
publishedDate: "2023-01-15",
author: "John Doe",
imageLink: "https://example.com/images/react-introduction.jpg",
},
{
id: 2,
title: "Setting Up Your Development Environment",
longDescription:
"This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description:
"Install and configure the necessary tools for React development",
watched: true,
publishedDate: "2023-01-22",
author: "Jane Smith",
},
{
id: 3,
title: "Components and Props",
longDescription:
"Dive deep into React's core concepts of components and props. Learn how to create reusable components and pass data between them using props.",
description: "Understand the building blocks of React applications",
watched: true,
publishedDate: "2023-01-29",
author: "John Doe",
},
{
id: 4,
title: "State and Lifecycle",
longDescription:
"Explore how to manage component state and lifecycle methods to build dynamic and interactive applications with React.",
description: "Manage component state and lifecycle methods",
watched: true,
publishedDate: "2023-02-05",
author: "Jane Smith",
},
{
id: 5,
title: "Handling Events",
longDescription:
"Learn how to handle user interactions in React, including events like clicks, form submissions, and more.",
description: "Learn how to handle user interactions in React",
watched: true,
publishedDate: "2023-02-12",
author: "John Doe",
},
{
id: 6,
title: "Conditional Rendering",
longDescription:
"Discover how to display different UI elements based on specific conditions in your React applications.",
description: "Display different UI based on conditions",
watched: false,
publishedDate: "2023-02-19",
author: "Jane Smith",
},
{
id: 7,
title: "Lists and Keys",
longDescription:
"Learn the best practices for rendering lists of components in React and how to efficiently manage them with unique keys.",
description: "Render multiple components efficiently",
watched: false,
publishedDate: "2023-02-26",
author: "John Doe",
},
{
id: 8,
title: "Forms in React",
longDescription:
"This video covers how to create and manage forms in React applications, including controlled and uncontrolled components.",
description: "Create and handle form inputs in React applications",
watched: false,
publishedDate: "2023-03-05",
author: "Jane Smith",
},
{
id: 9,
title: "Hooks: useState and useEffect",
longDescription:
"An introduction to React Hooks, focusing on useState and useEffect to manage state and side effects in functional components.",
description: "Understand and use basic React Hooks",
watched: false,
publishedDate: "2023-03-12",
author: "John Doe",
},
{
id: 10,
title: "Custom Hooks",
longDescription:
"Learn how to create custom hooks in React to encapsulate and reuse logic across components.",
description: "Create reusable logic with custom Hooks",
watched: false,
publishedDate: "2023-03-19",
author: "Jane Smith",
},
{
id: 11,
title: "Context API",
longDescription:
"Explore the Context API to manage global state in your React applications without the need for prop drilling.",
description: "Manage global state in your React application",
watched: false,
publishedDate: "2023-03-26",
author: "John Doe",
},
{
id: 12,
title: "React Router",
longDescription:
"Learn how to implement navigation in your single-page applications using React Router, including dynamic routing and nested routes.",
description: "Implement navigation in your single-page application",
watched: false,
publishedDate: "2023-04-02",
author: "Jane Smith",
},
];

export { mockVideoSrc, mockContentList };
1 change: 1 addition & 0 deletions utils/flags.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
COURSE_VIDEO: "course-video",
JOBS: "jobs",
// Add more feature flags as needed
} as const;
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
131 changes: 131 additions & 0 deletions app/(app)/courses/[slug]/[id]/_client.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
"use client";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { CircleCheck, SquarePlay } from "lucide-react";
import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { mockContentList } from "../../mock";

interface ContentProps {
session: Session | null;
}
Comment on lines +8 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove unused session prop

The session prop is defined in the interface but is not used anywhere within the Content component implementation. The component only uses feature flag checks and renders child components without any session-dependent functionality.

  • Remove the session prop from the ContentProps interface
  • Remove the session parameter from the Content component definition
🔗 Analysis chain

Verify usage of session prop

The session prop is defined in the interface but doesn't appear to be used within the Content component. Either remove the unused prop or implement the intended session-dependent functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash# Search for session usage in the component
ast-grep --pattern 'session.$$$'

Length of output: 10367


Script:

#!/bin/bash# Let's search for the Content component implementation and session usage
ast-grep --pattern 'const Content = ({ session }: ContentProps) => { $$$}'

Length of output: 1434


const Content = ({ session }: ContentProps) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);

if (!flagEnabled) {
notFound();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible stupid question on this one Carolina. Can you put featureflag the route? Then you can just let nexts router deal with the notFound ?

Reason this might be a stupid queston is I have no idea how nexts router works 🤣 I am basing this comment from my knowledge of react router

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we could but that's a good pattern that we could have. We will figure it out


return (
<div className="overflow-auto">
<div className="w-full divide-x divide-gray-700 lg:grid lg:grid-cols-12">
<VideoPlayer />
<Sidebar contentList={mockContentList} />
</div>
<div className="mx-auto max-w-5xl">
<ContentList contentList={mockContentList} />
</div>
</div>
);
};
Comment on lines +12 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error boundaries and loading states

The component should handle potential errors from mock data and add loading states for better UX. Consider implementing:

  1. Error boundaries to catch rendering errors
  2. Loading states while checking feature flags
  3. Proper error handling for missing or malformed mock data

Here's a suggested implementation:

 const Content = ({ session }: ContentProps) => {
+ const [isLoading, setIsLoading] = useState(true);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);
if (!flagEnabled) {
notFound();
}
+ if (isLoading) {+ return <div>Loading...</div>;+ }++ if (!mockContentList?.length) {+ return <div>No content available</div>;+ }+
return (
<div className="overflow-auto">

Committable suggestion was skipped due to low confidence.


const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
Comment on lines +32 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance iframe security and performance

The YouTube iframe implementation needs security and performance improvements:

Apply these security and performance enhancements:

 const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
+ loading="lazy"+ sandbox="allow-scripts allow-same-origin allow-presentation"+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"+ referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);

Also consider:

  1. Making the video URL configurable through props
  2. Adding error handling for failed video loads
  3. Implementing a loading placeholder
📝 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.

Suggested change
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
loading="lazy"
sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);


interface SidebarProps {
contentList: typeof mockContentList;
}

const Sidebar = ({ contentList }: SidebarProps) => (
<div className="col-span-3 flex w-full flex-col overflow-auto">
<ul className="divide-y divide-gray-700">
{contentList && contentList.length > 0 ? (
contentList.map((item, index) => (
<SidebarItem key={index} item={item} />
))
) : (
<li className="text-center text-gray-500">No content available</li>
)}
</ul>
</div>
);

interface SidebarItemProps {
item: (typeof mockContentList)[0];
}

const SidebarItem = ({ item }: SidebarItemProps) => (
<li className="flex flex-row items-center justify-between px-2 py-2">
<div className="flex flex-row items-center">
<SquarePlay
className="mr-4 h-5 w-5 text-white group-hover:text-white"
aria-hidden="true"
/>
<p>{item.title}</p>
</div>
<CircleCheck
className={`h-6 w-6 ${
item.watched ? "mr-2 h-5 w-5 text-pink-600" : "mr-2 h-5 w-5 text-white"
}`}
aria-hidden="true"
/>
</li>
);
Comment on lines +44 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve accessibility and type definitions

The sidebar implementation needs accessibility enhancements and better type definitions:

  1. Replace mock data type with proper interface:
interfaceContentItem{id: string;title: string;watched: boolean;// ... other properties}interfaceSidebarProps{contentList: ContentItem[];}
  1. Enhance accessibility:
-<ul className="divide-y divide-gray-700">+<ul + className="divide-y divide-gray-700"+ role="list"+ aria-label="Course content"+>-<li className="flex flex-row items-center justify-between px-2 py-2">+<li + className="flex flex-row items-center justify-between px-2 py-2"+ tabIndex={0}+ role="listitem"+>


interface ContentListProps {
contentList: typeof mockContentList;
}

const ContentList = ({ contentList }: ContentListProps) => {
return (
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<li key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
Comment on lines +88 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML structure and accessibility issues

The ContentList component has several structural and accessibility issues:

Apply these fixes:

 const ContentList = ({ contentList }: ContentListProps) => {
return (
- <>+ <div className="content-list" role="region" aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
- <li key={author} className="flex justify-between gap-x-6 py-5">+ <div key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
- alt=""+ alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
- </li>+ </div>- <div>{description}</div>- <div>{longDescription}</div>+ <div role="article">+ <h3 className="sr-only">Course description</h3>+ <p>{description}</p>+ <p>{longDescription}</p>+ </div>
</div>
),
)}
- </>+ </div>
);
};
📝 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.

Suggested change
constContentList=({ contentList }: ContentListProps)=>{
return(
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<likey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
constContentList=({ contentList }: ContentListProps)=>{
return(
<divclassName="content-list"role="region"aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<divkey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</div>
<divrole="article">
<h3className="sr-only">Course description</h3>
<p>{description}</p>
<p>{longDescription}</p>
</div>
</div>
),
)}
</div>
);
};


export default Content;
13 changes: 13 additions & 0 deletions app/(app)/courses/[slug]/[id]/page.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
import { getServerAuthSession } from "@/server/auth";
import Content from "./_client";
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding loading and error states.

The page lacks loading and error handling components which are important for better UX.

Add loading.tsx and error.tsx files in the same directory:

// loading.tsxexportdefaultfunctionLoading(){return<div>Loadingcoursecontent...</div>;}// error.tsxexportdefaultfunctionError({
error,
reset,}: {error: Error;reset: ()=>void;}){return(<div><h2>Somethingwentwrong!</h2><buttononClick={reset}>Tryagain</button></div>);}


export const metadata = {
title: "Video Course",
};
Comment on lines +4 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance metadata with dynamic course information.

The current metadata title is static. Consider utilizing the route parameters to display the actual course title.

export const metadata = {
- title: "Video Course",+ title: async function generateTitle({ params }) {+ const { slug } = params;+ return `${slug.replace('-', ' ')} | Video Course`;+ },
};

Committable suggestion was skipped due to low confidence.


export default async function Page() {
// Example of grabbing session in case it is needed
const session = await getServerAuthSession();

return <Content session={session} />;
}
Comment on lines +8 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize route parameters and add error handling.

The page component doesn't use the URL parameters ([slug] and [id]) which are essential for fetching the correct course content.

Consider implementing proper error handling and course data fetching:

export default async function Page({
+ params,+ }: {+ params: { slug: string; id: string };
}) {
const session = await getServerAuthSession();
++ // Fetch course data using params.slug and params.id+ const course = await getCourse(params.slug, params.id);++ if (!course) {+ notFound();+ }- return <Content session={session} />;+ return <Content session={session} course={course} />;
}

Committable suggestion was skipped due to low confidence.

130 changes: 130 additions & 0 deletions app/(app)/courses/mock.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
// Mock video source (this would typically be a path to a video file)
const mockVideoSrc = "https://youtu.be/Ln4KSN0rchI?t=12";

// Mock content list
const mockContentList = [
{
id: 1,
title: "Introduction to React",
longDescription:
"In this video, we will explore the fundamentals of React, including its component-based architecture, virtual DOM, and how to get started with your first React application. This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description: "Learn the basics of React and its core concepts",
watched: false,
publishedDate: "2023-01-15",
author: "John Doe",
imageLink: "https://example.com/images/react-introduction.jpg",
},
{
id: 2,
title: "Setting Up Your Development Environment",
longDescription:
"This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description:
"Install and configure the necessary tools for React development",
watched: true,
publishedDate: "2023-01-22",
author: "Jane Smith",
},
{
id: 3,
title: "Components and Props",
longDescription:
"Dive deep into React's core concepts of components and props. Learn how to create reusable components and pass data between them using props.",
description: "Understand the building blocks of React applications",
watched: true,
publishedDate: "2023-01-29",
author: "John Doe",
},
{
id: 4,
title: "State and Lifecycle",
longDescription:
"Explore how to manage component state and lifecycle methods to build dynamic and interactive applications with React.",
description: "Manage component state and lifecycle methods",
watched: true,
publishedDate: "2023-02-05",
author: "Jane Smith",
},
{
id: 5,
title: "Handling Events",
longDescription:
"Learn how to handle user interactions in React, including events like clicks, form submissions, and more.",
description: "Learn how to handle user interactions in React",
watched: true,
publishedDate: "2023-02-12",
author: "John Doe",
},
{
id: 6,
title: "Conditional Rendering",
longDescription:
"Discover how to display different UI elements based on specific conditions in your React applications.",
description: "Display different UI based on conditions",
watched: false,
publishedDate: "2023-02-19",
author: "Jane Smith",
},
{
id: 7,
title: "Lists and Keys",
longDescription:
"Learn the best practices for rendering lists of components in React and how to efficiently manage them with unique keys.",
description: "Render multiple components efficiently",
watched: false,
publishedDate: "2023-02-26",
author: "John Doe",
},
{
id: 8,
title: "Forms in React",
longDescription:
"This video covers how to create and manage forms in React applications, including controlled and uncontrolled components.",
description: "Create and handle form inputs in React applications",
watched: false,
publishedDate: "2023-03-05",
author: "Jane Smith",
},
{
id: 9,
title: "Hooks: useState and useEffect",
longDescription:
"An introduction to React Hooks, focusing on useState and useEffect to manage state and side effects in functional components.",
description: "Understand and use basic React Hooks",
watched: false,
publishedDate: "2023-03-12",
author: "John Doe",
},
{
id: 10,
title: "Custom Hooks",
longDescription:
"Learn how to create custom hooks in React to encapsulate and reuse logic across components.",
description: "Create reusable logic with custom Hooks",
watched: false,
publishedDate: "2023-03-19",
author: "Jane Smith",
},
{
id: 11,
title: "Context API",
longDescription:
"Explore the Context API to manage global state in your React applications without the need for prop drilling.",
description: "Manage global state in your React application",
watched: false,
publishedDate: "2023-03-26",
author: "John Doe",
},
{
id: 12,
title: "React Router",
longDescription:
"Learn how to implement navigation in your single-page applications using React Router, including dynamic routing and nested routes.",
description: "Implement navigation in your single-page application",
watched: false,
publishedDate: "2023-04-02",
author: "Jane Smith",
},
];

export { mockVideoSrc, mockContentList };
1 change: 1 addition & 0 deletions utils/flags.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
COURSE_VIDEO: "course-video",
JOBS: "jobs",
// Add more feature flags as needed
} as const;
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
131 changes: 131 additions & 0 deletions app/(app)/courses/[slug]/[id]/_client.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
"use client";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { CircleCheck, SquarePlay } from "lucide-react";
import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { mockContentList } from "../../mock";

interface ContentProps {
session: Session | null;
}
Comment on lines +8 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove unused session prop

The session prop is defined in the interface but is not used anywhere within the Content component implementation. The component only uses feature flag checks and renders child components without any session-dependent functionality.

  • Remove the session prop from the ContentProps interface
  • Remove the session parameter from the Content component definition
🔗 Analysis chain

Verify usage of session prop

The session prop is defined in the interface but doesn't appear to be used within the Content component. Either remove the unused prop or implement the intended session-dependent functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash# Search for session usage in the component
ast-grep --pattern 'session.$$$'

Length of output: 10367


Script:

#!/bin/bash# Let's search for the Content component implementation and session usage
ast-grep --pattern 'const Content = ({ session }: ContentProps) => { $$$}'

Length of output: 1434


const Content = ({ session }: ContentProps) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);

if (!flagEnabled) {
notFound();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible stupid question on this one Carolina. Can you put featureflag the route? Then you can just let nexts router deal with the notFound ?

Reason this might be a stupid queston is I have no idea how nexts router works 🤣 I am basing this comment from my knowledge of react router

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we could but that's a good pattern that we could have. We will figure it out


return (
<div className="overflow-auto">
<div className="w-full divide-x divide-gray-700 lg:grid lg:grid-cols-12">
<VideoPlayer />
<Sidebar contentList={mockContentList} />
</div>
<div className="mx-auto max-w-5xl">
<ContentList contentList={mockContentList} />
</div>
</div>
);
};
Comment on lines +12 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error boundaries and loading states

The component should handle potential errors from mock data and add loading states for better UX. Consider implementing:

  1. Error boundaries to catch rendering errors
  2. Loading states while checking feature flags
  3. Proper error handling for missing or malformed mock data

Here's a suggested implementation:

 const Content = ({ session }: ContentProps) => {
+ const [isLoading, setIsLoading] = useState(true);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);
if (!flagEnabled) {
notFound();
}
+ if (isLoading) {+ return <div>Loading...</div>;+ }++ if (!mockContentList?.length) {+ return <div>No content available</div>;+ }+
return (
<div className="overflow-auto">

Committable suggestion was skipped due to low confidence.


const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
Comment on lines +32 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance iframe security and performance

The YouTube iframe implementation needs security and performance improvements:

Apply these security and performance enhancements:

 const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
+ loading="lazy"+ sandbox="allow-scripts allow-same-origin allow-presentation"+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"+ referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);

Also consider:

  1. Making the video URL configurable through props
  2. Adding error handling for failed video loads
  3. Implementing a loading placeholder
📝 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.

Suggested change
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
loading="lazy"
sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);


interface SidebarProps {
contentList: typeof mockContentList;
}

const Sidebar = ({ contentList }: SidebarProps) => (
<div className="col-span-3 flex w-full flex-col overflow-auto">
<ul className="divide-y divide-gray-700">
{contentList && contentList.length > 0 ? (
contentList.map((item, index) => (
<SidebarItem key={index} item={item} />
))
) : (
<li className="text-center text-gray-500">No content available</li>
)}
</ul>
</div>
);

interface SidebarItemProps {
item: (typeof mockContentList)[0];
}

const SidebarItem = ({ item }: SidebarItemProps) => (
<li className="flex flex-row items-center justify-between px-2 py-2">
<div className="flex flex-row items-center">
<SquarePlay
className="mr-4 h-5 w-5 text-white group-hover:text-white"
aria-hidden="true"
/>
<p>{item.title}</p>
</div>
<CircleCheck
className={`h-6 w-6 ${
item.watched ? "mr-2 h-5 w-5 text-pink-600" : "mr-2 h-5 w-5 text-white"
}`}
aria-hidden="true"
/>
</li>
);
Comment on lines +44 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve accessibility and type definitions

The sidebar implementation needs accessibility enhancements and better type definitions:

  1. Replace mock data type with proper interface:
interfaceContentItem{id: string;title: string;watched: boolean;// ... other properties}interfaceSidebarProps{contentList: ContentItem[];}
  1. Enhance accessibility:
-<ul className="divide-y divide-gray-700">+<ul + className="divide-y divide-gray-700"+ role="list"+ aria-label="Course content"+>-<li className="flex flex-row items-center justify-between px-2 py-2">+<li + className="flex flex-row items-center justify-between px-2 py-2"+ tabIndex={0}+ role="listitem"+>


interface ContentListProps {
contentList: typeof mockContentList;
}

const ContentList = ({ contentList }: ContentListProps) => {
return (
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<li key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
Comment on lines +88 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML structure and accessibility issues

The ContentList component has several structural and accessibility issues:

Apply these fixes:

 const ContentList = ({ contentList }: ContentListProps) => {
return (
- <>+ <div className="content-list" role="region" aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
- <li key={author} className="flex justify-between gap-x-6 py-5">+ <div key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
- alt=""+ alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
- </li>+ </div>- <div>{description}</div>- <div>{longDescription}</div>+ <div role="article">+ <h3 className="sr-only">Course description</h3>+ <p>{description}</p>+ <p>{longDescription}</p>+ </div>
</div>
),
)}
- </>+ </div>
);
};
📝 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.

Suggested change
constContentList=({ contentList }: ContentListProps)=>{
return(
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<likey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
constContentList=({ contentList }: ContentListProps)=>{
return(
<divclassName="content-list"role="region"aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<divkey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</div>
<divrole="article">
<h3className="sr-only">Course description</h3>
<p>{description}</p>
<p>{longDescription}</p>
</div>
</div>
),
)}
</div>
);
};


export default Content;
13 changes: 13 additions & 0 deletions app/(app)/courses/[slug]/[id]/page.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
import { getServerAuthSession } from "@/server/auth";
import Content from "./_client";
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding loading and error states.

The page lacks loading and error handling components which are important for better UX.

Add loading.tsx and error.tsx files in the same directory:

// loading.tsxexportdefaultfunctionLoading(){return<div>Loadingcoursecontent...</div>;}// error.tsxexportdefaultfunctionError({
error,
reset,}: {error: Error;reset: ()=>void;}){return(<div><h2>Somethingwentwrong!</h2><buttononClick={reset}>Tryagain</button></div>);}


export const metadata = {
title: "Video Course",
};
Comment on lines +4 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance metadata with dynamic course information.

The current metadata title is static. Consider utilizing the route parameters to display the actual course title.

export const metadata = {
- title: "Video Course",+ title: async function generateTitle({ params }) {+ const { slug } = params;+ return `${slug.replace('-', ' ')} | Video Course`;+ },
};

Committable suggestion was skipped due to low confidence.


export default async function Page() {
// Example of grabbing session in case it is needed
const session = await getServerAuthSession();

return <Content session={session} />;
}
Comment on lines +8 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize route parameters and add error handling.

The page component doesn't use the URL parameters ([slug] and [id]) which are essential for fetching the correct course content.

Consider implementing proper error handling and course data fetching:

export default async function Page({
+ params,+ }: {+ params: { slug: string; id: string };
}) {
const session = await getServerAuthSession();
++ // Fetch course data using params.slug and params.id+ const course = await getCourse(params.slug, params.id);++ if (!course) {+ notFound();+ }- return <Content session={session} />;+ return <Content session={session} course={course} />;
}

Committable suggestion was skipped due to low confidence.

130 changes: 130 additions & 0 deletions app/(app)/courses/mock.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
// Mock video source (this would typically be a path to a video file)
const mockVideoSrc = "https://youtu.be/Ln4KSN0rchI?t=12";

// Mock content list
const mockContentList = [
{
id: 1,
title: "Introduction to React",
longDescription:
"In this video, we will explore the fundamentals of React, including its component-based architecture, virtual DOM, and how to get started with your first React application. This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description: "Learn the basics of React and its core concepts",
watched: false,
publishedDate: "2023-01-15",
author: "John Doe",
imageLink: "https://example.com/images/react-introduction.jpg",
},
{
id: 2,
title: "Setting Up Your Development Environment",
longDescription:
"This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description:
"Install and configure the necessary tools for React development",
watched: true,
publishedDate: "2023-01-22",
author: "Jane Smith",
},
{
id: 3,
title: "Components and Props",
longDescription:
"Dive deep into React's core concepts of components and props. Learn how to create reusable components and pass data between them using props.",
description: "Understand the building blocks of React applications",
watched: true,
publishedDate: "2023-01-29",
author: "John Doe",
},
{
id: 4,
title: "State and Lifecycle",
longDescription:
"Explore how to manage component state and lifecycle methods to build dynamic and interactive applications with React.",
description: "Manage component state and lifecycle methods",
watched: true,
publishedDate: "2023-02-05",
author: "Jane Smith",
},
{
id: 5,
title: "Handling Events",
longDescription:
"Learn how to handle user interactions in React, including events like clicks, form submissions, and more.",
description: "Learn how to handle user interactions in React",
watched: true,
publishedDate: "2023-02-12",
author: "John Doe",
},
{
id: 6,
title: "Conditional Rendering",
longDescription:
"Discover how to display different UI elements based on specific conditions in your React applications.",
description: "Display different UI based on conditions",
watched: false,
publishedDate: "2023-02-19",
author: "Jane Smith",
},
{
id: 7,
title: "Lists and Keys",
longDescription:
"Learn the best practices for rendering lists of components in React and how to efficiently manage them with unique keys.",
description: "Render multiple components efficiently",
watched: false,
publishedDate: "2023-02-26",
author: "John Doe",
},
{
id: 8,
title: "Forms in React",
longDescription:
"This video covers how to create and manage forms in React applications, including controlled and uncontrolled components.",
description: "Create and handle form inputs in React applications",
watched: false,
publishedDate: "2023-03-05",
author: "Jane Smith",
},
{
id: 9,
title: "Hooks: useState and useEffect",
longDescription:
"An introduction to React Hooks, focusing on useState and useEffect to manage state and side effects in functional components.",
description: "Understand and use basic React Hooks",
watched: false,
publishedDate: "2023-03-12",
author: "John Doe",
},
{
id: 10,
title: "Custom Hooks",
longDescription:
"Learn how to create custom hooks in React to encapsulate and reuse logic across components.",
description: "Create reusable logic with custom Hooks",
watched: false,
publishedDate: "2023-03-19",
author: "Jane Smith",
},
{
id: 11,
title: "Context API",
longDescription:
"Explore the Context API to manage global state in your React applications without the need for prop drilling.",
description: "Manage global state in your React application",
watched: false,
publishedDate: "2023-03-26",
author: "John Doe",
},
{
id: 12,
title: "React Router",
longDescription:
"Learn how to implement navigation in your single-page applications using React Router, including dynamic routing and nested routes.",
description: "Implement navigation in your single-page application",
watched: false,
publishedDate: "2023-04-02",
author: "Jane Smith",
},
];

export { mockVideoSrc, mockContentList };
1 change: 1 addition & 0 deletions utils/flags.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
COURSE_VIDEO: "course-video",
JOBS: "jobs",
// Add more feature flags as needed
} as const;
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
131 changes: 131 additions & 0 deletions app/(app)/courses/[slug]/[id]/_client.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
"use client";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { CircleCheck, SquarePlay } from "lucide-react";
import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { mockContentList } from "../../mock";

interface ContentProps {
session: Session | null;
}
Comment on lines +8 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove unused session prop

The session prop is defined in the interface but is not used anywhere within the Content component implementation. The component only uses feature flag checks and renders child components without any session-dependent functionality.

  • Remove the session prop from the ContentProps interface
  • Remove the session parameter from the Content component definition
🔗 Analysis chain

Verify usage of session prop

The session prop is defined in the interface but doesn't appear to be used within the Content component. Either remove the unused prop or implement the intended session-dependent functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash# Search for session usage in the component
ast-grep --pattern 'session.$$$'

Length of output: 10367


Script:

#!/bin/bash# Let's search for the Content component implementation and session usage
ast-grep --pattern 'const Content = ({ session }: ContentProps) => { $$$}'

Length of output: 1434


const Content = ({ session }: ContentProps) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);

if (!flagEnabled) {
notFound();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible stupid question on this one Carolina. Can you put featureflag the route? Then you can just let nexts router deal with the notFound ?

Reason this might be a stupid queston is I have no idea how nexts router works 🤣 I am basing this comment from my knowledge of react router

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we could but that's a good pattern that we could have. We will figure it out


return (
<div className="overflow-auto">
<div className="w-full divide-x divide-gray-700 lg:grid lg:grid-cols-12">
<VideoPlayer />
<Sidebar contentList={mockContentList} />
</div>
<div className="mx-auto max-w-5xl">
<ContentList contentList={mockContentList} />
</div>
</div>
);
};
Comment on lines +12 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error boundaries and loading states

The component should handle potential errors from mock data and add loading states for better UX. Consider implementing:

  1. Error boundaries to catch rendering errors
  2. Loading states while checking feature flags
  3. Proper error handling for missing or malformed mock data

Here's a suggested implementation:

 const Content = ({ session }: ContentProps) => {
+ const [isLoading, setIsLoading] = useState(true);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);
if (!flagEnabled) {
notFound();
}
+ if (isLoading) {+ return <div>Loading...</div>;+ }++ if (!mockContentList?.length) {+ return <div>No content available</div>;+ }+
return (
<div className="overflow-auto">

Committable suggestion was skipped due to low confidence.


const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
Comment on lines +32 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance iframe security and performance

The YouTube iframe implementation needs security and performance improvements:

Apply these security and performance enhancements:

 const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
+ loading="lazy"+ sandbox="allow-scripts allow-same-origin allow-presentation"+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"+ referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);

Also consider:

  1. Making the video URL configurable through props
  2. Adding error handling for failed video loads
  3. Implementing a loading placeholder
📝 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.

Suggested change
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
loading="lazy"
sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);


interface SidebarProps {
contentList: typeof mockContentList;
}

const Sidebar = ({ contentList }: SidebarProps) => (
<div className="col-span-3 flex w-full flex-col overflow-auto">
<ul className="divide-y divide-gray-700">
{contentList && contentList.length > 0 ? (
contentList.map((item, index) => (
<SidebarItem key={index} item={item} />
))
) : (
<li className="text-center text-gray-500">No content available</li>
)}
</ul>
</div>
);

interface SidebarItemProps {
item: (typeof mockContentList)[0];
}

const SidebarItem = ({ item }: SidebarItemProps) => (
<li className="flex flex-row items-center justify-between px-2 py-2">
<div className="flex flex-row items-center">
<SquarePlay
className="mr-4 h-5 w-5 text-white group-hover:text-white"
aria-hidden="true"
/>
<p>{item.title}</p>
</div>
<CircleCheck
className={`h-6 w-6 ${
item.watched ? "mr-2 h-5 w-5 text-pink-600" : "mr-2 h-5 w-5 text-white"
}`}
aria-hidden="true"
/>
</li>
);
Comment on lines +44 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve accessibility and type definitions

The sidebar implementation needs accessibility enhancements and better type definitions:

  1. Replace mock data type with proper interface:
interfaceContentItem{id: string;title: string;watched: boolean;// ... other properties}interfaceSidebarProps{contentList: ContentItem[];}
  1. Enhance accessibility:
-<ul className="divide-y divide-gray-700">+<ul + className="divide-y divide-gray-700"+ role="list"+ aria-label="Course content"+>-<li className="flex flex-row items-center justify-between px-2 py-2">+<li + className="flex flex-row items-center justify-between px-2 py-2"+ tabIndex={0}+ role="listitem"+>


interface ContentListProps {
contentList: typeof mockContentList;
}

const ContentList = ({ contentList }: ContentListProps) => {
return (
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<li key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
Comment on lines +88 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML structure and accessibility issues

The ContentList component has several structural and accessibility issues:

Apply these fixes:

 const ContentList = ({ contentList }: ContentListProps) => {
return (
- <>+ <div className="content-list" role="region" aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
- <li key={author} className="flex justify-between gap-x-6 py-5">+ <div key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
- alt=""+ alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
- </li>+ </div>- <div>{description}</div>- <div>{longDescription}</div>+ <div role="article">+ <h3 className="sr-only">Course description</h3>+ <p>{description}</p>+ <p>{longDescription}</p>+ </div>
</div>
),
)}
- </>+ </div>
);
};
📝 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.

Suggested change
constContentList=({ contentList }: ContentListProps)=>{
return(
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<likey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
constContentList=({ contentList }: ContentListProps)=>{
return(
<divclassName="content-list"role="region"aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<divkey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</div>
<divrole="article">
<h3className="sr-only">Course description</h3>
<p>{description}</p>
<p>{longDescription}</p>
</div>
</div>
),
)}
</div>
);
};


export default Content;
13 changes: 13 additions & 0 deletions app/(app)/courses/[slug]/[id]/page.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
import { getServerAuthSession } from "@/server/auth";
import Content from "./_client";
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding loading and error states.

The page lacks loading and error handling components which are important for better UX.

Add loading.tsx and error.tsx files in the same directory:

// loading.tsxexportdefaultfunctionLoading(){return<div>Loadingcoursecontent...</div>;}// error.tsxexportdefaultfunctionError({
error,
reset,}: {error: Error;reset: ()=>void;}){return(<div><h2>Somethingwentwrong!</h2><buttononClick={reset}>Tryagain</button></div>);}


export const metadata = {
title: "Video Course",
};
Comment on lines +4 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance metadata with dynamic course information.

The current metadata title is static. Consider utilizing the route parameters to display the actual course title.

export const metadata = {
- title: "Video Course",+ title: async function generateTitle({ params }) {+ const { slug } = params;+ return `${slug.replace('-', ' ')} | Video Course`;+ },
};

Committable suggestion was skipped due to low confidence.


export default async function Page() {
// Example of grabbing session in case it is needed
const session = await getServerAuthSession();

return <Content session={session} />;
}
Comment on lines +8 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize route parameters and add error handling.

The page component doesn't use the URL parameters ([slug] and [id]) which are essential for fetching the correct course content.

Consider implementing proper error handling and course data fetching:

export default async function Page({
+ params,+ }: {+ params: { slug: string; id: string };
}) {
const session = await getServerAuthSession();
++ // Fetch course data using params.slug and params.id+ const course = await getCourse(params.slug, params.id);++ if (!course) {+ notFound();+ }- return <Content session={session} />;+ return <Content session={session} course={course} />;
}

Committable suggestion was skipped due to low confidence.

130 changes: 130 additions & 0 deletions app/(app)/courses/mock.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
// Mock video source (this would typically be a path to a video file)
const mockVideoSrc = "https://youtu.be/Ln4KSN0rchI?t=12";

// Mock content list
const mockContentList = [
{
id: 1,
title: "Introduction to React",
longDescription:
"In this video, we will explore the fundamentals of React, including its component-based architecture, virtual DOM, and how to get started with your first React application. This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description: "Learn the basics of React and its core concepts",
watched: false,
publishedDate: "2023-01-15",
author: "John Doe",
imageLink: "https://example.com/images/react-introduction.jpg",
},
{
id: 2,
title: "Setting Up Your Development Environment",
longDescription:
"This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description:
"Install and configure the necessary tools for React development",
watched: true,
publishedDate: "2023-01-22",
author: "Jane Smith",
},
{
id: 3,
title: "Components and Props",
longDescription:
"Dive deep into React's core concepts of components and props. Learn how to create reusable components and pass data between them using props.",
description: "Understand the building blocks of React applications",
watched: true,
publishedDate: "2023-01-29",
author: "John Doe",
},
{
id: 4,
title: "State and Lifecycle",
longDescription:
"Explore how to manage component state and lifecycle methods to build dynamic and interactive applications with React.",
description: "Manage component state and lifecycle methods",
watched: true,
publishedDate: "2023-02-05",
author: "Jane Smith",
},
{
id: 5,
title: "Handling Events",
longDescription:
"Learn how to handle user interactions in React, including events like clicks, form submissions, and more.",
description: "Learn how to handle user interactions in React",
watched: true,
publishedDate: "2023-02-12",
author: "John Doe",
},
{
id: 6,
title: "Conditional Rendering",
longDescription:
"Discover how to display different UI elements based on specific conditions in your React applications.",
description: "Display different UI based on conditions",
watched: false,
publishedDate: "2023-02-19",
author: "Jane Smith",
},
{
id: 7,
title: "Lists and Keys",
longDescription:
"Learn the best practices for rendering lists of components in React and how to efficiently manage them with unique keys.",
description: "Render multiple components efficiently",
watched: false,
publishedDate: "2023-02-26",
author: "John Doe",
},
{
id: 8,
title: "Forms in React",
longDescription:
"This video covers how to create and manage forms in React applications, including controlled and uncontrolled components.",
description: "Create and handle form inputs in React applications",
watched: false,
publishedDate: "2023-03-05",
author: "Jane Smith",
},
{
id: 9,
title: "Hooks: useState and useEffect",
longDescription:
"An introduction to React Hooks, focusing on useState and useEffect to manage state and side effects in functional components.",
description: "Understand and use basic React Hooks",
watched: false,
publishedDate: "2023-03-12",
author: "John Doe",
},
{
id: 10,
title: "Custom Hooks",
longDescription:
"Learn how to create custom hooks in React to encapsulate and reuse logic across components.",
description: "Create reusable logic with custom Hooks",
watched: false,
publishedDate: "2023-03-19",
author: "Jane Smith",
},
{
id: 11,
title: "Context API",
longDescription:
"Explore the Context API to manage global state in your React applications without the need for prop drilling.",
description: "Manage global state in your React application",
watched: false,
publishedDate: "2023-03-26",
author: "John Doe",
},
{
id: 12,
title: "React Router",
longDescription:
"Learn how to implement navigation in your single-page applications using React Router, including dynamic routing and nested routes.",
description: "Implement navigation in your single-page application",
watched: false,
publishedDate: "2023-04-02",
author: "Jane Smith",
},
];

export { mockVideoSrc, mockContentList };
1 change: 1 addition & 0 deletions utils/flags.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
COURSE_VIDEO: "course-video",
JOBS: "jobs",
// Add more feature flags as needed
} as const;
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
131 changes: 131 additions & 0 deletions app/(app)/courses/[slug]/[id]/_client.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
"use client";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { CircleCheck, SquarePlay } from "lucide-react";
import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { mockContentList } from "../../mock";

interface ContentProps {
session: Session | null;
}
Comment on lines +8 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove unused session prop

The session prop is defined in the interface but is not used anywhere within the Content component implementation. The component only uses feature flag checks and renders child components without any session-dependent functionality.

  • Remove the session prop from the ContentProps interface
  • Remove the session parameter from the Content component definition
🔗 Analysis chain

Verify usage of session prop

The session prop is defined in the interface but doesn't appear to be used within the Content component. Either remove the unused prop or implement the intended session-dependent functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash# Search for session usage in the component
ast-grep --pattern 'session.$$$'

Length of output: 10367


Script:

#!/bin/bash# Let's search for the Content component implementation and session usage
ast-grep --pattern 'const Content = ({ session }: ContentProps) => { $$$}'

Length of output: 1434


const Content = ({ session }: ContentProps) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);

if (!flagEnabled) {
notFound();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible stupid question on this one Carolina. Can you put featureflag the route? Then you can just let nexts router deal with the notFound ?

Reason this might be a stupid queston is I have no idea how nexts router works 🤣 I am basing this comment from my knowledge of react router

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we could but that's a good pattern that we could have. We will figure it out


return (
<div className="overflow-auto">
<div className="w-full divide-x divide-gray-700 lg:grid lg:grid-cols-12">
<VideoPlayer />
<Sidebar contentList={mockContentList} />
</div>
<div className="mx-auto max-w-5xl">
<ContentList contentList={mockContentList} />
</div>
</div>
);
};
Comment on lines +12 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error boundaries and loading states

The component should handle potential errors from mock data and add loading states for better UX. Consider implementing:

  1. Error boundaries to catch rendering errors
  2. Loading states while checking feature flags
  3. Proper error handling for missing or malformed mock data

Here's a suggested implementation:

 const Content = ({ session }: ContentProps) => {
+ const [isLoading, setIsLoading] = useState(true);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);
if (!flagEnabled) {
notFound();
}
+ if (isLoading) {+ return <div>Loading...</div>;+ }++ if (!mockContentList?.length) {+ return <div>No content available</div>;+ }+
return (
<div className="overflow-auto">

Committable suggestion was skipped due to low confidence.


const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
Comment on lines +32 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance iframe security and performance

The YouTube iframe implementation needs security and performance improvements:

Apply these security and performance enhancements:

 const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
+ loading="lazy"+ sandbox="allow-scripts allow-same-origin allow-presentation"+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"+ referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);

Also consider:

  1. Making the video URL configurable through props
  2. Adding error handling for failed video loads
  3. Implementing a loading placeholder
📝 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.

Suggested change
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
loading="lazy"
sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);


interface SidebarProps {
contentList: typeof mockContentList;
}

const Sidebar = ({ contentList }: SidebarProps) => (
<div className="col-span-3 flex w-full flex-col overflow-auto">
<ul className="divide-y divide-gray-700">
{contentList && contentList.length > 0 ? (
contentList.map((item, index) => (
<SidebarItem key={index} item={item} />
))
) : (
<li className="text-center text-gray-500">No content available</li>
)}
</ul>
</div>
);

interface SidebarItemProps {
item: (typeof mockContentList)[0];
}

const SidebarItem = ({ item }: SidebarItemProps) => (
<li className="flex flex-row items-center justify-between px-2 py-2">
<div className="flex flex-row items-center">
<SquarePlay
className="mr-4 h-5 w-5 text-white group-hover:text-white"
aria-hidden="true"
/>
<p>{item.title}</p>
</div>
<CircleCheck
className={`h-6 w-6 ${
item.watched ? "mr-2 h-5 w-5 text-pink-600" : "mr-2 h-5 w-5 text-white"
}`}
aria-hidden="true"
/>
</li>
);
Comment on lines +44 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve accessibility and type definitions

The sidebar implementation needs accessibility enhancements and better type definitions:

  1. Replace mock data type with proper interface:
interfaceContentItem{id: string;title: string;watched: boolean;// ... other properties}interfaceSidebarProps{contentList: ContentItem[];}
  1. Enhance accessibility:
-<ul className="divide-y divide-gray-700">+<ul + className="divide-y divide-gray-700"+ role="list"+ aria-label="Course content"+>-<li className="flex flex-row items-center justify-between px-2 py-2">+<li + className="flex flex-row items-center justify-between px-2 py-2"+ tabIndex={0}+ role="listitem"+>


interface ContentListProps {
contentList: typeof mockContentList;
}

const ContentList = ({ contentList }: ContentListProps) => {
return (
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<li key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
Comment on lines +88 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML structure and accessibility issues

The ContentList component has several structural and accessibility issues:

Apply these fixes:

 const ContentList = ({ contentList }: ContentListProps) => {
return (
- <>+ <div className="content-list" role="region" aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
- <li key={author} className="flex justify-between gap-x-6 py-5">+ <div key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
- alt=""+ alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
- </li>+ </div>- <div>{description}</div>- <div>{longDescription}</div>+ <div role="article">+ <h3 className="sr-only">Course description</h3>+ <p>{description}</p>+ <p>{longDescription}</p>+ </div>
</div>
),
)}
- </>+ </div>
);
};
📝 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.

Suggested change
constContentList=({ contentList }: ContentListProps)=>{
return(
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<likey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
constContentList=({ contentList }: ContentListProps)=>{
return(
<divclassName="content-list"role="region"aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<divkey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</div>
<divrole="article">
<h3className="sr-only">Course description</h3>
<p>{description}</p>
<p>{longDescription}</p>
</div>
</div>
),
)}
</div>
);
};


export default Content;
13 changes: 13 additions & 0 deletions app/(app)/courses/[slug]/[id]/page.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
import { getServerAuthSession } from "@/server/auth";
import Content from "./_client";
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding loading and error states.

The page lacks loading and error handling components which are important for better UX.

Add loading.tsx and error.tsx files in the same directory:

// loading.tsxexportdefaultfunctionLoading(){return<div>Loadingcoursecontent...</div>;}// error.tsxexportdefaultfunctionError({
error,
reset,}: {error: Error;reset: ()=>void;}){return(<div><h2>Somethingwentwrong!</h2><buttononClick={reset}>Tryagain</button></div>);}


export const metadata = {
title: "Video Course",
};
Comment on lines +4 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance metadata with dynamic course information.

The current metadata title is static. Consider utilizing the route parameters to display the actual course title.

export const metadata = {
- title: "Video Course",+ title: async function generateTitle({ params }) {+ const { slug } = params;+ return `${slug.replace('-', ' ')} | Video Course`;+ },
};

Committable suggestion was skipped due to low confidence.


export default async function Page() {
// Example of grabbing session in case it is needed
const session = await getServerAuthSession();

return <Content session={session} />;
}
Comment on lines +8 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize route parameters and add error handling.

The page component doesn't use the URL parameters ([slug] and [id]) which are essential for fetching the correct course content.

Consider implementing proper error handling and course data fetching:

export default async function Page({
+ params,+ }: {+ params: { slug: string; id: string };
}) {
const session = await getServerAuthSession();
++ // Fetch course data using params.slug and params.id+ const course = await getCourse(params.slug, params.id);++ if (!course) {+ notFound();+ }- return <Content session={session} />;+ return <Content session={session} course={course} />;
}

Committable suggestion was skipped due to low confidence.

130 changes: 130 additions & 0 deletions app/(app)/courses/mock.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
// Mock video source (this would typically be a path to a video file)
const mockVideoSrc = "https://youtu.be/Ln4KSN0rchI?t=12";

// Mock content list
const mockContentList = [
{
id: 1,
title: "Introduction to React",
longDescription:
"In this video, we will explore the fundamentals of React, including its component-based architecture, virtual DOM, and how to get started with your first React application. This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description: "Learn the basics of React and its core concepts",
watched: false,
publishedDate: "2023-01-15",
author: "John Doe",
imageLink: "https://example.com/images/react-introduction.jpg",
},
{
id: 2,
title: "Setting Up Your Development Environment",
longDescription:
"This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description:
"Install and configure the necessary tools for React development",
watched: true,
publishedDate: "2023-01-22",
author: "Jane Smith",
},
{
id: 3,
title: "Components and Props",
longDescription:
"Dive deep into React's core concepts of components and props. Learn how to create reusable components and pass data between them using props.",
description: "Understand the building blocks of React applications",
watched: true,
publishedDate: "2023-01-29",
author: "John Doe",
},
{
id: 4,
title: "State and Lifecycle",
longDescription:
"Explore how to manage component state and lifecycle methods to build dynamic and interactive applications with React.",
description: "Manage component state and lifecycle methods",
watched: true,
publishedDate: "2023-02-05",
author: "Jane Smith",
},
{
id: 5,
title: "Handling Events",
longDescription:
"Learn how to handle user interactions in React, including events like clicks, form submissions, and more.",
description: "Learn how to handle user interactions in React",
watched: true,
publishedDate: "2023-02-12",
author: "John Doe",
},
{
id: 6,
title: "Conditional Rendering",
longDescription:
"Discover how to display different UI elements based on specific conditions in your React applications.",
description: "Display different UI based on conditions",
watched: false,
publishedDate: "2023-02-19",
author: "Jane Smith",
},
{
id: 7,
title: "Lists and Keys",
longDescription:
"Learn the best practices for rendering lists of components in React and how to efficiently manage them with unique keys.",
description: "Render multiple components efficiently",
watched: false,
publishedDate: "2023-02-26",
author: "John Doe",
},
{
id: 8,
title: "Forms in React",
longDescription:
"This video covers how to create and manage forms in React applications, including controlled and uncontrolled components.",
description: "Create and handle form inputs in React applications",
watched: false,
publishedDate: "2023-03-05",
author: "Jane Smith",
},
{
id: 9,
title: "Hooks: useState and useEffect",
longDescription:
"An introduction to React Hooks, focusing on useState and useEffect to manage state and side effects in functional components.",
description: "Understand and use basic React Hooks",
watched: false,
publishedDate: "2023-03-12",
author: "John Doe",
},
{
id: 10,
title: "Custom Hooks",
longDescription:
"Learn how to create custom hooks in React to encapsulate and reuse logic across components.",
description: "Create reusable logic with custom Hooks",
watched: false,
publishedDate: "2023-03-19",
author: "Jane Smith",
},
{
id: 11,
title: "Context API",
longDescription:
"Explore the Context API to manage global state in your React applications without the need for prop drilling.",
description: "Manage global state in your React application",
watched: false,
publishedDate: "2023-03-26",
author: "John Doe",
},
{
id: 12,
title: "React Router",
longDescription:
"Learn how to implement navigation in your single-page applications using React Router, including dynamic routing and nested routes.",
description: "Implement navigation in your single-page application",
watched: false,
publishedDate: "2023-04-02",
author: "Jane Smith",
},
];

export { mockVideoSrc, mockContentList };
1 change: 1 addition & 0 deletions utils/flags.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
COURSE_VIDEO: "course-video",
JOBS: "jobs",
// Add more feature flags as needed
} as const;
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
131 changes: 131 additions & 0 deletions app/(app)/courses/[slug]/[id]/_client.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
"use client";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { CircleCheck, SquarePlay } from "lucide-react";
import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { mockContentList } from "../../mock";

interface ContentProps {
session: Session | null;
}
Comment on lines +8 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove unused session prop

The session prop is defined in the interface but is not used anywhere within the Content component implementation. The component only uses feature flag checks and renders child components without any session-dependent functionality.

  • Remove the session prop from the ContentProps interface
  • Remove the session parameter from the Content component definition
🔗 Analysis chain

Verify usage of session prop

The session prop is defined in the interface but doesn't appear to be used within the Content component. Either remove the unused prop or implement the intended session-dependent functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash# Search for session usage in the component
ast-grep --pattern 'session.$$$'

Length of output: 10367


Script:

#!/bin/bash# Let's search for the Content component implementation and session usage
ast-grep --pattern 'const Content = ({ session }: ContentProps) => { $$$}'

Length of output: 1434


const Content = ({ session }: ContentProps) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);

if (!flagEnabled) {
notFound();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible stupid question on this one Carolina. Can you put featureflag the route? Then you can just let nexts router deal with the notFound ?

Reason this might be a stupid queston is I have no idea how nexts router works 🤣 I am basing this comment from my knowledge of react router

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we could but that's a good pattern that we could have. We will figure it out


return (
<div className="overflow-auto">
<div className="w-full divide-x divide-gray-700 lg:grid lg:grid-cols-12">
<VideoPlayer />
<Sidebar contentList={mockContentList} />
</div>
<div className="mx-auto max-w-5xl">
<ContentList contentList={mockContentList} />
</div>
</div>
);
};
Comment on lines +12 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error boundaries and loading states

The component should handle potential errors from mock data and add loading states for better UX. Consider implementing:

  1. Error boundaries to catch rendering errors
  2. Loading states while checking feature flags
  3. Proper error handling for missing or malformed mock data

Here's a suggested implementation:

 const Content = ({ session }: ContentProps) => {
+ const [isLoading, setIsLoading] = useState(true);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);
if (!flagEnabled) {
notFound();
}
+ if (isLoading) {+ return <div>Loading...</div>;+ }++ if (!mockContentList?.length) {+ return <div>No content available</div>;+ }+
return (
<div className="overflow-auto">

Committable suggestion was skipped due to low confidence.


const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
Comment on lines +32 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance iframe security and performance

The YouTube iframe implementation needs security and performance improvements:

Apply these security and performance enhancements:

 const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
+ loading="lazy"+ sandbox="allow-scripts allow-same-origin allow-presentation"+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"+ referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);

Also consider:

  1. Making the video URL configurable through props
  2. Adding error handling for failed video loads
  3. Implementing a loading placeholder
📝 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.

Suggested change
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
loading="lazy"
sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);


interface SidebarProps {
contentList: typeof mockContentList;
}

const Sidebar = ({ contentList }: SidebarProps) => (
<div className="col-span-3 flex w-full flex-col overflow-auto">
<ul className="divide-y divide-gray-700">
{contentList && contentList.length > 0 ? (
contentList.map((item, index) => (
<SidebarItem key={index} item={item} />
))
) : (
<li className="text-center text-gray-500">No content available</li>
)}
</ul>
</div>
);

interface SidebarItemProps {
item: (typeof mockContentList)[0];
}

const SidebarItem = ({ item }: SidebarItemProps) => (
<li className="flex flex-row items-center justify-between px-2 py-2">
<div className="flex flex-row items-center">
<SquarePlay
className="mr-4 h-5 w-5 text-white group-hover:text-white"
aria-hidden="true"
/>
<p>{item.title}</p>
</div>
<CircleCheck
className={`h-6 w-6 ${
item.watched ? "mr-2 h-5 w-5 text-pink-600" : "mr-2 h-5 w-5 text-white"
}`}
aria-hidden="true"
/>
</li>
);
Comment on lines +44 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve accessibility and type definitions

The sidebar implementation needs accessibility enhancements and better type definitions:

  1. Replace mock data type with proper interface:
interfaceContentItem{id: string;title: string;watched: boolean;// ... other properties}interfaceSidebarProps{contentList: ContentItem[];}
  1. Enhance accessibility:
-<ul className="divide-y divide-gray-700">+<ul + className="divide-y divide-gray-700"+ role="list"+ aria-label="Course content"+>-<li className="flex flex-row items-center justify-between px-2 py-2">+<li + className="flex flex-row items-center justify-between px-2 py-2"+ tabIndex={0}+ role="listitem"+>


interface ContentListProps {
contentList: typeof mockContentList;
}

const ContentList = ({ contentList }: ContentListProps) => {
return (
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<li key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
Comment on lines +88 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML structure and accessibility issues

The ContentList component has several structural and accessibility issues:

Apply these fixes:

 const ContentList = ({ contentList }: ContentListProps) => {
return (
- <>+ <div className="content-list" role="region" aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
- <li key={author} className="flex justify-between gap-x-6 py-5">+ <div key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
- alt=""+ alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
- </li>+ </div>- <div>{description}</div>- <div>{longDescription}</div>+ <div role="article">+ <h3 className="sr-only">Course description</h3>+ <p>{description}</p>+ <p>{longDescription}</p>+ </div>
</div>
),
)}
- </>+ </div>
);
};
📝 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.

Suggested change
constContentList=({ contentList }: ContentListProps)=>{
return(
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<likey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
constContentList=({ contentList }: ContentListProps)=>{
return(
<divclassName="content-list"role="region"aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<divkey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</div>
<divrole="article">
<h3className="sr-only">Course description</h3>
<p>{description}</p>
<p>{longDescription}</p>
</div>
</div>
),
)}
</div>
);
};


export default Content;
13 changes: 13 additions & 0 deletions app/(app)/courses/[slug]/[id]/page.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
import { getServerAuthSession } from "@/server/auth";
import Content from "./_client";
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding loading and error states.

The page lacks loading and error handling components which are important for better UX.

Add loading.tsx and error.tsx files in the same directory:

// loading.tsxexportdefaultfunctionLoading(){return<div>Loadingcoursecontent...</div>;}// error.tsxexportdefaultfunctionError({
error,
reset,}: {error: Error;reset: ()=>void;}){return(<div><h2>Somethingwentwrong!</h2><buttononClick={reset}>Tryagain</button></div>);}


export const metadata = {
title: "Video Course",
};
Comment on lines +4 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance metadata with dynamic course information.

The current metadata title is static. Consider utilizing the route parameters to display the actual course title.

export const metadata = {
- title: "Video Course",+ title: async function generateTitle({ params }) {+ const { slug } = params;+ return `${slug.replace('-', ' ')} | Video Course`;+ },
};

Committable suggestion was skipped due to low confidence.


export default async function Page() {
// Example of grabbing session in case it is needed
const session = await getServerAuthSession();

return <Content session={session} />;
}
Comment on lines +8 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize route parameters and add error handling.

The page component doesn't use the URL parameters ([slug] and [id]) which are essential for fetching the correct course content.

Consider implementing proper error handling and course data fetching:

export default async function Page({
+ params,+ }: {+ params: { slug: string; id: string };
}) {
const session = await getServerAuthSession();
++ // Fetch course data using params.slug and params.id+ const course = await getCourse(params.slug, params.id);++ if (!course) {+ notFound();+ }- return <Content session={session} />;+ return <Content session={session} course={course} />;
}

Committable suggestion was skipped due to low confidence.

130 changes: 130 additions & 0 deletions app/(app)/courses/mock.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
// Mock video source (this would typically be a path to a video file)
const mockVideoSrc = "https://youtu.be/Ln4KSN0rchI?t=12";

// Mock content list
const mockContentList = [
{
id: 1,
title: "Introduction to React",
longDescription:
"In this video, we will explore the fundamentals of React, including its component-based architecture, virtual DOM, and how to get started with your first React application. This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description: "Learn the basics of React and its core concepts",
watched: false,
publishedDate: "2023-01-15",
author: "John Doe",
imageLink: "https://example.com/images/react-introduction.jpg",
},
{
id: 2,
title: "Setting Up Your Development Environment",
longDescription:
"This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description:
"Install and configure the necessary tools for React development",
watched: true,
publishedDate: "2023-01-22",
author: "Jane Smith",
},
{
id: 3,
title: "Components and Props",
longDescription:
"Dive deep into React's core concepts of components and props. Learn how to create reusable components and pass data between them using props.",
description: "Understand the building blocks of React applications",
watched: true,
publishedDate: "2023-01-29",
author: "John Doe",
},
{
id: 4,
title: "State and Lifecycle",
longDescription:
"Explore how to manage component state and lifecycle methods to build dynamic and interactive applications with React.",
description: "Manage component state and lifecycle methods",
watched: true,
publishedDate: "2023-02-05",
author: "Jane Smith",
},
{
id: 5,
title: "Handling Events",
longDescription:
"Learn how to handle user interactions in React, including events like clicks, form submissions, and more.",
description: "Learn how to handle user interactions in React",
watched: true,
publishedDate: "2023-02-12",
author: "John Doe",
},
{
id: 6,
title: "Conditional Rendering",
longDescription:
"Discover how to display different UI elements based on specific conditions in your React applications.",
description: "Display different UI based on conditions",
watched: false,
publishedDate: "2023-02-19",
author: "Jane Smith",
},
{
id: 7,
title: "Lists and Keys",
longDescription:
"Learn the best practices for rendering lists of components in React and how to efficiently manage them with unique keys.",
description: "Render multiple components efficiently",
watched: false,
publishedDate: "2023-02-26",
author: "John Doe",
},
{
id: 8,
title: "Forms in React",
longDescription:
"This video covers how to create and manage forms in React applications, including controlled and uncontrolled components.",
description: "Create and handle form inputs in React applications",
watched: false,
publishedDate: "2023-03-05",
author: "Jane Smith",
},
{
id: 9,
title: "Hooks: useState and useEffect",
longDescription:
"An introduction to React Hooks, focusing on useState and useEffect to manage state and side effects in functional components.",
description: "Understand and use basic React Hooks",
watched: false,
publishedDate: "2023-03-12",
author: "John Doe",
},
{
id: 10,
title: "Custom Hooks",
longDescription:
"Learn how to create custom hooks in React to encapsulate and reuse logic across components.",
description: "Create reusable logic with custom Hooks",
watched: false,
publishedDate: "2023-03-19",
author: "Jane Smith",
},
{
id: 11,
title: "Context API",
longDescription:
"Explore the Context API to manage global state in your React applications without the need for prop drilling.",
description: "Manage global state in your React application",
watched: false,
publishedDate: "2023-03-26",
author: "John Doe",
},
{
id: 12,
title: "React Router",
longDescription:
"Learn how to implement navigation in your single-page applications using React Router, including dynamic routing and nested routes.",
description: "Implement navigation in your single-page application",
watched: false,
publishedDate: "2023-04-02",
author: "Jane Smith",
},
];

export { mockVideoSrc, mockContentList };
1 change: 1 addition & 0 deletions utils/flags.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
COURSE_VIDEO: "course-video",
JOBS: "jobs",
// Add more feature flags as needed
} as const;
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
131 changes: 131 additions & 0 deletions app/(app)/courses/[slug]/[id]/_client.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
"use client";
import { FEATURE_FLAGS, isFlagEnabled } from "@/utils/flags";
import { CircleCheck, SquarePlay } from "lucide-react";
import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { mockContentList } from "../../mock";

interface ContentProps {
session: Session | null;
}
Comment on lines +8 to +10

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove unused session prop

The session prop is defined in the interface but is not used anywhere within the Content component implementation. The component only uses feature flag checks and renders child components without any session-dependent functionality.

  • Remove the session prop from the ContentProps interface
  • Remove the session parameter from the Content component definition
🔗 Analysis chain

Verify usage of session prop

The session prop is defined in the interface but doesn't appear to be used within the Content component. Either remove the unused prop or implement the intended session-dependent functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash# Search for session usage in the component
ast-grep --pattern 'session.$$$'

Length of output: 10367


Script:

#!/bin/bash# Let's search for the Content component implementation and session usage
ast-grep --pattern 'const Content = ({ session }: ContentProps) => { $$$}'

Length of output: 1434


const Content = ({ session }: ContentProps) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);

if (!flagEnabled) {
notFound();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible stupid question on this one Carolina. Can you put featureflag the route? Then you can just let nexts router deal with the notFound ?

Reason this might be a stupid queston is I have no idea how nexts router works 🤣 I am basing this comment from my knowledge of react router

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how we could but that's a good pattern that we could have. We will figure it out


return (
<div className="overflow-auto">
<div className="w-full divide-x divide-gray-700 lg:grid lg:grid-cols-12">
<VideoPlayer />
<Sidebar contentList={mockContentList} />
</div>
<div className="mx-auto max-w-5xl">
<ContentList contentList={mockContentList} />
</div>
</div>
);
};
Comment on lines +12 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error boundaries and loading states

The component should handle potential errors from mock data and add loading states for better UX. Consider implementing:

  1. Error boundaries to catch rendering errors
  2. Loading states while checking feature flags
  3. Proper error handling for missing or malformed mock data

Here's a suggested implementation:

 const Content = ({ session }: ContentProps) => {
+ const [isLoading, setIsLoading] = useState(true);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSE_VIDEO);
if (!flagEnabled) {
notFound();
}
+ if (isLoading) {+ return <div>Loading...</div>;+ }++ if (!mockContentList?.length) {+ return <div>No content available</div>;+ }+
return (
<div className="overflow-auto">

Committable suggestion was skipped due to low confidence.


const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
Comment on lines +32 to +42

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enhance iframe security and performance

The YouTube iframe implementation needs security and performance improvements:

Apply these security and performance enhancements:

 const VideoPlayer = () => (
<div className="col-span-9">
<div className="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
+ loading="lazy"+ sandbox="allow-scripts allow-same-origin allow-presentation"+ allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"+ referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);

Also consider:

  1. Making the video URL configurable through props
  2. Adding error handling for failed video loads
  3. Implementing a loading placeholder
📝 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.

Suggested change
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
></iframe>
</div>
</div>
);
constVideoPlayer=()=>(
<divclassName="col-span-9">
<divclassName="mx-auto w-full max-w-7xl bg-black">
<iframe
className="aspect-video w-full"
src="https://www.youtube.com/embed/oKKG-CpDjrI"
title="YouTube video"
loading="lazy"
sandbox="allow-scripts allow-same-origin allow-presentation"
allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
referrerPolicy="strict-origin-when-cross-origin"
></iframe>
</div>
</div>
);


interface SidebarProps {
contentList: typeof mockContentList;
}

const Sidebar = ({ contentList }: SidebarProps) => (
<div className="col-span-3 flex w-full flex-col overflow-auto">
<ul className="divide-y divide-gray-700">
{contentList && contentList.length > 0 ? (
contentList.map((item, index) => (
<SidebarItem key={index} item={item} />
))
) : (
<li className="text-center text-gray-500">No content available</li>
)}
</ul>
</div>
);

interface SidebarItemProps {
item: (typeof mockContentList)[0];
}

const SidebarItem = ({ item }: SidebarItemProps) => (
<li className="flex flex-row items-center justify-between px-2 py-2">
<div className="flex flex-row items-center">
<SquarePlay
className="mr-4 h-5 w-5 text-white group-hover:text-white"
aria-hidden="true"
/>
<p>{item.title}</p>
</div>
<CircleCheck
className={`h-6 w-6 ${
item.watched ? "mr-2 h-5 w-5 text-pink-600" : "mr-2 h-5 w-5 text-white"
}`}
aria-hidden="true"
/>
</li>
);
Comment on lines +44 to +82

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve accessibility and type definitions

The sidebar implementation needs accessibility enhancements and better type definitions:

  1. Replace mock data type with proper interface:
interfaceContentItem{id: string;title: string;watched: boolean;// ... other properties}interfaceSidebarProps{contentList: ContentItem[];}
  1. Enhance accessibility:
-<ul className="divide-y divide-gray-700">+<ul + className="divide-y divide-gray-700"+ role="list"+ aria-label="Course content"+>-<li className="flex flex-row items-center justify-between px-2 py-2">+<li + className="flex flex-row items-center justify-between px-2 py-2"+ tabIndex={0}+ role="listitem"+>


interface ContentListProps {
contentList: typeof mockContentList;
}

const ContentList = ({ contentList }: ContentListProps) => {
return (
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<li key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
Comment on lines +88 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix HTML structure and accessibility issues

The ContentList component has several structural and accessibility issues:

Apply these fixes:

 const ContentList = ({ contentList }: ContentListProps) => {
return (
- <>+ <div className="content-list" role="region" aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
}) => (
<div key={id} className="mx-auto w-full max-w-7xl py-8">
<h2 className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
- <li key={author} className="flex justify-between gap-x-6 py-5">+ <div key={author} className="flex justify-between gap-x-6 py-5">
<div className="flex min-w-0 gap-x-4">
<img
- alt=""+ alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<div className="min-w-0 flex-auto">
<p className="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<p className="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
- </li>+ </div>- <div>{description}</div>- <div>{longDescription}</div>+ <div role="article">+ <h3 className="sr-only">Course description</h3>+ <p>{description}</p>+ <p>{longDescription}</p>+ </div>
</div>
),
)}
- </>+ </div>
);
};
📝 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.

Suggested change
constContentList=({ contentList }: ContentListProps)=>{
return(
<>
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<likey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt=""
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</li>
<div>{description}</div>
<div>{longDescription}</div>
</div>
),
)}
</>
);
};
constContentList=({ contentList }: ContentListProps)=>{
return(
<divclassName="content-list"role="region"aria-label="Course content details">
{contentList.map(
({
id,
title,
longDescription,
description,
publishedDate,
author,
imageLink,
})=>(
<divkey={id}className="mx-auto w-full max-w-7xl py-8">
<h2className="text-3xl font-extrabold tracking-tight text-neutral-800 dark:text-white">
{title}
</h2>
<divkey={author}className="flex justify-between gap-x-6 py-5">
<divclassName="flex min-w-0 gap-x-4">
<img
alt={`${author}'s profile picture`}
src={imageLink}
className="h-12 w-12 flex-none rounded-full bg-gray-50"
/>
<divclassName="min-w-0 flex-auto">
<pclassName="text-sm font-semibold leading-6 text-neutral-800 dark:text-white">
{author}
</p>
<pclassName="mt-1 truncate text-xs leading-5 text-gray-500">
{publishedDate}
</p>
</div>
</div>
</div>
<divrole="article">
<h3className="sr-only">Course description</h3>
<p>{description}</p>
<p>{longDescription}</p>
</div>
</div>
),
)}
</div>
);
};


export default Content;
13 changes: 13 additions & 0 deletions app/(app)/courses/[slug]/[id]/page.tsx
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
import { getServerAuthSession } from "@/server/auth";
import Content from "./_client";
Comment on lines +1 to +2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding loading and error states.

The page lacks loading and error handling components which are important for better UX.

Add loading.tsx and error.tsx files in the same directory:

// loading.tsxexportdefaultfunctionLoading(){return<div>Loadingcoursecontent...</div>;}// error.tsxexportdefaultfunctionError({
error,
reset,}: {error: Error;reset: ()=>void;}){return(<div><h2>Somethingwentwrong!</h2><buttononClick={reset}>Tryagain</button></div>);}


export const metadata = {
title: "Video Course",
};
Comment on lines +4 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance metadata with dynamic course information.

The current metadata title is static. Consider utilizing the route parameters to display the actual course title.

export const metadata = {
- title: "Video Course",+ title: async function generateTitle({ params }) {+ const { slug } = params;+ return `${slug.replace('-', ' ')} | Video Course`;+ },
};

Committable suggestion was skipped due to low confidence.


export default async function Page() {
// Example of grabbing session in case it is needed
const session = await getServerAuthSession();

return <Content session={session} />;
}
Comment on lines +8 to +13

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize route parameters and add error handling.

The page component doesn't use the URL parameters ([slug] and [id]) which are essential for fetching the correct course content.

Consider implementing proper error handling and course data fetching:

export default async function Page({
+ params,+ }: {+ params: { slug: string; id: string };
}) {
const session = await getServerAuthSession();
++ // Fetch course data using params.slug and params.id+ const course = await getCourse(params.slug, params.id);++ if (!course) {+ notFound();+ }- return <Content session={session} />;+ return <Content session={session} course={course} />;
}

Committable suggestion was skipped due to low confidence.

130 changes: 130 additions & 0 deletions app/(app)/courses/mock.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
// Mock video source (this would typically be a path to a video file)
const mockVideoSrc = "https://youtu.be/Ln4KSN0rchI?t=12";

// Mock content list
const mockContentList = [
{
id: 1,
title: "Introduction to React",
longDescription:
"In this video, we will explore the fundamentals of React, including its component-based architecture, virtual DOM, and how to get started with your first React application. This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description: "Learn the basics of React and its core concepts",
watched: false,
publishedDate: "2023-01-15",
author: "John Doe",
imageLink: "https://example.com/images/react-introduction.jpg",
},
{
id: 2,
title: "Setting Up Your Development Environment",
longDescription:
"This tutorial guides you through the installation and configuration of essential tools like Node.js, npm, and create-react-app to set up your React development environment.",
description:
"Install and configure the necessary tools for React development",
watched: true,
publishedDate: "2023-01-22",
author: "Jane Smith",
},
{
id: 3,
title: "Components and Props",
longDescription:
"Dive deep into React's core concepts of components and props. Learn how to create reusable components and pass data between them using props.",
description: "Understand the building blocks of React applications",
watched: true,
publishedDate: "2023-01-29",
author: "John Doe",
},
{
id: 4,
title: "State and Lifecycle",
longDescription:
"Explore how to manage component state and lifecycle methods to build dynamic and interactive applications with React.",
description: "Manage component state and lifecycle methods",
watched: true,
publishedDate: "2023-02-05",
author: "Jane Smith",
},
{
id: 5,
title: "Handling Events",
longDescription:
"Learn how to handle user interactions in React, including events like clicks, form submissions, and more.",
description: "Learn how to handle user interactions in React",
watched: true,
publishedDate: "2023-02-12",
author: "John Doe",
},
{
id: 6,
title: "Conditional Rendering",
longDescription:
"Discover how to display different UI elements based on specific conditions in your React applications.",
description: "Display different UI based on conditions",
watched: false,
publishedDate: "2023-02-19",
author: "Jane Smith",
},
{
id: 7,
title: "Lists and Keys",
longDescription:
"Learn the best practices for rendering lists of components in React and how to efficiently manage them with unique keys.",
description: "Render multiple components efficiently",
watched: false,
publishedDate: "2023-02-26",
author: "John Doe",
},
{
id: 8,
title: "Forms in React",
longDescription:
"This video covers how to create and manage forms in React applications, including controlled and uncontrolled components.",
description: "Create and handle form inputs in React applications",
watched: false,
publishedDate: "2023-03-05",
author: "Jane Smith",
},
{
id: 9,
title: "Hooks: useState and useEffect",
longDescription:
"An introduction to React Hooks, focusing on useState and useEffect to manage state and side effects in functional components.",
description: "Understand and use basic React Hooks",
watched: false,
publishedDate: "2023-03-12",
author: "John Doe",
},
{
id: 10,
title: "Custom Hooks",
longDescription:
"Learn how to create custom hooks in React to encapsulate and reuse logic across components.",
description: "Create reusable logic with custom Hooks",
watched: false,
publishedDate: "2023-03-19",
author: "Jane Smith",
},
{
id: 11,
title: "Context API",
longDescription:
"Explore the Context API to manage global state in your React applications without the need for prop drilling.",
description: "Manage global state in your React application",
watched: false,
publishedDate: "2023-03-26",
author: "John Doe",
},
{
id: 12,
title: "React Router",
longDescription:
"Learn how to implement navigation in your single-page applications using React Router, including dynamic routing and nested routes.",
description: "Implement navigation in your single-page application",
watched: false,
publishedDate: "2023-04-02",
author: "Jane Smith",
},
];

export { mockVideoSrc, mockContentList };
1 change: 1 addition & 0 deletions utils/flags.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
COURSE_VIDEO: "course-video",
JOBS: "jobs",
// Add more feature flags as needed
} as const;
Expand Down