feat: migrate video shorts to learning resources API#3361
Open
daniellefrappier18 wants to merge 6 commits into
Open
feat: migrate video shorts to learning resources API#3361daniellefrappier18 wants to merge 6 commits into
daniellefrappier18 wants to merge 6 commits into
Conversation
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the Home Page “Video Shorts” feature from the legacy v0 VideoShort API shape to the v1 Learning Resources VideoResource shape, including switching playback to video.streaming_url.
Changes:
- Replace v0
useVideoShortsListusage in the Home Page section with a v1 Learning Resources search-based hook. - Update the Video Shorts modal to consume
VideoResourceand playvideo.video.streaming_urlwith revised portrait sizing. - Update modal tests to use v1 factories/types and validate
streaming_urlhandling.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontends/main/src/app-pages/HomePage/VideoShortsSection.tsx | Swaps data source to v1 VideoResource list and updates thumbnail rendering + keys. |
| frontends/main/src/app-pages/HomePage/VideoShortsModal.tsx | Switches modal playback to streaming_url and adjusts slide sizing/styling. |
| frontends/main/src/app-pages/HomePage/VideoShortsModal.test.tsx | Updates tests to v1 VideoResource factories and adjusts assertions for new fields. |
| frontends/api/src/hooks/videoShorts/index.ts | Adds a new hook that fetches Video Shorts via Learning Resources search and filters to videos. |
Comments suppressed due to low confidence (1)
frontends/main/src/app-pages/HomePage/VideoShortsModal.tsx:189
video.video.streaming_urlis documented/produced as an HLS.m3u8URL (see backend help_text/tests). A plain<video src="...m3u8">will not play on many browsers (notably Chrome/Firefox) without an HLS-capable player (e.g., video.js/hls.js) or a non-HLS fallback. Consider reusing the existingVideoJsPlayer/VideoResourcePlayer+resolveVideoSourcespattern used elsewhere inmainso HLS works cross-browser, rather than relying on native<video>.
<Video
ref={refCallback}
onClick={onVideoClick}
src={src}
autoPlay
muted
playsInline
webkit-playsinline="true"
controlsList="nofullscreen"
disablePictureInPicture
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
frontends/main/src/app-pages/HomePage/VideoShortsModal.tsx:196
video.video?.streaming_urlwill often be an HLS.m3u8(per existing VideoResource playback paths). Rendering it directly in a native<video src=...>will fail to play on browsers without native HLS support (e.g., Chrome/Firefox). Consider reusing the existingVideoJsPlayer/VideoResourcePlayer+resolveVideoSourcesapproach (or an HLS polyfill) so Video Shorts are playable cross-browser.
<Video
ref={refCallback}
onClick={onVideoClick}
src={src}
autoPlay
muted
playsInline
webkit-playsinline="true"
controlsList="nofullscreen"
disablePictureInPicture
width={videoWidth}
height={videoHeight}
preload="metadata"
loop
/>
Comment on lines
155
to
186
| @@ -172,7 +180,7 @@ const VideoWithErrorHandler = ({ | |||
| <Video | |||
| ref={refCallback} | |||
| onClick={onVideoClick} | |||
| src={`${NEXT_PUBLIC_ORIGIN}${video.video_url}`} | |||
| src={src} | |||
| autoPlay | |||
| muted | |||
| playsInline | |||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/11036
Description (What does it do?)
This branch migrates Video Shorts from the old v0 API shape to the v1 learning-resources API shape. The modal now uses VideoResource and plays video.streaming_url instead of the old direct MP4 URL.
Known Issue
Video Shorts can look soft when playback first begins in the modal.
Why It’s Happening
The old implementation played a direct MP4. The new implementation plays an HLS streaming_url.
The v1 API provides Video Shorts as HLS streams via .m3u8 streaming_url rather than direct MP4 files. An .m3u8 URL is an HLS playlist, not a single video file, so the browser now performs adaptive streaming selection during playback. In testing, videos start on a low HLS rendition and then become sharper after a short period, which suggests native HLS is choosing a low-quality variant for fast startup and switching up once playback stabilizes. If we want to avoid that behavior entirely, we would need a non-HLS source, such as a direct MP4, instead of the current .m3u8
Because the modal presents the video in a large portrait treatment, the low startup rendition is especially noticeable.
Possible Solutions
Long term, the best fix is to improve the video assets delivered by OVS, either by generating better renditions or by providing portrait-oriented versions of these shorts. The frontend is currently displaying the stream it receives, and the remaining softness appears to come from the quality and shape of the delivered HLS media rather than the API integration itself.
Screenshots (if appropriate):
video-shorts.mov
How can this be tested?
Additional Context