Skip to content
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.

Sign up for GitHub

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

Don't add style elements if VIDEOJS_NO_DYNAMIC_STYLE is set to true #3093

Closed
wants to merge 12 commits into from
Closed

Don't add style elements if VIDEOJS_NO_DYNAMIC_STYLE is set to true #3093

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
45ef924
Don't add style elements if VIDEOJS_NO_BASE_THEME is set to true
gkatsev Feb 9, 2016
cdd631a
Add player tests to verify VIDEOJS_NO_BASE_THEME
gkatsev Feb 10, 2016
3ffd8c3
Add section about default style elements in vjs
gkatsev Feb 10, 2016
22f3788
Fixup markdown for skins guide
gkatsev Feb 10, 2016
9e570ff
rename to VIDEOJS_NO_DYNAMIC_STYLE
gkatsev Mar 14, 2016
7793956
apply width/height to tech attributes if VIDEOJS_NO_DYNAMIC_STYLE is …
gkatsev Mar 14, 2016
8a7519e
Add a note about Player#width and Player#height
gkatsev Mar 14, 2016
f0cf8b1
Add a unit test for the new width/height functionality
gkatsev Mar 14, 2016
c7d8309
ignore some es6 code for jshint
gkatsev Mar 14, 2016
39bfc62
make sure that values of 0 for width and height can be used
gkatsev Mar 15, 2016
ddb97bf
code review comments
gkatsev Mar 15, 2016
23dbaf3
Add an example of a case
gkatsev Mar 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions docs/guides/skins.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,23 @@ Skins

## Base Skin
The base Video.js skin is made using HTML and CSS (although we use the [Sass preprocessor](http://sass-lang.com)),
and by default these styles are added to the dom for you!
and by default these styles are added to the DOM for you!
That means you can build a custom skin by simply taking advantage of the cascading aspect of CSS and overriding
the styles you'd like to change.

If you don't want Video.js to inject the base styles for you, you can disable it by setting `window.VIDEOJS_NO_BASE_THEME = true` before Video.js is loaded.
Keep in mind that without these base styles enabled, you'll need to manually include them.

Video.js does not currently include the base skin automatically yet, so, this option isn't necessary.

## Default style elements
Video.js uses a couple of dynamically, specifically, there's a default styles element as well as a player dimensions style element.
Video.js uses a couple of style elements dynamically, specifically, there's a default styles element as well as a player dimensions style element.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

styles element => style element?

#donthitme

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I think both are technically correct though a bit ambiguous. I was going for ((default styles) element) as opposed to a (default (style element)).
Maybe I should just change it to default styles style element?

They are used to provide extra default flexiblity with styling the player. However, in a lot of cases, users do not want this.
When `window.VIDEOJS_NO_DYNAMIC_STYLE` is set to `true`, videojs will *not* include these element in the page.
When `window.VIDEOJS_NO_DYNAMIC_STYLE` is set to `true`, video.js will *not* include these element in the page.
This means that default dimensions and configured player dimensions will *not* be applied.
For example, the following player will end up having a width and height of 0 when initialized if `window.VIDEOJS_NO_DYNAMIC_STYLE === true`:
```html
<video width=600 height=300></video>
<video width="600" height="300"></video>
```

### `Player#width` and `Player#height`
Expand Down
1 change: 1 addition & 0 deletions src/js/video.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ let videojs = function(id, options, ready){
// Add default styles
if (window.VIDEOJS_NO_DYNAMIC_STYLE !== true) {
let style = Dom.$('.vjs-styles-defaults');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the linter will complain that these let statements are not followed by an empty line.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, though, videojs itself doesn't use vjsstandard yet

>.>
<.<


if (!style) {
style = stylesheet.createStyleElement('vjs-styles-defaults');
let head = Dom.$('head');
Expand Down