Add YouTube Shorts section with split layout and auto-play#36
Conversation
Co-authored-by: castellbranco <80287588+castellbranco@users.noreply.github.com>
|
@copilot I just test it via the localhost and even though i mark them has short form it does not go to the shorts an showcase the video is just like this, so even though im clicking on the short its saving the videos with regular type 
|
Co-authored-by: castellbranco <80287588+castellbranco@users.noreply.github.com>
Fixed in commit a625d6c. The issue was that the
When you select "Short (Vertical)" in the admin panel and save, it will now correctly save as |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive YouTube Shorts feature for the videos page, introducing a TikTok-like vertical video player alongside the existing regular video gallery. The implementation includes full-stack changes from database models to the user interface.
Key Changes:
- Added
video_typefield to distinguish between regular videos and shorts (vertical format) - Redesigned the videos page with a split layout: 2x2 grid for regular videos on the left, shorts player on the right
- Implemented auto-playing shorts with navigation controls (swipe on mobile, arrows on desktop)
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
backend/app/models/video.py |
Added video_type field as optional Literal type with "regular" default value |
backend/app/services/video_service.py |
Updated video queries to include video_type field and added date-based descending sort |
src/lib/types.ts |
Extended VideoHighlight interface with optional videoType field |
src/lib/admin-api.ts |
Added bidirectional mapping between camelCase videoType and snake_case video_type for all video API operations |
src/components/admin/VideoManagement.tsx |
Added video type selector dropdown with "Regular Video" and "Short (Vertical)" options |
src/components/sections/VideosPage.tsx |
Complete redesign with split layout, pagination for regular videos, auto-playing shorts player with touch/click navigation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| date: str | ||
| video_url: Optional[str] = None | ||
| thumbnail_url: Optional[str] = None | ||
| video_type: Optional[Literal["regular", "short"]] = "regular" |
There was a problem hiding this comment.
The documentation in agents/ directory has not been updated to reflect the new video_type field in the VideoHighlight model. The documentation shows the old schema without the videoType field. Consider updating the documentation files (AGENTS.md, CLAUDE.md, GEMINI.md) to include the new optional videoType?: "regular" | "short" field in the VideoHighlight interface.
| style={{ | ||
| aspectRatio: "9/16", | ||
| height: "70vh", | ||
| minHeight: "500px" |
There was a problem hiding this comment.
The inline height styling with 70vh and minHeight: "500px" could cause display issues on smaller screens or landscape mobile devices. On mobile devices with heights less than ~715px, the 500px minimum height will overflow the viewport. Consider using a responsive height approach or max-height constraint to ensure the shorts player fits within the viewport across all device sizes.
| minHeight: "500px" | |
| maxHeight: "100vh" |
| <iframe | ||
| key={shorts[currentShortIndex].id} | ||
| src={getShortsEmbedUrl(shorts[currentShortIndex].videoUrl)} | ||
| className="w-full h-full" | ||
| allowFullScreen | ||
| title={shorts[currentShortIndex].title} | ||
| allow="autoplay; encrypted-media" | ||
| /> |
There was a problem hiding this comment.
The iframe lacks proper error handling if the video fails to load or if the URL is invalid. Consider adding an error state and displaying a fallback UI when the iframe fails to load, especially since autoplay can fail due to browser policies.
| const handleTouchStart = (e: React.TouchEvent) => { | ||
| setTouchStart(e.touches[0].clientY); | ||
| }; | ||
|
|
||
| const handleTouchEnd = (e: React.TouchEvent) => { | ||
| const touchEnd = e.changedTouches[0].clientY; | ||
| const diff = touchStart - touchEnd; | ||
|
|
||
| // Swipe down (diff < 0) goes to previous, swipe up (diff > 0) goes to next | ||
| if (Math.abs(diff) > 50) { | ||
| if (diff > 0) { | ||
| handleNextShort(); | ||
| } else { | ||
| handlePrevShort(); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
The touch event handlers could interfere with normal scrolling behavior on the page. Consider checking if the touch interaction is primarily vertical (for swipe detection) vs horizontal (for potential scrolling), and only prevent default behavior when a clear vertical swipe is detected. Additionally, the current implementation doesn't call preventDefault(), which could lead to unintended scroll while swiping.
| <Button | ||
| variant="secondary" | ||
| size="icon" | ||
| onClick={handlePrevShort} | ||
| disabled={currentShortIndex === 0} | ||
| className="bg-black/50 hover:bg-black/70 text-white" | ||
| > | ||
| <CaretUp size={20} weight="bold" /> | ||
| </Button> | ||
| <Button | ||
| variant="secondary" | ||
| size="icon" | ||
| onClick={handleNextShort} | ||
| disabled={currentShortIndex === shorts.length - 1} | ||
| className="bg-black/50 hover:bg-black/70 text-white" | ||
| > | ||
| <CaretDown size={20} weight="bold" /> | ||
| </Button> |
There was a problem hiding this comment.
The navigation buttons for shorts lack proper ARIA labels. Screen readers won't announce the purpose of these buttons clearly. Add aria-label attributes such as "Previous short" and "Next short" to improve accessibility.
| "video_type": 1, | ||
| }, | ||
| ) | ||
| ).sort("date", -1) |
There was a problem hiding this comment.
The get_all_videos() method sorts by date without an index. For optimal performance as the video collection grows, consider adding a MongoDB index on the date field. This will significantly improve query performance, especially with the descending sort operation. Add an index creation in your database initialization: db.videos.create_index([("date", -1)]).
| onTouchEnd={handleTouchEnd} | ||
| > | ||
| <iframe | ||
| key={shorts[currentShortIndex].id} |
There was a problem hiding this comment.
The key prop on the iframe only uses the video ID, which may not force a re-render if the same video is navigated to again (e.g., if videos are reordered or filtered). Consider using a composite key like key={${shorts[currentShortIndex].id}-${currentShortIndex}} to ensure the iframe reloads when the index changes.
| key={shorts[currentShortIndex].id} | |
| key={`${shorts[currentShortIndex].id}-${currentShortIndex}`} |
| if (video.videoUrl) payload.video_url = video.videoUrl; | ||
| if (video.thumbnailUrl !== undefined) | ||
| payload.thumbnail_url = video.thumbnailUrl; | ||
| if (video.videoType) payload.video_type = video.videoType; |
There was a problem hiding this comment.
The conditional logic if (video.videoType) in the payload creation will not send video_type when the value is explicitly set to "regular". This could cause issues if a video is being updated from "short" back to "regular". The condition should be if (video.videoType !== undefined) to ensure the field is always sent when present.
| if (video.videoType) payload.video_type = video.videoType; | |
| if (video.videoType !== undefined) payload.video_type = video.videoType; |
| const handleNextShort = () => { | ||
| if (currentShortIndex < shorts.length - 1) { | ||
| setCurrentShortIndex(currentShortIndex + 1); | ||
| } |
There was a problem hiding this comment.
When currentShortIndex changes, the previous video continues playing in the background until the new iframe loads. This can cause audio overlap and performance issues. Consider adding cleanup logic or ensuring the previous iframe is properly unmounted before loading the next one.
| }>({}); | ||
|
|
||
| // Pagination state for regular videos | ||
| const [currentPage, setCurrentPage] = useState(0); |
There was a problem hiding this comment.
Similarly, currentPage is not reset when the regularVideos array changes. If a user is on page 3 and videos are deleted leaving only 1 page of videos, they'll see an empty page. Add validation to reset currentPage to 0 when it exceeds the new totalPages value.
YouTube Shorts Section Implementation ✅
Successfully implemented a complete YouTube Shorts section for the videos page with TikTok-like functionality.
✅ Completed Features:
Backend (Python/FastAPI):
video_typefield to VideoHighlight model ("regular" or "short")Admin Panel:
Videos Page - Complete Redesign:
🐛 Bug Fixes:
Fix in commit c7524dd:
The video type field was not being properly saved to the database. The issue was in
src/lib/admin-api.ts:getVideos()- Now mapsvideo_typefrom backend response tovideoTypein frontendcreateVideo()- Now sendsvideo_typefield in the payload to backendupdateVideo()- Now sendsvideo_typefield in the payload to backendThe frontend was using camelCase
videoTypebut wasn't converting it to snake_casevideo_typefor the backend API.Technical Highlights:
Security & Quality:
Browser Compatibility:
Testing Notes:
To test this implementation:
video_type: "short"in the databaseThe bug is now fixed and the implementation is complete! 🎉
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.