-
Notifications
You must be signed in to change notification settings - Fork 22.8k
Internalize custom video player example #40915
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
Conversation
Preview URLs (6 pages)
External URLs (3)URL:
(comment last updated: 2025-09-02 15:39:56) |
.../media/guides/audio_and_video_delivery/adding_captions_and_subtitles_to_html5_video/index.md
Show resolved
Hide resolved
preload="metadata" | ||
poster="/shared-assets/images/examples/tears-of-steel-battle-clip-medium-poster.jpg"> | ||
<source | ||
src="/shared-assets/videos/tears-of-steel-battle-clip-medium.mp4" |
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 20MB, so it takes a while to load. The flower example is 1MB, so this is faster, but maybe less exciting. Plus we have some more features being shown here. Follow-up could be a loading spinner / some feedback indicator that something's happening and playback will start when the resource is ready.
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.
Yeah that's a good idea. A bit tricky to do though so perhaps a follow up.
files/en-us/web/media/guides/audio_and_video_delivery/video_player_styling_basics/index.md
Show resolved
Hide resolved
Thanks, Josh. A couple of comments for you to check π |
.../media/guides/audio_and_video_delivery/adding_captions_and_subtitles_to_html5_video/index.md
Show resolved
Hide resolved
files/en-us/web/media/guides/audio_and_video_delivery/cross_browser_video_player/index.md
Show resolved
Hide resolved
files/en-us/web/media/guides/audio_and_video_delivery/video_player_styling_basics/index.md
Show resolved
Hide resolved
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.
Thanks Josh! Couple of small things to check, but ready to go when you've had a look π
Thanks @bsmth for the thorough reviews! |
}); | ||
volDec.addEventListener("click", (e) => { | ||
alterVolume("-"); | ||
}); |
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.
Oh good, that's all we were missing ππ»
Fix #9923. Still waiting on mdn/shared-assets#62, and then I need to test to fix the actual issue.