Skip to content

Conversation

@HarshMN2345
Copy link
Member

@HarshMN2345 HarshMN2345 commented Dec 3, 2025

What does this PR do?

image image image

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

yes

Summary by CodeRabbit

  • New Features
    • Added a VideoCard component to display video tutorials with title, conditional duration (MM:SS), responsive layouts, and external-link behavior.
    • Enabled embedding of VideoCard via documentation tags so videos can be placed inside docs.
    • Added a new "Create database" step to the Quick Start docs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

Adds a new Svelte component VideoCard (src/lib/components/VideoCard.svelte) with props: href, title, duration?, and class?. It parses duration inputs (number, numeric string, or time-formatted string) into MM:SS and renders a responsive anchor with desktop and mobile variants, duration display, and external-link icon. Exports VideoCard from src/lib/components/index.ts. Adds a Markdoc wrapper tag (src/markdoc/tags/Video_Card.svelte) and re-exports it in src/markdoc/tags/_Module.svelte. Inserts a new "Create database" section into a docs page.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review parseDuration function for edge cases and input normalization (e.g., "1:2", "90", invalid strings).
  • Verify conditional rendering of duration and that undefined/invalid durations produce no display.
  • Check responsive/layout classes and use of the cn utility for class composition.
  • Confirm Icon usage (play and ext-link) matches available icon API.
  • Validate Markdoc wrapper prop forwarding and type compatibility between Video_Card and VideoCard.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a new VideoCard component for documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-SER-652-new-video-card-component-for-docs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/markdoc/tags/Video_Card.svelte (1)

1-10: Align $props usage with idiomatic Svelte 5 patterns

The wrapper wiring looks fine, but you may want to align with the common Svelte 5 $props pattern for reactivity and typing:

-    interface Props {
-        href: string;
-        title: string;
-    }
-
-    const { href, title }: Props = $props();
+    type Props = {
+        href: string;
+        title: string;
+    };
+
+    let { href, title } = $props<Props>();

This keeps props reactive if the tag is ever used with dynamic values and improves TypeScript inference.

Please double-check against the current Svelte 5 $props documentation to confirm whether const destructuring is acceptable or if let is preferred in this codebase.

src/lib/components/VideoCard.svelte (1)

1-12: Consider using let + generic for $props in Svelte 5

The props definition is clear, but for Svelte 5 runes it’s typically more idiomatic (and safer for reactivity/TS) to do:

-    interface Props {
-        href: string;
-        title: string;
-        class?: string;
-    }
-
-    const { href, title, class: className = '' }: Props = $props();
+    type Props = {
+        href: string;
+        title: string;
+        class?: string;
+    };
+
+    let { href, title, class: className = '' } = $props<Props>();

This keeps the props reactive and tightens TypeScript inference around $props.

Please verify this against the current Svelte 5 $props typing guidance used in this repo.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0ba1de and 6625a69.

