Making changes based on comments

videojs · May 12, 2015 · 13c9e04 · 13c9e04
1 parent 5fb6aed
commit 13c9e04
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 25 deletions.
diff --git a/src/js/player.js b/src/js/player.js
@@ -258,21 +258,24 @@ class Player extends Component {
   }
 
   dimension(dimension, value) {
-    let property = dimension + '_';
+    let privDimension = dimension + '_';
 
     if (value === undefined) {
-      return this[property] || 0;
+      return this[privDimension] || 0;
     }
 
     if (value === '') {
       // If an empty string is given, reset the dimension to be automatic
-      this[property] = undefined;
+      this[privDimension] = undefined;
     } else {
-      this[property] = parseFloat(value);
+      let parsedVal = parseFloat(value);
 
-      if (isNaN(this[property])) {
+      if (isNaN(this[privDimension])) {
         Lib.log.error(`Improper value "${value}" supplied for for ${dimension}`);
+        return this;
       }
+
+      this[privDimension] = parsedVal;
     }
 
     this.updateStyleEl_();
@@ -450,7 +453,7 @@ class Player extends Component {
     this.on(this.tech, 'ratechange', this.handleTechRateChange);
     this.on(this.tech, 'volumechange', this.handleTechVolumeChange);
     this.on(this.tech, 'texttrackchange', this.onTextTrackChange);
-	this.on(this.tech, 'loadedmetadata', this.updateStyleEl_);
+    this.on(this.tech, 'loadedmetadata', this.updateStyleEl_);
 
     if (this.controls() && !this.usingNativeControls()) {
       this.addTechControlsListeners();

diff --git a/test/unit/player.js b/test/unit/player.js
@@ -153,25 +153,6 @@ test('should asynchronously fire error events during source selection', function
   Lib.log.error.restore();
 });
 
-// test('should set the width and height of the player', function(){
-//   var player = TestHelpers.makePlayer({ width: 123, height: '100%' });
-//
-//   ok(player.width() === 123);
-//   ok(player.el().style.width === '123px');
-//
-//   var fixture = document.getElementById('qunit-fixture');
-//   var container = document.createElement('div');
-//   fixture.appendChild(container);
-//
-//   // Player container needs to have height in order to have height
-//   // Don't want to mess with the fixture itself
-//   container.appendChild(player.el());
-//   container.style.height = '1000px';
-//   ok(player.height() === 1000);
-//
-//   player.dispose();
-// });
-
 test('should set the width, height, and aspect ratio via a css class', function(){
   let player = TestHelpers.makePlayer();
   let getStyleText = function(styleEl){