Manual Documentation Improvements #3703

misteroneill · 2016-10-23T17:26:56Z

The first step in sweeping the docs/ folder for completeness and accuracy.

This PR focuses on the entry-level guides and the README. It also features a few removals that have come from various conversations. And it removes the API guide because it was duplicating the setup guide.

Requirements Checklist

Feature implemented / Bug fixed
Reviewed by Two Core Contributors

gkatsev

Reviewed README.md

gkatsev · 2016-10-26T16:03:15Z

README.md

+    preload="auto"
+    poster="//vjs.zencdn.net/v/oceans.png"
+    data-setup='{}'>
+  <source src="//vjs.zencdn.net/v/oceans.mp4" type="video/mp4"></source>


I'd be inclined to leave the old fake urls.

Why is that? I feel like this gives people something they could copy/paste and see it working.

I think using real urls is good, we just need to make sure we think about the docs if any of these urls change in the future.

They shouldn't, they're on the CDN.

gkatsev · 2016-10-26T16:07:19Z

README.md

+Video.js is a free and open source library, and we appreciate any help you're willing to give - whether it's fixing bugs, improving documentation, or suggesting new features. Check out the [contributing guide][contributing] for more!
+
+_Video.js uses [BrowserStack][browserstack] for compatibility testing._
+
 ## License


Should we link the heading too? Makes it much more visible as well.

gkatsev

options.md

gkatsev · 2016-10-26T16:10:59Z

docs/guides/options.md

 ```html
 <video controls ...>
+<video ...>


should we also show <video loop="loop">? As that's a common thing to do as well.

gkatsev · 2016-10-26T16:14:34Z

docs/guides/options.md

 ```

-> The biggest issue people run into is trying to set these values to false using false as the value (e.g. controls="false") which actually does the opposite and sets the value to true because the attribute is still included. If you need the attribute to include an equals sign for XHTML validation, you can set the attribute's value to the same as its name (e.g. controls="controls").
+### Precedence


While there sort of is a precedence here, I'm not sure we actually officially support it.

This seems relevant, though. The sort of thing I'd want to know without having to experiment for myself. I feel like it's the implicit reality.

I'm not comfortable with documenting a behavior we haven't been testing with, whether it's there implicitly or not.

Fair enough.

gkatsev · 2016-10-26T16:15:10Z

docs/guides/options.md


+### Standard `<video>` Element Options


Can we link to MDN or something here as well, for more info?

gkatsev · 2016-10-26T16:16:11Z

docs/guides/options.md

-or
-{ "preload": "auto" }
-```
+> **Note:** As of iOS 10, Apple offers `autoplay` support in Safari. However, in order for it to work, the `<video>` element must have `muted` and `playsinline` attributes!


If it doesn't have audio tracks it also works.
We should also double-check whether playsinline is required.

Good call. Rather than outlining how it works here, I'm going to link to the WebKit blog post.

gkatsev · 2016-10-26T16:18:34Z

docs/guides/options.md

+
+> Type: `Boolean`
+
+When `true`, the Video.js player will have a fluid size. In other words, it will scale to fit its container.


Can we add something like:

If a class of vjs-fluid is added to the player before initialization, this option is automatically set.

gkatsev · 2016-10-26T16:19:18Z

docs/guides/options.md

+
+When `true`, the Video.js player will have a fluid size. In other words, it will scale to fit its container.
+
+#### `inactivityTimeout`


0 means that the user will never become inactive.

gkatsev · 2016-10-26T16:21:21Z

docs/guides/options.md

+
+> Type: `Boolean`
+
+Tells Video.js to prefer the order of [`sources`](#sources) over [`techOrder`](#techOrder) in selecting a source and playback tech.


Should we add an example?

+1 This can be confusing

gkatsev

setup.md

gkatsev · 2016-10-26T16:26:37Z

docs/guides/setup.md

 ```

-\* If you have trouble playing back content you know is in the [correct format](http://blog.zencoder.com/2013/09/13/what-formats-do-i-need-for-html5-video/), your HTTP server might not be delivering the content with the correct [MIME type](http://en.wikipedia.org/wiki/Internet_media_type#Type_video). Please double check your content's headers before opening an [issue](/CONTRIBUTING.md).
+Regardless, in either case, the `data-setup` attribute is still supported for providing [options][options] to Video.js. Additionally, the `videojs` function can take options as its second argument. [Learn more about Video.js options][options].


I'm not sure this is strictly true.

Based on the getTagSettings function and the Player constructor it is...

gkatsev · 2016-10-26T16:28:19Z

docs/guides/setup.md


-We include a stripped down Google Analytics pixel that tracks a random percentage (currently 1%) of players loaded from the CDN. This allows us to see (roughly) what browsers are in use in the wild, along with other useful metrics such as OS and device. If you'd like to disable analytics, you can simply include the following global **before** including Video.js:


this section should be kept.

Ah, yes, I thought the GA thing was in the "Getting Started" document, but it seems not.

Actually, I think we should move this to the "Getting Started" guide. It's silly to have two documents covering the same thing.

gkatsev · 2016-10-26T16:29:09Z

docs/guides/setup.md


-### Self Hosted. ###


We should keep this section as well.

~~There's a "build-your own" section in the CONTRIBUTING.md, we could potentially just link to that.~~
Ignore this one. It's not relevant.

Another thing I thought was covered by "Getting Started"... will bring it back with accurate information!

Actually, I think we should move this to the "Getting Started" guide. It's silly to have two documents covering the same thing.

gkatsev

index.md

gkatsev · 2016-10-26T16:31:25Z

docs/index.md


-<h1>Video.js Documentation</h1>
+There are two categories of docs: [Guides][guides] and [API docs][api].


what about having API Docs link to the second below instead?

I think that's fine for now, but I wonder if we need the API guide at all.

mister-ben · 2016-10-26T20:55:23Z

docs/guides/options.md

+
+> Type: `String`
+
+A [two-letter code][lang-codes] matching one of the available languages in the player. This sets the initial language for a player, but it can always be changed.


Not necessarily two-letter. And while it's true the player's language can be changed later on, that can lead to confusion as localisable text does not get updated.

Good point 👍

brandonocasey · 2016-11-14T17:39:21Z

General comments for this file.

We should use casing as defined in eslint to be consistent with jsdoc comments and JavaScript itself
Options should probably have a Type: and an Example
We may want to specify at the bottom of each guide:
"See a typo? Does something not make sense or seem incorrect? video.js is an Open Source project on GitHub. Feel free to open a PR or issue on video.js to fix this documentation!"

brandonocasey · 2016-11-14T16:44:02Z

README.md

-```javascript
-var player = videojs('really-cool-video', { /* Options */ }, function() {
-  console.log('Good to go!');
+If you don't want to use automatic setup, you can leave off the `data-setup` attribute and initialize a `<video>` element manually:


maybe it would be good to specify "using the main videojs function like this:"

brandonocasey · 2016-11-14T16:45:37Z

README.md


-  this.play(); // if you don't trust autoplay for some reason
+```js
+var player = videojs('my-player', { /* options */ }, function() {


A few things here:

it might be nice to link to the API docs for videojs and Player here. We may want to think about it when we know where the API docs will end up? Or maybe we add the old link now and update it later?

it might be nice for new users to specify what that last function is somehow, maybe through a function name or a comment? ie function playerReadyFunction()

It would seem more clear to me if we defined options object above the videojs call so that we don't need to have a weird { /* options */ } comment-object thing.

It would also be a good idea to specify that this in the ready function is a reference to the Player that was created and is now ready for playback.

It would probably be a good idea to have a second example without options/readyFunction as they are optional.

brandonocasey · 2016-11-14T16:52:57Z

docs/guides/options.md

+  - [`notSupportedMessage`](#notsupportedmessage)
+  - [`plugins`](#plugins)
+  - [`sourceOrder`](#sourceorder)
+  - [`sources`](#sources)


Some options I found in the code, idk if we want to add them, but here they are:

Player

vtt.js

starttime. should this actually be startTime?

createEl

initChildren

reportTouchActivity

Tech

nativeControlsForTouch

starttime. should this actually be startTime?

nativeCaptions and nativeTextTracks

audioTracks, videoTracks, textTracks

Do we want to specify anything from the default options? (Seems like some of these may not even exist anymore...)

Yeah, with regards to the default options, like I said above, I left out things that don't appear to be used.

vtt.js - Adding.

starttime - I think starttime was assuming it was coming from an attribute like starttime="1". Probably old code and probably not worth documenting.

createEl, initChildren, reportTouchActivity - Wouldn't make sense to document since it's always set to false explicitly. And I think its existence is a bit of a hack to begin with.

nativeControlsForTouch - Adding here and also under tech options.

starttime - Addressed above.

nativeCaptions and nativeTextTracks - Adding under tech options.

*Tracks - These aren't available as options are they?

Thinking back on it now*Tracks can be a passed in *TrackList, but we may not want to document it since the way we are using it (as a way to share tracks between techs and player) is not very standard.

The weird thing with start time is that it has two separate names in the code. starttime and startTime maybe we need another investigation to remove or fix it

Let's leave the tracks options off if we don't want to publicize it. Also, we should figure out the starttime thing. It's not a standard attribute, so I suspect it's legacy and could probably be removed.

brandonocasey · 2016-11-14T16:56:19Z

docs/guides/options.md

+- [Component Options](#component-options)
+  - [`children`](#children-1)
+  - [`${componentName}`](#componentname)
+- [Tech Options](#tech-options)


I think it would be good to have a section for source handler options under tech options.

You mean that the source handlers get the tech options as an argument?

This is what I mean:

var sourceHandleOptions = {something:true}; videojs({ html5: {hls: sourceHandlerOptions}, flash: {hls: sourceHandlerOptions}, });

Hmm, well, that's what I'm saying, right? {hls: sourceHandlerOptions} gets passed through to source handlers. Or does HLS do some magic that's not in Video.js proper?

hmmm seems like your right sourceHandlers do get all of techs options and not just specific source handler options. I guess maybe just specify that in the Tech options section?

brandonocasey · 2016-11-14T17:04:33Z

docs/guides/options.md

+- [Standard `<video>` Element Options](#standard-video-element-options)
+  - [`autoplay`](#autoplay)
+  - [`controls`](#controls)
+  - [`height`](#height)


Looking at the spec here we may be missing (or maybe we have not implemented them):

playbackRate

defaultPlaybackRate

volume

defaultMuted

I didn't list things that don't appear to be supported in the code as options.

I will add an issue to see if we want to support them

Yeah, that'd probably be a 6.0 thing and should probably not block this PR.

brandonocasey · 2016-11-14T17:53:07Z

docs/guides/setup.md

+
+```html
+<video controls ...>
+<video loop="loop" ...>


is this correct? I think the way advised above says not to use boolean attributes like this (even though it will work in this case)

Yes, those are the two valid ways to create a boolean attribute. I'll clarify the text above to mention this.

brandonocasey · 2016-11-14T17:55:10Z

docs/guides/setup.md

+});
+```
+
+In each case, the callback is called asynchronously - _even if the player is already ready!_


Just a general question here, not really about the docs, should we support all three use cases?

Good question, but one that I think is outside the scope of this PR. :)

Also isn't there an option to make this synchronous somehow?

We don't have any way to make it synchronous. It was removed in 5.0, since previously, it depended on what the tech did, so, html5 was sync but flash was async. Now it's always async. However, #3782 makes sense. I think we may not want all 3, but maybe two of them.

Don't forget, the Component#ready function can take the second sync argument, which I recall was a point of contention early in the 5.x process.

yeah thats the one that I am talking about, since player.ready can be synchronous should we mention that here?

I did forget. And everyone should also forget :P

I think it should remain undocumented at best.

brandonocasey · 2016-11-14T17:59:53Z

docs/index.md


-* [API](./guides/api.md) - The Video.js API allows you to control the video through javascript or trigger event listeners, whether the video is playing through HTML5, flash, or another playback technology.
+Tracks are used for displaying text information over a video, selecting different audio tracks for a video, or selecting different video tracks.


if we have a video track guide, we may want to specify that video tracks are untested and may not work.

I may or may not have changed that in the tracks work. I'll have to check.

brandonocasey · 2016-11-14T18:00:25Z

docs/index.md


-* [Skins](./guides/skins.md) - You can change the look of the player across playback technologies just by editing a CSS file. The skins documentation gives you a intro to how the HTML and CSS of the default skin is put together.
+### Customizing


Should we add a guide to Components?

brandonocasey · 2016-11-14T18:01:08Z

docs/index.md


-### Resources
+You can package up interesting Video.js customizations and reuse them elsewhere. Find out how to build your own plugin or use one created by someone else.


I think that it would be good to add a link to the wiki were all the plugins are listed.

misteroneill · 2016-11-14T19:01:33Z

The concern I have with adding an example for each option is that it becomes extremely redundant and verbose. The documentation for how to set options is in setup.md and there is a blurb at the top of each section explaining commonalities of options in that section (for example, the ability for autoplay to be set via attribute or option).

brandonocasey · 2016-11-14T19:04:44Z

My thinking was not an example for each option, but more like an example value for anything that is not a boolean. Such as aspectRatio or width.

misteroneill · 2016-11-14T19:44:36Z

Gotcha. I'll see if there's anything that warrants example(s).

brandonocasey · 2016-11-14T20:55:55Z

README.md


-> Video.js is a web video player built from the ground up for an HTML5 world. It supports HTML5 and Flash video, as well as YouTube and Vimeo (through [plugins](https://github.com/videojs/video.js/wiki/Plugins)). It supports video playback on desktops and mobile devices. This project was started mid 2010, and the player is now used on over ~~50,000~~ ~~100,000~~ 200,000 websites.
+> Video.js is a web video player built from the ground up for an HTML5 world. It supports HTML5 and Flash video, as well as YouTube and Vimeo (through [plugins][plugins]). It supports video playback on desktops and mobile devices. This project was started mid 2010, and the player is now used on over ~~50,000~~ ~~100,000~~ ~~200,000~~ 400,000 websites.


I think this comment was lost before, I am adding it back in here:

I think we should link to https://trends.builtwith.com/media/VideoJS here so that it i apparent that we are not just making a number up.

gkatsev · 2016-11-29T19:23:38Z

docs/guides/options.md

-> ### Note on Video Tag Attributes ###
-> With HTML5 video tag attributes that can only be true or false (boolean), you simply include the attribute (no equals sign) to turn it on, or exclude it to turn it off. For example, to turn controls on:
+### `sourceOrder`
+> Type: `Array`


sourceOrder is a boolean.

Also, should we mention now that in 6.0 it will "default" to true, whatever that means in the world of middleware.

and it defaults to false right now.

gkatsev · 2016-11-29T19:26:29Z

docs/guides/options.md


-'metadata': Load only the meta data of the video, which includes information like the duration and dimensions of the video.
+Allows overriding the default URL to vtt.js, which may be loaded asynchronously to polyfill support for `WebVTT`.


We should add a note that it'll be used in the novtt build of videojs.

gkatsev · 2016-11-29T19:27:28Z

docs/guides/options.md

 ```

-The [components guide](./components.md) has an excellent breakdown of the structure of a player, you
-just need to remember to nest child components in a `children` array for each level.
+#### `nativeCaptions`


This is an alias to nativeTextTracks. I think it should be mentioned below instead of having its own section.

misteroneill · 2016-12-02T15:21:43Z

Since all my sub-PRs (in my fork) were approved, I have merged all 5 branches into one and rebased on master. This should be good to go. 👍

misteroneill added the documentation label Oct 23, 2016

gkatsev reviewed Oct 26, 2016

View reviewed changes

mister-ben reviewed Oct 26, 2016

View reviewed changes

misteroneill force-pushed the manual-docs branch from 9a95f3c to de03392 Compare October 31, 2016 14:04

misteroneill changed the title ~~Manual Documentation Improvements (WIP)~~ Manual Documentation Improvements - Part 1 Oct 31, 2016

misteroneill added the needs: LGTM Needs one or more additional approvals label Nov 11, 2016

misteroneill force-pushed the manual-docs branch from cc6d7f7 to bceba84 Compare November 14, 2016 16:45

brandonocasey requested changes Nov 14, 2016

View reviewed changes

gkatsev modified the milestone: 5.14 Nov 15, 2016

brandonocasey approved these changes Nov 15, 2016

View reviewed changes

brandonocasey added confirmed and removed needs: LGTM Needs one or more additional approvals labels Nov 15, 2016

misteroneill changed the title ~~Manual Documentation Improvements - Part 1~~ Manual Documentation Improvements Nov 21, 2016

gkatsev requested changes Nov 29, 2016

View reviewed changes

gkatsev approved these changes Nov 30, 2016

View reviewed changes

misteroneill added 6 commits December 2, 2016 10:19

Rewrite setup.md

62d9913

Rewrite README

948897e

Rewrite index.md

97db487

Rewrite options.md

835e4fe

ToCs

f9c01a4

Changes due to feedback

adf3197

misteroneill added 10 commits December 2, 2016 10:19

Reshuffle options/setup; remove api/glossary

f8d9afc

Feedback changes

bdef60a

Add link to builtwith

0ab9f86

Address code review comments

ed0022e

tracks index

13a0eec

Update tracks guides

d219575

Address PR concerns

0410954

Move removing players to workflows guide

79a6c30

Address some comments, add placeholders at bottom

0a6ccbd

Mostly complete components doc

e71c7a6

misteroneill force-pushed the manual-docs branch from 60e0355 to a36b640 Compare December 2, 2016 15:20

misteroneill added 4 commits December 2, 2016 10:21

Minor reorganization

ca8e613

Update languages.md

5cfdcad

Minor formatting cleanup

e6a964a

Skins guide and icons sandbox example update

a36b640

gkatsev merged commit d24fe40 into videojs:master Dec 2, 2016

misteroneill deleted the manual-docs branch December 2, 2016 19:55


		> Type: `Boolean`

		When `true`, the Video.js player will have a fluid size. In other words, it will scale to fit its container.


		When `true`, the Video.js player will have a fluid size. In other words, it will scale to fit its container.

		#### `inactivityTimeout`


		> Type: `Boolean`

		Tells Video.js to prefer the order of [`sources`](#sources) over [`techOrder`](#techOrder) in selecting a source and playback tech.


		We include a stripped down Google Analytics pixel that tracks a random percentage (currently 1%) of players loaded from the CDN. This allows us to see (roughly) what browsers are in use in the wild, along with other useful metrics such as OS and device. If you'd like to disable analytics, you can simply include the following global before including Video.js:


		<h1>Video.js Documentation</h1>
		There are two categories of docs: [Guides][guides] and [API docs][api].


		> Type: `String`

		A [two-letter code][lang-codes] matching one of the available languages in the player. This sets the initial language for a player, but it can always be changed.


		* [API](./guides/api.md) - The Video.js API allows you to control the video through javascript or trigger event listeners, whether the video is playing through HTML5, flash, or another playback technology.
		Tracks are used for displaying text information over a video, selecting different audio tracks for a video, or selecting different video tracks.


		* [Skins](./guides/skins.md) - You can change the look of the player across playback technologies just by editing a CSS file. The skins documentation gives you a intro to how the HTML and CSS of the default skin is put together.
		### Customizing


		### Resources
		You can package up interesting Video.js customizations and reuse them elsewhere. Find out how to build your own plugin or use one created by someone else.


		> Video.js is a web video player built from the ground up for an HTML5 world. It supports HTML5 and Flash video, as well as YouTube and Vimeo (through [plugins](https://github.com/videojs/video.js/wiki/Plugins)). It supports video playback on desktops and mobile devices. This project was started mid 2010, and the player is now used on over ~~50,000~~ ~~100,000~~ 200,000 websites.
		> Video.js is a web video player built from the ground up for an HTML5 world. It supports HTML5 and Flash video, as well as YouTube and Vimeo (through [plugins][plugins]). It supports video playback on desktops and mobile devices. This project was started mid 2010, and the player is now used on over ~~50,000~~ ~~100,000~~ ~~200,000~~ 400,000 websites.


		'metadata': Load only the meta data of the video, which includes information like the duration and dimensions of the video.
		Allows overriding the default URL to vtt.js, which may be loaded asynchronously to polyfill support for `WebVTT`.

Manual Documentation Improvements #3703

Manual Documentation Improvements #3703

Conversation

misteroneill commented Oct 23, 2016 • edited Loading

Requirements Checklist

gkatsev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev Oct 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandonocasey commented Nov 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Player

Tech

Choose a reason for hiding this comment

misteroneill Nov 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

misteroneill commented Oct 23, 2016 •

edited

Loading

gkatsev Oct 26, 2016 •

edited

Loading

brandonocasey commented Nov 14, 2016 •

edited

Loading

misteroneill Nov 14, 2016 •

edited

Loading