@mmcc switched to using button elements for button components. closes #2209

CHANGELOG.md

@@ -39,6 +39,7 @@ CHANGELOG
 * @heff fixed instances of tabIndex that did not have a capital I   ([view](https://github.com/videojs/video.js/pull/2204))
 * @heff fixed a number of IE8 and Flash related issues  ([view](https://github.com/videojs/video.js/pull/2206))
 * @heff Reverted .video-js inline-block style to fix Flash fullscreen  ([view](https://github.com/videojs/video.js/pull/2217))
+* @mmcc switched to using button elements for button components ([view](https://github.com/videojs/video.js/pull/2209))
 
 --------------------
 

grunt.js

@@ -412,7 +412,7 @@ module.exports = function(grunt) {
   grunt.registerTask('default', ['build', 'test-local']);
 
   // Development watch task. Doing the minimum required.
-  grunt.registerTask('dev', ['connect:dev', 'jshint', 'sass', 'browserify:build', 'karma:chrome']);
+  grunt.registerTask('dev', ['jshint', 'sass', 'browserify:build', 'karma:chrome']);
 
   // Skin development watch task.
   grunt.registerTask('skin-dev', ['connect:dev', 'watch:skin']);

src/css/_utilities.scss

@@ -93,3 +93,12 @@
   -webkit-order: $value;
   order: $value;
 }
+
+%icon-default {
+  position: absolute;
+  top: 0;
+  left: 0;
+  width: 100%;
+  height: 100%;
+  text-align: center;
+}

src/css/components/_big-play.scss

@@ -1,12 +1,14 @@
 .video-js .vjs-big-play-button {
   font-size: 3em;
-  line-height: 1em;
+  line-height: 1.5em;
+  height: 1.5em;
+  width: 3em; // Firefox bug: For some reason without width the icon wouldn't show up. Switched to using width and removed padding.
   display: block;
   z-index: 2;
   position: absolute;
   top: 10px;
   left: 10px;
-  padding: 0.2em 1em;
+  padding: 0;
   cursor: pointer;
   opacity: 1;
   border: 2px solid $primary-text;
@@ -17,6 +19,12 @@
   @include transition(all 0.4s);
 
   @extend .vjs-icon-play;
+
+  // Since the big play button doesn't inherit from vjs-control, we need to specify a bit more than
+  // other buttons for the icon.
+  &:before {
+    @extend %icon-default
+  }
 }
 
 /* Hide if controls are disabled, the video is playing, or native controls are used. */

src/css/components/_button.scss

@@ -0,0 +1,13 @@
+.video-js button {
+  background: none;
+  border: none;
+  color: $primary-text;
+  display: inline-block;
+
+  overflow: visible; // IE8
+  font-size: inherit; // IE in general. WTF.
+
+  -webkit-appearance: none;
+  -moz-appearance:    none;
+  appearance:         none;
+}

src/css/components/_control.scss

@@ -7,17 +7,13 @@
   height: 100%;
   width: 4em;
   @include flex(none);
-}
 
-.video-js .vjs-control:before {
+  &:before {
-  font-size: 1.8em;
+    font-size: 1.8em;
-  line-height: 1.67;
+    line-height: 1.67;
-  position: absolute;
+
-  top: 0;
+    @extend %icon-default;
-  left: 0;
+  }
-  width: 100%;
-  height: 100%;
-  text-align: center;
 }
 
 /* Replacement for focus outline */

src/css/components/_menu.scss

@@ -28,12 +28,12 @@
   overflow: auto;
 }
 
-.video-js .vjs-menu-button:hover .vjs-control-content .vjs-menu,
+.video-js .vjs-menu-button:hover .vjs-menu,
-.video-js .vjs-control-content .vjs-menu.vjs-lock-showing {
+.video-js .vjs-menu.vjs-lock-showing {
   display: block;
 }
 /* prevent menus from opening while scrubbing (FF, IE) */
-.video-js.vjs-scrubbing .vjs-menu-button:hover .vjs-control-content .vjs-menu {
+.video-js.vjs-scrubbing .vjs-menu-button:hover .vjs-menu {
   display: none;
 }
 .video-js .vjs-menu-button ul li {

src/css/video-js.scss

@@ -4,6 +4,7 @@
 @import "components/icons";
 @import "components/layout";
 @import "components/big-play";
+@import "components/button";
 @import "components/control-bar";
 @import "components/control";
 @import "components/progress";

src/js/big-play-button.js

@@ -13,12 +13,8 @@ import Component from './component.js';
  */
 class BigPlayButton extends Button {
 
-  createEl() {
+  buildCSSClass() {
-    return super.createEl('div', {
+    return 'vjs-big-play-button';
-      className: 'vjs-big-play-button',
-      innerHTML: '<span aria-hidden="true"></span>',
-      'aria-label': 'play video'
-    });
   }
 
   handleClick() {
@@ -27,5 +23,7 @@ class BigPlayButton extends Button {
 
 }
 
+BigPlayButton.prototype.controlText_ = 'Play Video';
+
 Component.registerComponent('BigPlayButton', BigPlayButton);
 export default BigPlayButton;

src/js/button.js

@@ -27,7 +27,7 @@ class Button extends Component {
     this.on('blur', this.handleBlur);
   }
 
-  createEl(type, props) {
+  createEl(type='button', props={}) {
     // Add standard Aria and Tabindex info
     props = assign({
       className: this.buildCSSClass(),
@@ -38,24 +38,26 @@ class Button extends Component {
 
     let el = super.createEl(type, props);
 
-    // if innerHTML hasn't been overridden (bigPlayButton), add content elements
+    this.controlTextEl_ = Dom.createEl('span', {
-    if (!props.innerHTML) {
+      className: 'vjs-control-text'
-      this.contentEl_ = Dom.createEl('div', {
+    });
-        className: 'vjs-control-content'
-      });
 
-      this.controlText_ = Dom.createEl('span', {
+    el.appendChild(this.controlTextEl_);
-        className: 'vjs-control-text',
-        innerHTML: this.localize(this.buttonText) || 'Need Text'
-      });
 
-      this.contentEl_.appendChild(this.controlText_);
+    this.controlText(this.controlText_);
-      el.appendChild(this.contentEl_);
-    }
 
     return el;
   }
 
+  controlText(text) {
+    if (!text) return this.controlText_ || 'Need Text';
+
+    this.controlText_ = text;
+    this.controlTextEl_.innerHTML = this.localize(this.controlText_);
+
+    return this;
+  }
+
   buildCSSClass() {
     return `vjs-control vjs-button ${super.buildCSSClass()}`;
   }

src/js/control-bar/fullscreen-toggle.js

@@ -17,16 +17,16 @@ class FullscreenToggle extends Button {
   handleClick() {
     if (!this.player_.isFullscreen()) {
       this.player_.requestFullscreen();
-      this.controlText_.innerHTML = this.localize('Non-Fullscreen');
+      this.controlText('Non-Fullscreen');
     } else {
       this.player_.exitFullscreen();
-      this.controlText_.innerHTML = this.localize('Fullscreen');
+      this.controlText('Fullscreen');
     }
   }
 
 }
 
-FullscreenToggle.prototype.buttonText = 'Fullscreen';
+FullscreenToggle.prototype.controlText_ = 'Fullscreen';
 
 Component.registerComponent('FullscreenToggle', FullscreenToggle);
 export default FullscreenToggle;

src/js/control-bar/mute-toggle.js

@@ -30,13 +30,6 @@ class MuteToggle extends Button {
     });
   }
 
-  createEl() {
-    return super.createEl('div', {
-      className: this.buildCSSClass(),
-      innerHTML: `<div><span class="vjs-control-text">${this.localize('Mute')}</span></div>`
-    });
-  }
-
   buildCSSClass() {
     return `vjs-mute-control ${super.buildCSSClass()}`;
   }
@@ -62,8 +55,8 @@ class MuteToggle extends Button {
     // This check is needed because this function gets called every time the volume level is changed.
     let toMute = this.player_.muted() ? 'Unmute' : 'Mute';
     let localizedMute = this.localize(toMute);
-    if (this.el_.children[0].children[0].innerHTML !== localizedMute) {
+    if (this.controlText() !== localizedMute) {
-      this.el_.children[0].children[0].innerHTML = localizedMute;
+      this.controlText(localizedMute);
     }
 
     /* TODO improve muted icon classes */
@@ -75,5 +68,7 @@ class MuteToggle extends Button {
 
 }
 
+MuteToggle.prototype.controlText_ = 'Mute';
+
 Component.registerComponent('MuteToggle', MuteToggle);
 export default MuteToggle;

src/js/control-bar/play-toggle.js

@@ -34,19 +34,19 @@ class PlayToggle extends Button {
   handlePlay() {
     this.removeClass('vjs-paused');
     this.addClass('vjs-playing');
-    this.el_.children[0].children[0].innerHTML = this.localize('Pause'); // change the button text to "Pause"
+    this.controlText('Pause'); // change the button text to "Pause"
   }
 
   // handlePause - Add the vjs-paused class to the element so it can change appearance
   handlePause() {
     this.removeClass('vjs-playing');
     this.addClass('vjs-paused');
-    this.el_.children[0].children[0].innerHTML = this.localize('Play'); // change the button text to "Play"
+    this.controlText('Play'); // change the button text to "Play"
   }
 
 }
 
-PlayToggle.prototype.buttonText = 'Play';
+PlayToggle.prototype.controlText_ = 'Play';
 
 Component.registerComponent('PlayToggle', PlayToggle);
 export default PlayToggle;

src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js

@@ -36,6 +36,10 @@ class PlaybackRateMenuButton extends MenuButton {
     return el;
   }
 
+  buildCSSClass() {
+    return `vjs-playback-rate ${super.buildCSSClass()}`;
+  }
+
   // Menu creation
   createMenu() {
     let menu = new Menu(this.player());
@@ -102,8 +106,7 @@ class PlaybackRateMenuButton extends MenuButton {
 
 }
 
-PlaybackRateMenuButton.prototype.buttonText = 'Playback Rate';
+PlaybackRateMenuButton.prototype.controlText_ = 'Playback Rate';
-PlaybackRateMenuButton.prototype.className = 'vjs-playback-rate';
 
 Component.registerComponent('PlaybackRateMenuButton', PlaybackRateMenuButton);
 export default PlaybackRateMenuButton;

src/js/control-bar/text-track-controls/captions-button.js

@@ -14,6 +14,10 @@ class CaptionsButton extends TextTrackButton {
     this.el_.setAttribute('aria-label','Captions Menu');
   }
 
+  buildCSSClass() {
+    return `vjs-captions-button ${super.buildCSSClass()}`;
+  }
+
   update() {
     let threshold = 2;
     super.update();
@@ -43,8 +47,7 @@ class CaptionsButton extends TextTrackButton {
 }
 
 CaptionsButton.prototype.kind_ = 'captions';
-CaptionsButton.prototype.buttonText = 'Captions';
+CaptionsButton.prototype.controlText_ = 'Captions';
-CaptionsButton.prototype.className = 'vjs-captions-button';
 
 Component.registerComponent('CaptionsButton', CaptionsButton);
 export default CaptionsButton;

src/js/control-bar/text-track-controls/chapters-button.js

@@ -22,6 +22,10 @@ class ChaptersButton extends TextTrackButton {
     this.el_.setAttribute('aria-label','Chapters Menu');
   }
 
+  buildCSSClass() {
+    return `vjs-chapters-button ${super.buildCSSClass()}`;
+  }
+
   // Create a menu item for each text track
   createItems() {
     let items = [];
@@ -105,8 +109,7 @@ class ChaptersButton extends TextTrackButton {
 }
 
 ChaptersButton.prototype.kind_ = 'chapters';
-ChaptersButton.prototype.buttonText = 'Chapters';
+ChaptersButton.prototype.controlText_ = 'Chapters';
-ChaptersButton.prototype.className = 'vjs-chapters-button';
 
 Component.registerComponent('ChaptersButton', ChaptersButton);
 export default ChaptersButton;

src/js/control-bar/text-track-controls/subtitles-button.js

@@ -13,11 +13,14 @@ class SubtitlesButton extends TextTrackButton {
     this.el_.setAttribute('aria-label','Subtitles Menu');
   }
 
+  buildCSSClass() {
+    return `vjs-subtitles-button ${super.buildCSSClass()}`;
+  }
+
 }
 
 SubtitlesButton.prototype.kind_ = 'subtitles';
-SubtitlesButton.prototype.buttonText = 'Subtitles';
+SubtitlesButton.prototype.controlText_ = 'Subtitles';
-SubtitlesButton.prototype.className = 'vjs-subtitles-button';
 
 Component.registerComponent('SubtitlesButton', SubtitlesButton);
 export default SubtitlesButton;

src/js/control-bar/volume-menu-button.js

@@ -31,6 +31,10 @@ class VolumeMenuButton extends MenuButton {
     this.addClass('vjs-menu-button');
   }
 
+  buildCSSClass() {
+    return `vjs-volume-menu-button ${super.buildCSSClass()}`;
+  }
+
   createMenu() {
     let menu = new Menu(this.player_, {
       contentElType: 'div'
@@ -57,16 +61,10 @@ class VolumeMenuButton extends MenuButton {
     super.handleClick();
   }
 
-  createEl() {
-    return super.createEl('div', {
-      className: 'vjs-volume-menu-button vjs-menu-button vjs-control vjs-button',
-      innerHTML: `<div><span class="vjs-control-text">${this.localize('Mute')}</span></div>`
-    });
-  }
-
 }
 
 VolumeMenuButton.prototype.volumeUpdate = MuteToggle.prototype.update;
+VolumeMenuButton.prototype.controlText_ = 'Mute';
 
 Component.registerComponent('VolumeMenuButton', VolumeMenuButton);
 export default VolumeMenuButton;

src/js/menu/menu-button.js

@@ -76,9 +76,15 @@ class MenuButton extends Button {
    */
   createItems(){}
 
+  createEl() {
+    return super.createEl('div', {
+      className: this.buildCSSClass()
+    });
+  }
+
   /** @inheritDoc */
   buildCSSClass() {
-    return `${this.className} vjs-menu-button ${super.buildCSSClass()}`;
+    return `vjs-menu-button ${super.buildCSSClass()}`;
   }
 
   // Focus - Add keyboard functionality to element

test/unit/button.test.js

@@ -18,8 +18,8 @@ test('should localize its text', function(){
   });
 
   testButton = new Button(player);
-  testButton.buttonText = 'Play';
+  testButton.controlText_ = 'Play';
   el = testButton.createEl();
 
-  ok(el.innerHTML, 'Juego', 'translation was successful');
+  ok(el.innerHTML.match('Juego'));
 });