📒 Files selected for processing (5)
  • src/lib/components/VideoCard.svelte (1 hunks)
  • src/lib/components/index.ts (1 hunks)
  • src/markdoc/tags/Video_Card.svelte (1 hunks)
  • src/markdoc/tags/_Module.svelte (1 hunks)
  • src/routes/docs/products/databases/quick-start/+page.markdoc (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-05T19:35:04.162Z
Learnt from: ebenezerdon
Repo: appwrite/website PR: 2372
File: src/routes/docs/tutorials/nuxt/step-6/+page.markdoc:79-83
Timestamp: 2025-09-05T19:35:04.162Z
Learning: In Nuxt tutorial files, the correct Appwrite database instance to use is `tablesDB` (imported from "~/appwrite"), not `database`. All tablesDB method calls should use the object parameter format: listRows({ databaseId, tableId, queries }), createRow({ databaseId, tableId, rowId, data }), and deleteRow({ databaseId, tableId, rowId }).

Applied to files:

  • src/routes/docs/products/databases/quick-start/+page.markdoc
📚 Learning: 2025-09-05T19:35:59.449Z
Learnt from: ebenezerdon
Repo: appwrite/website PR: 2372
File: src/routes/docs/tutorials/react-native/step-6/+page.markdoc:67-73
Timestamp: 2025-09-05T19:35:59.449Z
Learning: In React Native tutorials, the lib/appwrite.js file exports `tablesDB` (not `databases`), so imports should use `import { tablesDB } from "../lib/appwrite"` to match the export pattern established in step-3.

Applied to files:

  • src/routes/docs/products/databases/quick-start/+page.markdoc
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build
  • GitHub Check: assets
  • GitHub Check: tests
🔇 Additional comments (3)
src/markdoc/tags/_Module.svelte (1)

24-24: Video_Card tag wiring looks correct

Exporting Video_Card here matches the existing pattern and should make the {% video_card %} Markdoc tag available without impacting other tags.

src/lib/components/index.ts (1)

17-17: Barrel export for VideoCard is consistent

Publicly exporting VideoCard here is consistent with the rest of the component index and enables reuse outside Markdoc.

src/lib/components/VideoCard.svelte (1)

83-186: Responsive styling and typography look consistent

The custom CSS around .video-card, desktop/mobile layouts, and breakpoints (1023.9px / 767.9px / 375px) is coherent and should give a predictable responsive layout. Typography tokens (--font-family-sansSerif, --font-size-small, etc.) also match the design-system style.

Comment on lines 14 to 75
<a
{href}
target="_blank"
rel="noopener noreferrer"
class={cn(
'video-card no-underline',
'bg-white/90 dark:bg-white/[0.04]',
'border-black/8 dark:border-white/[0.06]',
'hover:bg-white/95 dark:hover:bg-white/[0.06]',
className
)}
>
<div class="video-card-desktop">
<div
class="video-tutorial-label bg-greyscale-200 flex shrink-0 items-center gap-1.5 rounded-md px-2 py-0.5 dark:bg-[rgb(51,51,51)]"
>
<Icon
name="play"
class="video-tutorial-icon text-greyscale-900 dark:text-greyscale-100 h-5 w-5 shrink-0"
/>
<span class="text-greyscale-900 dark:text-greyscale-100 whitespace-nowrap"
>Video tutorial</span
>
</div>

<!-- Main title -->
<span class="video-card-title text-greyscale-900 dark:text-greyscale-100 flex-1">
{title}
</span>

<Icon
name="ext-link"
class={cn(
'video-card-ext-link h-5 w-5 shrink-0 transition-transform group-hover:translate-x-0.5 group-hover:-translate-y-0.5',
'text-greyscale-900 dark:text-greyscale-100'
)}
/>
</div>

<div class="video-card-mobile">
<div class="video-card-header">
<!-- video label -->
<div
class="video-tutorial-label bg-greyscale-200 flex shrink-0 items-center gap-1.5 rounded-md px-2 py-0.5 dark:bg-[rgb(51,51,51)]"
>
<Icon
name="play"
class="video-tutorial-icon text-greyscale-900 dark:text-greyscale-100 h-5 w-5 shrink-0"
/>
<span class="text-greyscale-900 dark:text-greyscale-100 whitespace-nowrap"
>Video tutorial</span
>
</div>

<Icon
name="ext-link"
class={cn(
'video-card-ext-link h-5 w-5 shrink-0 transition-transform group-hover:translate-x-0.5 group-hover:-translate-y-0.5',
'text-greyscale-900 dark:text-greyscale-100'
)}
/>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/lib/components/VideoCard.svelte

Repository: appwrite/website

Length of output: 6350


🏁 Script executed:

fd -e js -e ts -e json "tailwind" -H | head -20

Repository: appwrite/website

Length of output: 42


🏁 Script executed:

rg "group-hover" -t svelte -t ts -t js --no-heading

Repository: appwrite/website

Length of output: 90


🏁 Script executed:

rg "class.*group\"" -t svelte --max-count 20

Repository: appwrite/website

Length of output: 90


🏁 Script executed:

find . -name "tailwind.config.*" -o -name "tailwind.config*" 2>/dev/null | head -10

Repository: appwrite/website

Length of output: 42


🏁 Script executed:

rg "group-hover" --no-filename -g "*.svelte" -g "*.ts" -g "*.js"

Repository: appwrite/website

Length of output: 8121


🏁 Script executed:

rg "\.group[\"'\s]" --no-filename

Repository: appwrite/website

Length of output: 199


🏁 Script executed:

rg "group-hover:" -B 5 -A 2 --no-filename | head -60

Repository: appwrite/website

Length of output: 2651


🏁 Script executed:

git ls-files | grep -E "\.(svelte|ts|js)$" | xargs grep -l "class.*group['\"]" | head -10

Repository: appwrite/website

Length of output: 241


🏁 Script executed:

cat tailwind.config.ts 2>/dev/null || cat tailwind.config.js 2>/dev/null || echo "No config found"

Repository: appwrite/website

Length of output: 75


🏁 Script executed:

rg "group" tsconfig.json tailwind.config.* -A 3 2>/dev/null || true

Repository: appwrite/website

Length of output: 42


Add .group class to enable group-hover:* transitions on external link icons

The external link icons on lines 47 and 71 use group-hover:translate-x-0.5 group-hover:-translate-y-0.5, but the root <a> element lacks a .group class. Without it, these hover transitions won't trigger. Other components in the codebase (Carousel, DemoCarousel, TestimonialsCarousel) correctly use .group as a parent for group-hover:* utilities.

Add group to the root element:

     class={cn(
-        'video-card no-underline',
+        'video-card group no-underline',
         'bg-white/90 dark:bg-white/[0.04]',

Alternatively, replace group-hover:* with hover:* if group semantics aren't needed.

🤖 Prompt for AI Agents
In src/lib/components/VideoCard.svelte around lines 14 to 75, the root <a>
element uses child classes with group-hover but the anchor's class list
(cn(...)) does not include the Tailwind "group" utility, so group-hover
transitions on the external link icons won't run; fix by adding "group" to the
anchor's class array passed to cn (e.g., include 'group' alongside 'video-card
no-underline' or merge with className) so the child group-hover utilities are
activated (alternatively, if group semantics are unnecessary, replace child
group-hover utilities with hover:*).

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/lib/components/VideoCard.svelte (1)

70-135: group-hover:* transitions won’t activate without .group on the root anchor

The external link icons use group-hover:translate-x-0.5 group-hover:-translate-y-0.5 (Lines 107 and 131), but the root <a>’s class list (Lines 74–80) does not include the Tailwind group utility, so those transitions never trigger.

Consider updating the anchor to include group:

<a
    {href}
    target="_blank"
    rel="noopener noreferrer"
    class={cn(
-        'video-card no-underline',
+        'video-card group no-underline',
        'bg-white/90 dark:bg-white/[0.04]',
        'border-black/8 dark:border-white/[0.06]',
        'hover:bg-white/95 dark:hover:bg-white/[0.06]',
        className
    )}
>

Alternatively, if group semantics aren’t needed, replace group-hover:* with hover:* on the icon classes.

🧹 Nitpick comments (2)
src/routes/api/youtube-duration/+server.ts (1)

3-72: Endpoint logic looks solid; consider tightening parsing and error semantics

The handler is functionally correct and safe for untrusted id values, but a few small refinements would make it more robust long‑term:

  • The duration extraction is tightly coupled to YouTube’s current HTML/JSON structure and regex variants. If this becomes user‑visible and important, consider either:
    • Preferring a single, better‑targeted JSON snippet (e.g., videoDetails.lengthSeconds) over multiple loosely targeted regexes, or
    • Moving to a structured API/metadata source, if product constraints ever allow it.
  • All non‑OK upstream responses are mapped to a 500. If you want clearer client behavior, you could special‑case response.status === 404 and propagate 404 instead of treating it as a generic server error.
  • JSON.stringify + manual Content-Type is fine, but using SvelteKit’s json() helper would reduce repetition and keep response formatting consistent across endpoints.

These are non‑blocking, but worth considering for resilience and maintainability.

src/lib/components/VideoCard.svelte (1)

14-67: Minor cleanup in state and response handling

Two small, non‑blocking improvements here:

  • duration (Line 14) is never read in the template; only durationFormatted is used. You can drop duration entirely and just keep the formatted string to reduce state noise.
  • When reading data.duration (Lines 55–60), you may want to guard on type as well as truthiness, e.g. if (typeof data.duration === 'number' && data.duration > 0) to avoid unexpected shapes from the API in the future.

Behavior is otherwise sound; onMount‑only fetch keeps SSR safe, and failures are handled gracefully by simply omitting the duration.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6625a69 and 4f34033.

📒 Files selected for processing (3)
  • src/lib/components/VideoCard.svelte (1 hunks)
  • src/routes/api/youtube-duration/+server.ts (1 hunks)
  • src/routes/docs/products/databases/quick-start/+page.markdoc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/routes/docs/products/databases/quick-start/+page.markdoc
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/api/youtube-duration/+server.ts (1)
server/sitemap.js (1)
  • match (47-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: assets
  • GitHub Check: build
  • GitHub Check: tests

Comment on lines 51 to 53
if (!response.ok) {
throw new Error(`Failed to fetch duration: ${response.status}`);
}
Copy link
Member

Choose a reason for hiding this comment

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

lets not throw. If there's an error or the duration is not available then we can just hide the time. early return sounds better here.

Comment on lines 26 to 34
const html = await response.text();
const patterns = [
/"lengthSeconds":"(\d+)"/,
/"approxDurationMs":"(\d+)"/,
/"videoDetails":\s*\{[^}]*"lengthSeconds":\s*"(\d+)"/,
/"lengthSeconds":\s*"(\d+)"/,
/"duration":\s*"PT(\d+)S"/,
/itemprop="duration"\s+content="PT(\d+)S"/
];
Copy link
Member

Choose a reason for hiding this comment

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

this is very fragile and can easily break if something changes on the domain. We might have to look for some better logic here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/lib/components/VideoCard.svelte (1)

39-49: Add .group class to enable group-hover:* transitions.

The external link icons on lines 76 and 100 use group-hover:translate-x-0.5 group-hover:-translate-y-0.5, but the root <a> element still lacks the .group class. These hover transitions won't trigger without it.

Add group to the class list:

     class={cn(
-        'video-card no-underline',
+        'video-card group no-underline',
         'bg-white/90 dark:bg-white/[0.04]',
🧹 Nitpick comments (2)
src/lib/components/VideoCard.svelte (2)

51-113: Consider refactoring duplicate code between desktop and mobile layouts.

The desktop (lines 51-80) and mobile (lines 82-113) layouts share significant duplicate code for the video tutorial label, external link icon, and title display. This duplication increases maintenance burden.

Consider extracting shared markup into separate components or using conditional CSS for layout switching:

Option 1: Extract shared elements into local components within this file.

Option 2: Use Tailwind responsive utilities with a single layout:

<div class="flex flex-col md:flex-row md:items-center gap-3 w-full">
    <div class="flex items-center justify-between md:justify-start gap-3">
        <!-- label -->
        <div class="video-tutorial-label ...">...</div>
        <!-- icon shown on mobile, hidden on desktop -->
        <Icon class="md:hidden ..." />
    </div>
    <!-- title -->
    <span class="flex-1">...</span>
    <!-- icon shown on desktop, hidden on mobile -->
    <Icon class="hidden md:block ..." />
</div>

152-199: Consider aligning custom breakpoints with Tailwind's standard breakpoints.

The custom media queries use breakpoints at 1023.9px, 767.9px, and 375px. The .9px technique creates a "just below" breakpoint, but Tailwind v4's standard breakpoints (md: 768px, lg: 1024px) could be used directly with max-width for cleaner code.

Consider using Tailwind's standard breakpoints:

-    @media (max-width: 1023.9px) {
+    @media (max-width: 1024px) {
         .video-card {
             flex-wrap: wrap;
         }
     }

-    @media (max-width: 767.9px) {
+    @media (max-width: 768px) {
         .video-card {

Alternatively, leverage Tailwind's responsive utilities more directly in the markup to reduce custom CSS.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 331c2fd and a2511f5.

📒 Files selected for processing (3)
  • src/lib/components/VideoCard.svelte (1 hunks)
  • src/markdoc/tags/Video_Card.svelte (1 hunks)
  • src/routes/docs/products/databases/quick-start/+page.markdoc (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-05T19:35:04.162Z
Learnt from: ebenezerdon
Repo: appwrite/website PR: 2372
File: src/routes/docs/tutorials/nuxt/step-6/+page.markdoc:79-83
Timestamp: 2025-09-05T19:35:04.162Z
Learning: In Nuxt tutorial files, the correct Appwrite database instance to use is `tablesDB` (imported from "~/appwrite"), not `database`. All tablesDB method calls should use the object parameter format: listRows({ databaseId, tableId, queries }), createRow({ databaseId, tableId, rowId, data }), and deleteRow({ databaseId, tableId, rowId }).

Applied to files:

  • src/routes/docs/products/databases/quick-start/+page.markdoc
📚 Learning: 2025-09-05T19:35:59.449Z
Learnt from: ebenezerdon
Repo: appwrite/website PR: 2372
File: src/routes/docs/tutorials/react-native/step-6/+page.markdoc:67-73
Timestamp: 2025-09-05T19:35:59.449Z
Learning: In React Native tutorials, the lib/appwrite.js file exports `tablesDB` (not `databases`), so imports should use `import { tablesDB } from "../lib/appwrite"` to match the export pattern established in step-3.

Applied to files:

  • src/routes/docs/products/databases/quick-start/+page.markdoc
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: assets
  • GitHub Check: build
  • GitHub Check: tests
🔇 Additional comments (3)
src/lib/components/VideoCard.svelte (1)

5-10: LGTM!

The Props interface is well-defined with appropriate types for all properties.

src/routes/docs/products/databases/quick-start/+page.markdoc (1)

7-7: LGTM!

The video_card component is properly integrated with a real YouTube URL and appropriate metadata. The previous mock placeholder concern has been addressed.

src/markdoc/tags/Video_Card.svelte (1)

1-13: LGTM!

The markdoc tag wrapper correctly forwards props to the VideoCard component. The duration?: string type is appropriate since markdoc attributes are typically strings, and the VideoCard's parseDuration function handles string inputs correctly.

@HarshMN2345 HarshMN2345 merged commit c7e8d76 into main Dec 5, 2025
6 checks passed
@HarshMN2345 HarshMN2345 deleted the feat-SER-652-new-video-card-component-for-docs branch December 5, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants