Skip to content

Commit

Permalink
feat(fluid): use default aspect ratio for fluid players if width unkn…
Browse files Browse the repository at this point in the history
…own (#3614)

If a player is fluid and does not have a width set, and preload is set to none, the height of the player is zero. This includes where preload is forced to none by mobile Chrome as in #3606.

* If the player has the .vjs-fluid class when initialised, fluid is set to true, so adding the class behaves the same as {fluid: true} in the setup options.
* The fluid(bool) setter calls player.updateStyleEl_(). Otherwise it won't be triggered in createEl() if an aspect ratio is not also set.
* Corrects the test for a set videoWidth() in updateStyleEl_() - videoWidth() returns 0 if the width is unknown. This allows the default 16:9 to kick in rather than using 0:0.
  • Loading branch information
mister-ben authored and gkatsev committed Nov 3, 2016
1 parent f599ef4 commit 2988f6a
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ class Player extends Component {
}

/**
* Add/remove the vjs-fluid class
* Get/set fluid mode
*
* @param {Boolean} bool Value of true adds the class, value of false removes the class
*/
Expand All @@ -512,6 +512,8 @@ class Player extends Component {
} else {
this.removeClass('vjs-fluid');
}

this.updateStyleEl_();
}

/**
Expand Down Expand Up @@ -568,7 +570,7 @@ class Player extends Component {
if (this.aspectRatio_ !== undefined && this.aspectRatio_ !== 'auto') {
// Use any aspectRatio that's been specifically set
aspectRatio = this.aspectRatio_;
} else if (this.videoWidth()) {
} else if (this.videoWidth() > 0) {
// Otherwise try to get the aspect ratio from the video metadata
aspectRatio = this.videoWidth() + ':' + this.videoHeight();
} else {
Expand Down Expand Up @@ -2623,6 +2625,10 @@ class Player extends Component {
const tagOptions = Dom.getElAttributes(tag);
const dataSetup = tagOptions['data-setup'];

if (Dom.hasElClass(tag, 'vjs-fluid')) {
tagOptions.fluid = true;
}

// Check if data-setup attr exists.
if (dataSetup !== null) {
// Parse options JSON
Expand Down
16 changes: 16 additions & 0 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,22 @@ QUnit.test('should set the width, height, and aspect ratio via a css class', fun
player.dispose();
});

QUnit.test('should default to 16:9 when fluid', function(assert) {
const player = TestHelpers.makePlayer({fluid: true});

assert.equal(player.currentHeight() / player.currentWidth(), 0.5625, 'fluid player without dimensions defaults to 16:9');
});

QUnit.test('should set fluid to true if element has vjs-fluid class', function(assert) {
const tag = TestHelpers.makeTag();

tag.className += ' vjs-fluid';

const player = TestHelpers.makePlayer({}, tag);

assert.ok(player.fluid(), 'fluid is true with vjs-fluid class');
});

QUnit.test('should use an class name that begins with an alpha character', function(assert) {
const alphaPlayer = TestHelpers.makePlayer({ id: 'alpha1' });
const numericPlayer = TestHelpers.makePlayer({ id: '1numeric' });
Expand Down

0 comments on commit 2988f6a

Please sign in to comment.