-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
WebVTT - general restructure to match normal API docs #34034
Conversation
Preview URLs (17 pages)
Flaws (21)Note! 11 documents with no flaws that don't need to be listed. 🎉 URL:
URL:
URL:
URL:
URL:
URL:
External URLs (2)URL:
URL:
(comment last updated: 2024-07-12 06:39:16) |
This pull request has merge conflicts that must be resolved before it can be merged. |
448af34
to
e23b0cb
Compare
1f4466d
to
205ae20
Compare
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.
@hamishwillee quite a few comments here, but mainly grammatical quibbles. Nothing particularly major. I know this is a restructure, but I couldn't help myself ;-)
I'm approving it so as not to hold you up.
@@ -11358,6 +11358,7 @@ | |||
/en-US/docs/Web/CSS/::-webkit-scrollbar-thumb /en-US/docs/Web/CSS/::-webkit-scrollbar | |||
/en-US/docs/Web/CSS/::-webkit-scrollbar-track /en-US/docs/Web/CSS/::-webkit-scrollbar | |||
/en-US/docs/Web/CSS/::-webkit-scrollbar-track-piece /en-US/docs/Web/CSS/::-webkit-scrollbar | |||
/en-US/docs/Web/CSS/::cue-region /en-US/docs/Web/API/WebVTT_API |
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.
I understand why you'd want to delete this, as it has no support anywhere. Estelle and I have started adding notes to spec landing pages to mention items that are specified but not supported anywhere, just in case people go "x is in the spec; why is it not documented?"
So for example:
Note: The specification defines another pseudo-element,
::cue-region
, but this is not supported by any browsers.
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.
@chrisdavidmills Is there a discussion on this? The reason I ask is that I love the idea of making it clear that things are deliberately not documented, but we should harmonize the team approach.
If it were me, we would list them (as though they were defined) in the overview page but be tagged with a macro that applies styling {{no_browser_implementation("::cue-region")}}
to indicate that they aren't supported (linked to info text). This could then be added/removed by BCD possibly.
But there are many options, and maybe a note is the right one.
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.
PS For now I have done as you suggested!
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.
I don't know if there is a discussion anywhere about it. I guess you could ask Estelle. I think it is a reasonable approach for now, and there aren't so many of them that we couldn't change if something else is eventually decided.
The `TextTrack` interface—part of the API for handling WebVTT (text tracks on media presentations)—describes and controls the text track associated with a particular {{HTMLElement("track")}} element. | ||
The `TextTrack` interface of the [WebVTT API](/en-US/docs/Web/API/WebVTT_API) represents a text track associated with a media element. | ||
|
||
The object owns the list of {{domxref("VTTCue")}} objects that will be displayed over the video at various points. |
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.
Seems a bit odd that this page doesn't indicate the relationship between TextTrack
, <track>
/HTMLTrackElement
, and media elements like <video>
/HTMLVideoElement
; it is just implied by an example later on. Maybe add just a sentence to talk about this, e.g.
"A TextTrack
can be added to a media element using the addTrack()
method. This has the same effect as declaratively adding a text track via a <track>
element".
Just give people an idea of how they relate, as it is confusing.
Similar comment applies to the <track>
/HTMLTrackElement
pages and <video>
/HTMLVideoElement
pages.
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.
Very point. I have fixed this up a bit TextTrack, TextTrackList and <track>
now properly cross link.
I added an example of using track in the video element too. It's not something that integrates well, and that video element page needs more love and testing than I am willing to give it.
That set of changes in 6952a0a
files/en-us/web/api/webvtt_api/web_video_text_tracks_format/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/webvtt_api/web_video_text_tracks_format/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/webvtt_api/web_video_text_tracks_format/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/webvtt_api/web_video_text_tracks_format/index.md
Outdated
Show resolved
Hide resolved
files/en-us/web/api/webvtt_api/web_video_text_tracks_format/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
<track> element creation info Track/Video element cross linking
5a42143
to
6952a0a
Compare
files/en-us/web/api/webvtt_api/web_video_text_tracks_format/index.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
### Detecting track addition and removal | ||
|
||
You can detect when tracks are added to and removed from a `<video>` element using the {{domxref("VideoTrackList/addtrack_event", "addtrack")}} and {{domxref("VideoTrackList/removetrack_event", "removetrack")}} events. However, these events aren't sent directly to the `<video>` element itself. Instead, they're sent to the track list object within the `<video>` element's {{domxref("HTMLMediaElement")}} that corresponds to the type of track that was added to the element: | ||
You can detect when tracks are added to and removed from a `<video>` element using the {{domxref("VideoTrackList/addtrack_event", "addtrack")}} and {{domxref("VideoTrackList/removetrack_event", "removetrack")}} events. However, these events aren't sent directly to the `<video>` element itself. |
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.
FYI, I found this whole thing a bit confusing because the track elements dont allow you to add anything other than text tracks to the video/audio. So it's a bit unclear how you'd add new audio tracks (for example).
Thanks for that mammoth review @chrisdavidmills - I have taken almost all of it on board. This is much better than it was. |
- {{domxref("HTMLMediaElement.videoTracks")}} | ||
- : Add an `addtrack` listener to this {{domxref("VideoTrackList")}} object to be informed when video tracks are added to the element. | ||
- : A {{domxref("VideoTrackList")}} containing all of the media element's audio tracks. |
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.
@hamishwillee I did find one small thing here — this line should say "video tracks".
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! Fixed by #34832
The current WebVTT docs include all information in a single doc which mixes the Web API, guide, WebVTT file format, tutorials, etc and has quite a bit of duplication.
This attempts to push the document into a more standard pattern
Note, I did not do this - cannot get Regions to work in FF in any form:
This is part of minimum work to meet minimum docs criteria for #33589