@heff updated the poster to use CSS styles to display; fixed the post…

…er not showing if not originally set. closes #1568

.jshintrc

@@ -25,6 +25,7 @@
         "process",
 
         "PlayerTest",
+        "TestHelpers",
         "asyncTest",
         "deepEqual",
         "equal",

CHANGELOG.md

@@ -7,6 +7,7 @@ CHANGELOG
 * @OlehTsvirko added a Ukrainian translation ([view](https://github.com/videojs/video.js/pull/1562))
 * @OlehTsvirko added a Russian translation ([view](https://github.com/videojs/video.js/pull/1563))
 * @thijstriemstra added a Dutch translation ([view](https://github.com/videojs/video.js/pull/1566))
+* @heff updated the poster to use CSS styles to display; fixed the poster not showing if not originally set ([view](https://github.com/videojs/video.js/pull/1568))
 
 --------------------
 

src/css/video-js.less

@@ -936,6 +936,22 @@ body.vjs-full-window {
   width: 100%;
 }
 
+/* Hide the poster after the video has started playing */
+.video-js.vjs-has-started .vjs-poster {
+  display: none;
+}
+
+/* Don't hide the poster if we're playing audio */
+.video-js.vjs-audio.vjs-has-started .vjs-poster {
+  display: block;
+}
+
+/* Hide the poster when controls are disabled because it's clickable
+    and the native poster can take over */
+.video-js.vjs-controls-disabled .vjs-poster {
+  display: none;
+}
+
 /* Hide the poster when native controls are used otherwise it covers them */
 .video-js.vjs-using-native-controls .vjs-poster {
   display: none;

src/js/component.js

@@ -47,8 +47,14 @@ vjs.Component = vjs.CoreObject.extend({
     // Updated options with supplied options
     options = this.options(options);
 
-    // Get ID from options, element, or create using player ID and unique ID
-    this.id_ = options['id'] || ((options['el'] && options['el']['id']) ? options['el']['id'] : player.id() + '_component_' + vjs.guid++ );
+    // Get ID from options or options element if one is supplied
+    this.id_ = options['id'] || (options['el'] && options['el']['id']);
+
+    // If there was no ID from the options, generate one
+    if (!this.id_) {
+      // Don't require the player ID function in the case of mock players
+      this.id_ = ((player.id && player.id()) || 'no_player') + '_component_' + vjs.guid++;
+    }
 
     this.name_ = options['name'] || null;
 
@@ -976,6 +982,11 @@ vjs.Component.prototype.emitTapEvents = function(){
 vjs.Component.prototype.enableTouchActivity = function() {
   var report, touchHolding, touchEnd;
 
+  // Don't continue if the root player doesn't support reporting user activity
+  if (!this.player().reportUserActivity) {
+    return;
+  }
+
   // listener for reporting that the user is active
   report = vjs.bind(this.player(), this.player().reportUserActivity);
 

src/js/lib.js

@@ -412,6 +412,7 @@ vjs.IS_FIREFOX = (/Firefox/i).test(vjs.USER_AGENT);
 vjs.IS_CHROME = (/Chrome/i).test(vjs.USER_AGENT);
 
 vjs.TOUCH_ENABLED = !!(('ontouchstart' in window) || window.DocumentTouch && document instanceof window.DocumentTouch);
+vjs.BACKGROUND_SIZE_SUPPORTED = 'backgroundSize' in vjs.TEST_VID.style;
 
 /**
  * Apply attributes to an HTML element.

src/js/player.js

@@ -55,9 +55,10 @@ vjs.Player = vjs.Component.extend({
     this.cache_ = {};
 
     // Set poster
-    this.poster_ = options['poster'];
+    this.poster_ = options['poster'] || '';
+
     // Set controls
-    this.controls_ = options['controls'];
+    this.controls_ = !!options['controls'];
     // Original tag settings stored in options
     // now remove immediately so native controls don't flash.
     // May be turned back on by HTML5 tech if nativeControlsForTouch is true
@@ -1284,6 +1285,12 @@ vjs.Player.prototype.poster = function(src){
     return this.poster_;
   }
 
+  // The correct way to remove a poster is to set as an empty string
+  // other falsey values will throw errors
+  if (!src) {
+    src = '';
+  }
+
   // update the internal poster variable
   this.poster_ = src;
 

src/js/poster.js

@@ -12,27 +12,23 @@ vjs.PosterImage = vjs.Button.extend({
   init: function(player, options){
     vjs.Button.call(this, player, options);
 
-    if (player.poster()) {
-      this.src(player.poster());
-    }
-
-    if (!player.poster() || !player.controls()) {
-      this.hide();
-    }
-
-    player.on('posterchange', vjs.bind(this, function(){
-      this.src(player.poster());
-    }));
-
-    if (!player.isAudio()) {
-      player.on('play', vjs.bind(this, this.hide));
-    }
+    this.update();
+    player.on('posterchange', vjs.bind(this, this.update));
   }
 });
 
-// use the test el to check for backgroundSize style support
-var _backgroundSizeSupported = 'backgroundSize' in vjs.TEST_VID.style;
+/**
+ * Clean up the poster image
+ */
+vjs.PosterImage.prototype.dispose = function(){
+  this.player().off('posterchange', this.update);
+  vjs.Button.prototype.dispose.call(this);
+};
 
+/**
+ * Create the poster image element
+ * @return {Element}
+ */
 vjs.PosterImage.prototype.createEl = function(){
   var el = vjs.createEl('div', {
     className: 'vjs-poster',
@@ -41,40 +37,63 @@ vjs.PosterImage.prototype.createEl = function(){
     tabIndex: -1
   });
 
-  if (!_backgroundSizeSupported) {
-    // setup an img element as a fallback for IE8
-    el.appendChild(vjs.createEl('img'));
+  // To ensure the poster image resizes while maintaining its original aspect
+  // ratio, use a div with `background-size` when available. For browsers that
+  // do not support `background-size` (e.g. IE8), fall back on using a regular
+  // img element.
+  if (!vjs.BACKGROUND_SIZE_SUPPORTED) {
+    this.fallbackImg_ = vjs.createEl('img');
+    el.appendChild(this.fallbackImg_);
   }
 
   return el;
 };
 
-vjs.PosterImage.prototype.src = function(url){
-  var el = this.el();
+/**
+ * Event handler for updates to the player's poster source
+ */
+vjs.PosterImage.prototype.update = function(){
+  var url = this.player().poster();
+
+  this.setSrc(url);
 
-  // getter
-  // can't think of a need for a getter here
-  // see #838 if on is needed in the future
-  // still don't want a getter to set src as undefined
-  if (url === undefined) {
-    return;
+  // If there's no poster source we should display:none on this component
+  // so it's not still clickable or right-clickable
+  if (url) {
+    // Remove the display style property that hide() adds
+    // as opposed to show() which sets display to block
+    // In the future it might be worth creating an `unhide` component method
+    this.el_.style.display = '';
+  } else {
+    this.hide();
   }
+};
 
-  // setter
-  // To ensure the poster image resizes while maintaining its original aspect
-  // ratio, use a div with `background-size` when available. For browsers that
-  // do not support `background-size` (e.g. IE8), fall back on using a regular
-  // img element.
-  if (_backgroundSizeSupported) {
-    el.style.backgroundImage = 'url("' + url + '")';
+/**
+ * Set the poster source depending on the display method
+ */
+vjs.PosterImage.prototype.setSrc = function(url){
+  var backgroundImage;
+
+  if (this.fallbackImg_) {
+    this.fallbackImg_.src = url;
   } else {
-    el.firstChild.src = url;
+    backgroundImage = '';
+    // Any falsey values should stay as an empty string, otherwise
+    // this will throw an extra error
+    if (url) {
+      backgroundImage = 'url("' + url + '")';
+    }
+
+    this.el_.style.backgroundImage = backgroundImage;
   }
 };
 
+/**
+ * Event handler for clicks on the poster image
+ */
 vjs.PosterImage.prototype.onClick = function(){
-  // Only accept clicks when controls are enabled
-  if (this.player().controls()) {
-    this.player_.play();
-  }
+  // We don't want a click to trigger playback when controls are disabled
+  // but CSS should be hiding the poster to prevent that from happening
+  this.player_.play();
 };

test/index.html

@@ -3,6 +3,9 @@
 <head>
   <title>Video.js Test Suite</title>
 
+  <!-- Video.js CSS -->
+  <link rel="stylesheet" href="../build/files/video-js.css" type="text/css">
+
   <!-- Sinon -->
   <script src="../node_modules/sinon/pkg/sinon.js"></script>
   <script src="../node_modules/sinon/pkg/sinon-ie.js"></script>
@@ -11,9 +14,6 @@
   <link rel="stylesheet" href="../node_modules/qunitjs/qunit/qunit.css" />
   <script src="../node_modules/qunitjs/qunit/qunit.js"></script>
 
-  <!-- Video.js CSS -->
-  <link rel="stylesheet" href="../build/files/video-js.css" type="text/css">
-
   <script type="text/javascript">
     (function(){
 

test/karma.conf.js

@@ -59,6 +59,7 @@ module.exports = function(config) {
     customLaunchers: customLaunchers,
 
     files: [
+      '../build/files/video-js.css',
       '../test/karma-qunit-shim.js',
       "../src/js/core.js",
       "../src/js/core-object.js",

test/karma.minified.api.conf.js

@@ -7,6 +7,7 @@ module.exports = function(config) {
     singleRun: true,
 
     files: [
+      '../build/files/video-js.min.css',
       '../test/karma-qunit-shim.js',
       '../node_modules/sinon/pkg/sinon.js',
       '../build/files/minified.video.js',

test/karma.minified.conf.js

@@ -7,6 +7,7 @@ module.exports = function(config) {
     singleRun: true,
 
     files: [
+      '../build/files/video-js.min.css',
       '../test/karma-qunit-shim.js',
       '../node_modules/sinon/pkg/sinon.js',
       '../build/files/test.minified.video.js'

test/minified.html

@@ -3,6 +3,9 @@
 <head>
   <title>Video.js Test Suite</title>
 
+  <!-- Video.js CSS -->
+  <link rel="stylesheet" href="../build/files/video-js.css" type="text/css">
+
   <!-- Sinon -->
   <script src="../node_modules/sinon/pkg/sinon.js"></script>
   <script src="../node_modules/sinon/pkg/sinon-ie.js"></script>
@@ -11,9 +14,6 @@
   <link rel="stylesheet" href="../node_modules/qunitjs/qunit/qunit.css" />
   <script src="../node_modules/qunitjs/qunit/qunit.js"></script>
 
-  <!-- Video.js CSS -->
-  <link rel="stylesheet" href="../build/files/video-js.css" type="text/css">
-
   <!-- SOURCE COMPILED WITH TESTS
   grunt-contrib-qunit doesn't support query vars, so making this script
   so we can automatically run compiled tests too.