Add video support to embedded galleries; fix gallery limit bug #3821
Conversation
Add full video support to the embed widget, allowing videos to be displayed with play icon overlays on thumbnails and native HTML5 video playback in the lightbox. Changes: - Add is_video and duration fields to EmbedPhotoResource - Display play icon overlay on video thumbnails in all layouts - Implement video player in lightbox with autoplay - Show video-specific metadata (duration and framerate) instead of photo EXIF - Update TypeScript types to support video content - Fix size variants type to allow nullable original for videos Videos now work seamlessly in embedded galleries across all layout modes (justified, masonry, grid, square, filmstrip) with proper metadata display. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fixed variable reference error in EmbededRequest that caused only 1 photo to be returned regardless of the selected limit (6, 12, 18, 30, etc.). The bug was on line 69 where the code incorrectly cast $this->limit instead of the local $limit variable, causing (int) null to equal 0, which always evaluated to max(1, min(0, 500)) = 1. Issue: embedded galleries only showed one item when a specific photo count was selected. Fix: now correctly cast $limit variable to properly apply user-selected limits.
📝 WalkthroughWalkthroughThis pull request adds video support to the embed widget system. Changes include modifying limit validation in embed requests, adding video metadata properties to photo resources, introducing video overlay UI components, implementing video playback in the lightbox viewer, updating TypeScript types to support video fields, and adjusting utility function signatures for nullable size variants. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks❌ Failed checks (1 warning)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
resources/js/embed/components/EmbedWidget.vue (1)
60-66: Consider extracting the play icon SVG to reduce duplication.The same play icon SVG markup is repeated in three locations (filmstrip main viewer, filmstrip thumbnails, and regular grid). While this works fine, extracting it to a reusable component or constant would improve maintainability.
Example approach:
<template> <div v-if="isVideo" class="lychee-embed__video-overlay"> <PlayIcon /> </div> </template>Or define the SVG as a constant if you prefer to keep it inline.
Also applies to: 127-133, 165-171
app/Http/Resources/Embed/EmbedPhotoResource.php (1)
42-45: Add clarifying comment to EXIF export for video duration in aperture field.The frontend correctly protects against displaying video duration as aperture via the
!is_videocheck (Lightbox.vue lines 95–101). However, the EXIF export at line 57 of EmbedPhotoResource.php exposes the aperture field for all photos and videos without documenting that it contains duration in seconds for videos. To prevent future confusion and improve API transparency, add a clarifying comment above the EXIF array noting this dual-purpose behavior:// For videos, aperture contains duration in seconds (see line 43) $this->exif = [ 'make' => $photo->make, 'model' => $photo->model, 'lens' => $photo->lens, 'iso' => $photo->iso, 'aperture' => $photo->aperture, // Dual-purpose: f-stop for photos, duration (seconds) for videos 'shutter' => $photo->shutter, 'focal' => $photo->focal, 'taken_at' => $photo->taken_at?->toIso8601String(), ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/Http/Requests/Embed/EmbededRequest.php(1 hunks)app/Http/Resources/Embed/EmbedPhotoResource.php(3 hunks)resources/js/embed/components/EmbedWidget.vue(4 hunks)resources/js/embed/components/Lightbox.vue(4 hunks)resources/js/embed/types.ts(2 hunks)resources/js/embed/utils/columns.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-14T10:17:35.082Z
Learnt from: ildyria
Repo: LycheeOrg/Lychee PR: 3616
File: app/Actions/Album/PositionData.php:38-39
Timestamp: 2025-08-14T10:17:35.082Z
Learning: PositionDataResource uses toPhotoResources() method to convert photos to PhotoResource instances, and PhotoResource accesses the tags relationship ($photo->tags->pluck('name')->all()), making the 'tags' eager-loading in PositionData necessary to avoid N+1 queries.
Applied to files:
app/Http/Resources/Embed/EmbedPhotoResource.php
🧬 Code graph analysis (1)
app/Http/Resources/Embed/EmbedPhotoResource.php (1)
app/Assets/Helpers.php (1)
secondsToHMS(135-144)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: 3️⃣ PHP dist / 8.4 - postgresql
- GitHub Check: 3️⃣ PHP dist / 8.3 - sqlite
- GitHub Check: 3️⃣ PHP dist / 8.4 - sqlite
- GitHub Check: 3️⃣ PHP dist / 8.3 - postgresql
- GitHub Check: 2️⃣ PHP tests / 8.4 - sqlite -- Install
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.4 - postgresql -- Install
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- ImageProcessing
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- ImageProcessing
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Webshop
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Install
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- Unit
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- Webshop
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- Unit
- GitHub Check: 2️⃣ PHP tests / 8.3 - sqlite -- Unit
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- ImageProcessing
- GitHub Check: 2️⃣ PHP tests / 8.3 - postgresql -- Feature_v2
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- Install
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- Webshop
- GitHub Check: 2️⃣ PHP tests / 8.3 - mariadb -- Feature_v2
🔇 Additional comments (8)
app/Http/Requests/Embed/EmbededRequest.php (1)
69-69: Excellent bug fix!This correctly fixes the gallery limit bug mentioned in the PR title. The original code was incorrectly using
$this->limit(which isnullat this point) instead of the local$limitvariable, causing the limit validation to fail.resources/js/embed/types.ts (2)
99-100: LGTM: Clean addition of video metadata fields.The
is_videoanddurationfields align well with the backendEmbedPhotoResourceand enable the video support features throughout the UI.
109-109: Good consistency improvement for size variant types.Changing
originalfrom a nested object toSizeVariantData | nullmakes it consistent with other size variants. The codebase already handles this correctly with optional chaining ingetSafeDimensions(columns.ts) andgetVideoUrl(Lightbox.vue).resources/js/embed/utils/columns.ts (1)
175-175: LGTM: Type signature update aligns with nullable original variant.The signature change correctly supports the updated
originaltype fromtypes.ts. Makingwidthandheightoptional provides additional safety, and the function already handles missing dimensions with fallback logic.resources/js/embed/components/EmbedWidget.vue (1)
636-664: LGTM: Well-implemented video overlay styling.The CSS provides good visual feedback with hover transitions and responsive sizing constraints. The
pointer-events: noneensures clicks pass through to the underlying elements correctly.resources/js/embed/components/Lightbox.vue (3)
40-52: LGTM: Clean video element implementation.The video element correctly uses
controlsandautoplayfor good UX, and properly handles theloadeddataevent for videos (vs.loadfor images).Minor note: The error handler at line 318 displays "Failed to load image" which would be shown for videos too. This is a very minor UX inconsistency.
219-225: LGTM: Clean video URL helper.The
getVideoUrlfunction correctly returns the original size variant URL for videos, ensuring best quality playback. The optional chaining and empty string fallback handle missing variants safely.
114-130: Video metadata section correctly implements documented field reuse pattern.The
focalfield reuse for framerate is intentionally documented in the Photo model (lines 350-355) with an acknowledgment that it should be refactored. The Vue component correctly displays this field with the "fps" suffix for videos (line 126-128), and theaperturefield similarly stores duration for videos.The conditional rendering properly separates photo EXIF from video metadata using
is_videochecks.
|
You can test this change live at https://www.dzombak.com/blog/2025/11/beavers-in-dexter-mi/ . |
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
2 participants
This PR:
limitparameter other than "no limit" resulted in only one photo being returnedSummary by CodeRabbit
Release Notes