-
Notifications
You must be signed in to change notification settings - Fork 294
feat: new video card component for docs #2647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: new video card component for docs #2647
Conversation
WalkthroughAdds 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
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/markdoc/tags/Video_Card.svelte (1)
1-10: Align$propsusage with idiomatic Svelte 5 patternsThe wrapper wiring looks fine, but you may want to align with the common Svelte 5
$propspattern 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
$propsdocumentation to confirm whetherconstdestructuring is acceptable or ifletis preferred in this codebase.src/lib/components/VideoCard.svelte (1)
1-12: Consider usinglet+ generic for$propsin Svelte 5The 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
$propstyping guidance used in this repo.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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 correctExporting
Video_Cardhere 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 forVideoCardis consistentPublicly exporting
VideoCardhere 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 consistentThe 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.
| <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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/components/VideoCard.svelteRepository: appwrite/website
Length of output: 6350
🏁 Script executed:
fd -e js -e ts -e json "tailwind" -H | head -20Repository: appwrite/website
Length of output: 42
🏁 Script executed:
rg "group-hover" -t svelte -t ts -t js --no-headingRepository: appwrite/website
Length of output: 90
🏁 Script executed:
rg "class.*group\"" -t svelte --max-count 20Repository: appwrite/website
Length of output: 90
🏁 Script executed:
find . -name "tailwind.config.*" -o -name "tailwind.config*" 2>/dev/null | head -10Repository: 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-filenameRepository: appwrite/website
Length of output: 199
🏁 Script executed:
rg "group-hover:" -B 5 -A 2 --no-filename | head -60Repository: appwrite/website
Length of output: 2651
🏁 Script executed:
git ls-files | grep -E "\.(svelte|ts|js)$" | xargs grep -l "class.*group['\"]" | head -10Repository: 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 || trueRepository: 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:*).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/lib/components/VideoCard.svelte (1)
70-135:group-hover:*transitions won’t activate without.groupon the root anchorThe 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 Tailwindgrouputility, 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:*withhover:*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 semanticsThe handler is functionally correct and safe for untrusted
idvalues, 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 === 404and propagate 404 instead of treating it as a generic server error.JSON.stringify+ manualContent-Typeis fine, but using SvelteKit’sjson()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 handlingTwo small, non‑blocking improvements here:
duration(Line 14) is never read in the template; onlydurationFormattedis used. You can dropdurationentirely 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
📒 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
src/lib/components/VideoCard.svelte
Outdated
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch duration: ${response.status}`); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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"/ | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is very fragile and can easily break if something changes on the domain. We might have to look for some better logic here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib/components/VideoCard.svelte (1)
39-49: Add.groupclass to enablegroup-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.groupclass. These hover transitions won't trigger without it.Add
groupto 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, and375px. The.9pxtechnique creates a "just below" breakpoint, but Tailwind v4's standard breakpoints (md: 768px,lg: 1024px) could be used directly withmax-widthfor 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
📒 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?: stringtype is appropriate since markdoc attributes are typically strings, and the VideoCard's parseDuration function handles string inputs correctly.
What does this PR do?
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
✏️ Tip: You can customize this high-level summary in your review settings.