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.

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
43 changes: 34 additions & 9 deletions docs/guides/skins.md
Original file line number Diff line number Diff line change
@@ -1,29 +1,54 @@
Skins
=====

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! That means you can build a custom skin by simply taking advantage of the cascading aspect of CSS and overriding
## 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!
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 = false` before Video.js is loaded. Keep in mind that without these base styles
enabled, you'll need to manually include them.
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.
Copy link
Member Author

Choose a reason for hiding this comment

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

I changed window.VIDEOJS_NO_BASE_THEME = false to window.VIDEOJS_NO_BASE_THEME = true because that's how it's implemented and that's what the name implies.

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 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 some cases, like if a user has the HEAD tag managed by React, users do not want this.
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>
```

### `Player#width` and `Player#height`
When `VIDEOJS_NO_DYNAMIC_STYLE` is set, `Player#width` and `Player#height` will apply any width and height
that is set directly to the video element (or whatever element the current tech uses).


## Icons

You can view all of the icons available in the base theme by renaming and viewing [`icons.html.example`](https://github.com/videojs/video.js/blob/master/sandbox/icons.html.example) in the sandbox directory.
You can view all of the icons available in the base theme by renaming and viewing
[`icons.html.example`](https://github.com/videojs/video.js/blob/master/sandbox/icons.html.example) in the sandbox directory.

## Customization

When you create a new skin, the easiest way to get started is to simply override the base Video.js theme. You should include a new class matching the
name of your theme, then just start overriding!
When you create a new skin, the easiest way to get started is to simply override the base Video.js theme.
You should include a new class matching the name of your theme, then just start overriding!

```css
.vjs-skin-hotdog-stand { color: #FF0000; }
.vjs-skin-hotdog-stand .vjs-control-bar { background: #FFFF00; }
.vjs-skin-hotdog-stand .vjs-play-progress { background: #FF0000; }
```

This would take care of the major areas of the skin (play progress, the control bar background, and icon colors), but you can skin any other aspect.
Our suggestion is to use a browser such as Firefox and Chrome, and use the developer tools to inspect the different elements and see what you'd like to change and what classes
This would take care of the major areas of the skin (play progress, the control bar background, and icon colors),
but you can skin any other aspect.
Our suggestion is to use a browser such as Firefox and Chrome,
and use the developer tools to inspect the different elements and see what you'd like to change and what classes
to target when you do so.

More custom skins will be available for download soon. If you have one you like you can share it by forking [this example on CodePen.io](http://codepen.io/heff/pen/EarCt), and adding a link on the [Skins wiki page](https://github.com/videojs/video.js/wiki/Skins).
More custom skins will be available for download soon.
If you have one you like you can share it by forking [this example on CodePen.io](http://codepen.io/heff/pen/EarCt),
and adding a link on the [Skins wiki page](https://github.com/videojs/video.js/wiki/Skins).
27 changes: 23 additions & 4 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,12 @@ class Player extends Component {
// Add a style element in the player that we'll use to set the width/height
// of the player in a way that's still overrideable by CSS, just like the
// video element
this.styleEl_ = stylesheet.createStyleElement('vjs-styles-dimensions');
let defaultsStyleEl = Dom.$('.vjs-styles-defaults');
let head = Dom.$('head');
head.insertBefore(this.styleEl_, defaultsStyleEl ? defaultsStyleEl.nextSibling : head.firstChild);
if (window.VIDEOJS_NO_DYNAMIC_STYLE !== true) {
this.styleEl_ = stylesheet.createStyleElement('vjs-styles-dimensions');
let defaultsStyleEl = Dom.$('.vjs-styles-defaults');
let head = Dom.$('head');
head.insertBefore(this.styleEl_, defaultsStyleEl ? defaultsStyleEl.nextSibling : head.firstChild);
}

// Pass in the width/height/aspectRatio options which will update the style el
this.width(this.options_.width);
Expand Down Expand Up @@ -423,6 +425,23 @@ class Player extends Component {
* @method updateStyleEl_
*/
updateStyleEl_() {
if (window.VIDEOJS_NO_DYNAMIC_STYLE === true) {
const width = typeof this.width_ === 'number' ? this.width_ : this.options_.width;
const height = typeof this.height_ === 'number' ? this.height_ : this.options_.height;
let techEl = this.tech_ && this.tech_.el();

if (techEl) {
if (width >= 0) {
techEl.width = width;
}
if (height >= 0) {
techEl.height = height;
}
}

return;
}

let width;
let height;
let aspectRatio;
Expand Down
32 changes: 18 additions & 14 deletions src/js/video.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* @file video.js
*/
import window from 'global/window';
import document from 'global/document';
import * as setup from './setup';
import * as stylesheet from './utils/stylesheet.js';
Expand Down Expand Up @@ -99,21 +100,24 @@ let videojs = function(id, options, ready){
};

// Add default styles
let style = Dom.$('.vjs-styles-defaults');
if (!style) {
style = stylesheet.createStyleElement('vjs-styles-defaults');
let head = Dom.$('head');
head.insertBefore(style, head.firstChild);
stylesheet.setTextContent(style, `
.video-js {
width: 300px;
height: 150px;
}
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');
head.insertBefore(style, head.firstChild);
stylesheet.setTextContent(style, `
.video-js {
width: 300px;
height: 150px;
}

.vjs-fluid {
padding-top: 56.25%
}
`);
.vjs-fluid {
padding-top: 56.25%
}
`);
}
}

// Run Auto-load players
Expand Down
54 changes: 54 additions & 0 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -951,3 +951,57 @@ test('Remove waiting class on timeupdate after tech waiting', function() {
player.trigger('timeupdate');
ok(!/vjs-waiting/.test(player.el().className), 'vjs-waiting is removed from the player el on timeupdate');
});

test('Make sure that player\'s style el respects VIDEOJS_NO_DYNAMIC_STYLE option', function() {
// clear the HEAD before running this test
let styles = document.querySelectorAll('style');
let i = styles.length;
while (i--) {
let style = styles[i];
style.parentNode.removeChild(style);
}

let tag = TestHelpers.makeTag();
tag.id = 'vjs-no-base-theme-tag';
tag.width = 600;
tag.height = 300;

window.VIDEOJS_NO_DYNAMIC_STYLE = true;
let player = TestHelpers.makePlayer({}, tag);
styles = document.querySelectorAll('style');
equal(styles.length, 0, 'we should not get any style elements included in the DOM');

window.VIDEOJS_NO_DYNAMIC_STYLE = false;
player = TestHelpers.makePlayer({}, tag);
styles = document.querySelectorAll('style');
equal(styles.length, 1, 'we should have one style element in the DOM');
equal(styles[0].className, 'vjs-styles-dimensions', 'the class name is the one we expected');
});

test('When VIDEOJS_NO_DYNAMIC_STYLE is set, apply sizing directly to the tech el', function() {
// clear the HEAD before running this test
let styles = document.querySelectorAll('style');
let i = styles.length;
while (i--) {
let style = styles[i];
style.parentNode.removeChild(style);
}

let tag = TestHelpers.makeTag();
tag.id = 'vjs-no-base-theme-tag';
tag.width = 600;
tag.height = 300;

window.VIDEOJS_NO_DYNAMIC_STYLE = true;
let player = TestHelpers.makePlayer({}, tag);

player.width(300);
player.height(600);
equal(player.tech_.el().width, 300, 'the width is equal to 300');
equal(player.tech_.el().height, 600, 'the height is equal 600');

player.width(600);
player.height(300);
equal(player.tech_.el().width, 600, 'the width is equal to 600');
equal(player.tech_.el().height, 300, 'the height is equal 300');
});