From eeff79c5e8b56f647535d3794b1d907631482301 Mon Sep 17 00:00:00 2001 From: Harisha Rajam Swaminathan <35213866+harisha-swaminathan@users.noreply.github.com> Date: Thu, 17 Mar 2022 17:10:33 -0400 Subject: [PATCH] refactor: Unify audioOnly mode and audioPoster mode (#7678) Co-authored-by: Alex Barstow --- .../control-bar/picture-in-picture-toggle.js | 15 +++ src/js/player.js | 97 ++++++++++++--- test/unit/controls.test.js | 17 +++ test/unit/player.test.js | 117 ++++++++++++++++-- 4 files changed, 213 insertions(+), 33 deletions(-) diff --git a/src/js/control-bar/picture-in-picture-toggle.js b/src/js/control-bar/picture-in-picture-toggle.js index 5302da2e05..addb71c639 100644 --- a/src/js/control-bar/picture-in-picture-toggle.js +++ b/src/js/control-bar/picture-in-picture-toggle.js @@ -29,6 +29,21 @@ class PictureInPictureToggle extends Button { this.on(player, ['enterpictureinpicture', 'leavepictureinpicture'], (e) => this.handlePictureInPictureChange(e)); this.on(player, ['disablepictureinpicturechanged', 'loadedmetadata'], (e) => this.handlePictureInPictureEnabledChange(e)); + this.on(player, ['loadedmetadata', 'audioonlymodechange', 'audiopostermodechange'], () => { + // This audio detection will not detect HLS or DASH audio-only streams because there was no reliable way to detect them at the time + const isSourceAudio = player.currentType().substring(0, 5) === 'audio'; + + if (isSourceAudio || player.audioPosterMode() || player.audioOnlyMode()) { + if (player.isInPictureInPicture()) { + player.exitPictureInPicture(); + } + this.hide(); + } else { + this.show(); + } + + }); + // TODO: Deactivate button on player emptied event. this.disable(); } diff --git a/src/js/player.js b/src/js/player.js index ebdeecfbbe..67d5052c5e 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -398,6 +398,9 @@ class Player extends Component { // Init state audioOnlyMode_ this.audioOnlyMode_ = false; + // Init state audioPosterMode_ + this.audioPosterMode_ = false; + // Init state audioOnlyCache_ this.audioOnlyCache_ = { playerHeight: null, @@ -583,7 +586,15 @@ class Player extends Component { this.breakpoints(this.options_.breakpoints); this.responsive(this.options_.responsive); - this.audioOnlyMode(this.options_.audioOnlyMode); + + // Calling both the audio mode methods after the player is fully + // setup to be able to listen to the events triggered by them + this.on('ready', () => { + // Calling the audioPosterMode method first so that + // the audioOnlyMode can take precedence when both options are set to true + this.audioPosterMode(this.options_.audioPosterMode); + this.audioOnlyMode(this.options_.audioOnlyMode); + }); } /** @@ -4303,11 +4314,6 @@ class Player extends Component { return !!this.isAudio_; } - updateAudioOnlyModeState_(value) { - this.audioOnlyMode_ = value; - this.trigger('audioonlymodechange'); - } - enableAudioOnlyUI_() { // Update styling immediately to show the control bar so we can get its height this.addClass('vjs-audio-only-mode'); @@ -4334,7 +4340,7 @@ class Player extends Component { // Set the player height the same as the control bar this.height(controlBarHeight); - this.updateAudioOnlyModeState_(true); + this.trigger('audioonlymodechange'); } disableAudioOnlyUI_() { @@ -4345,7 +4351,7 @@ class Player extends Component { // Reset player height this.height(this.audioOnlyCache_.playerHeight); - this.updateAudioOnlyModeState_(false); + this.trigger('audioonlymodechange'); } /** @@ -4366,6 +4372,8 @@ class Player extends Component { return this.audioOnlyMode_; } + this.audioOnlyMode_ = value; + const PromiseClass = this.options_.Promise || window.Promise; if (PromiseClass) { @@ -4382,6 +4390,10 @@ class Player extends Component { exitPromises.push(this.exitFullscreen()); } + if (this.audioPosterMode()) { + exitPromises.push(this.audioPosterMode(false)); + } + return PromiseClass.all(exitPromises).then(() => this.enableAudioOnlyUI_()); } @@ -4404,35 +4416,80 @@ class Player extends Component { } } + enablePosterModeUI_() { + // Hide the video element and show the poster image to enable posterModeUI + const tech = this.tech_ && this.tech_; + + tech.hide(); + this.addClass('vjs-audio-poster-mode'); + this.trigger('audiopostermodechange'); + } + + disablePosterModeUI_() { + // Show the video element and hide the poster image to disable posterModeUI + const tech = this.tech_ && this.tech_; + + tech.show(); + this.removeClass('vjs-audio-poster-mode'); + this.trigger('audiopostermodechange'); + } + /** * Get the current audioPosterMode state or set audioPosterMode to true or false * * @param {boolean} [value] * The value to set audioPosterMode to. * - * @return {boolean} - * True if audioPosterMode is on, false otherwise. + * @return {Promise|boolean} + * A Promise is returned when setting the state, and a boolean when getting + * the present state */ audioPosterMode(value) { - if (this.audioPosterMode_ === undefined) { - this.audioPosterMode_ = this.options_.audioPosterMode; - } - if (typeof value !== 'boolean' || value === this.audioPosterMode_) { return this.audioPosterMode_; } this.audioPosterMode_ = value; - if (this.audioPosterMode_) { - this.tech_.hide(); - this.addClass('vjs-audio-poster-mode'); + const PromiseClass = this.options_.Promise || window.Promise; + + if (PromiseClass) { + + if (value) { + + if (this.audioOnlyMode()) { + const audioOnlyModePromise = this.audioOnlyMode(false); + + return audioOnlyModePromise.then(() => { + // enable audio poster mode after audio only mode is disabled + this.enablePosterModeUI_(); + }); + } + + return PromiseClass.resolve().then(() => { + // enable audio poster mode + this.enablePosterModeUI_(); + }); + } + + return PromiseClass.resolve().then(() => { + // disable audio poster mode + this.disablePosterModeUI_(); + }); + } + + if (value) { + + if (this.audioOnlyMode()) { + this.audioOnlyMode(false); + } + + this.enablePosterModeUI_(); return; } - // Show the video element and hide the poster image if audioPosterMode is set to false - this.tech_.show(); - this.removeClass('vjs-audio-poster-mode'); + + this.disablePosterModeUI_(); } /** diff --git a/test/unit/controls.test.js b/test/unit/controls.test.js index fad4d0b246..8cc3ebbf59 100644 --- a/test/unit/controls.test.js +++ b/test/unit/controls.test.js @@ -283,6 +283,23 @@ QUnit.test('Picture-in-Picture control enabled property value should be correct pictureInPictureToggle.dispose(); }); +QUnit.test('Picture-in-Picture control is hidden when the source is audio', function(assert) { + const player = TestHelpers.makePlayer({}); + const pictureInPictureToggle = new PictureInPictureToggle(player); + + player.src({src: 'example.mp4', type: 'video/mp4'}); + player.trigger('loadedmetadata'); + + assert.notOk(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is not hidden initially'); + + player.src({src: 'example1.mp3', type: 'audio/mp3'}); + player.trigger('loadedmetadata'); + assert.ok(pictureInPictureToggle.hasClass('vjs-hidden'), 'pictureInPictureToggle button is hidden whenh the source is audio'); + + player.dispose(); + pictureInPictureToggle.dispose(); +}); + QUnit.test('Fullscreen control text should be correct when fullscreenchange is triggered', function(assert) { const player = TestHelpers.makePlayer({controlBar: false}); const fullscreentoggle = new FullscreenToggle(player); diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 2e832695ba..275c61fb34 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -1573,34 +1573,77 @@ QUnit.test('should add an audio player region if an audio el is used', function( QUnit.test('default audioPosterMode value at player creation', function(assert) { const player = TestHelpers.makePlayer({}); + player.trigger('ready'); assert.ok(player.audioPosterMode() === false, 'audioPosterMode is false by default'); const player2 = TestHelpers.makePlayer({ audioPosterMode: true }); + const done = assert.async(); - assert.ok(player2.audioPosterMode() === true, 'audioPosterMode can be set to true when the player is created'); - player.dispose(); - player2.dispose(); + player2.trigger('ready'); + player2.one('audiopostermodechange', () => { + assert.ok(player2.audioPosterMode(), 'audioPosterMode can be set to true when the player is created'); + done(); + }); }); QUnit.test('get and set audioPosterMode value', function(assert) { const player = TestHelpers.makePlayer({}); - player.audioPosterMode(true); - assert.ok(player.audioPosterMode() === true, 'audioPosterMode is set to true'); + return player.audioPosterMode(true) + .then(() => { + assert.ok(player.audioPosterMode(), 'audioPosterMode is set to true'); + }); }); QUnit.test('vjs-audio-poster-mode class and video element visibility when audioPosterMode is true', function(assert) { const player = TestHelpers.makePlayer({}); - player.audioPosterMode(true); - assert.ok(player.el().className.indexOf('vjs-audio-poster-mode') !== -1, 'vjs-audio-poster-mode class is present'); - assert.ok(player.tech_.el().className.indexOf('vjs-hidden') !== -1, 'video element is hidden'); + player.trigger('ready'); - player.audioPosterMode(false); - assert.ok(player.el().className.indexOf('vjs-audio-poster-mode') === -1, 'vjs-audio-poster-mode class is removed'); + assert.ok(player.el().className.indexOf('vjs-audio-poster-mode') === -1, 'vjs-audio-poster-mode class is not present initially'); assert.ok(player.tech_.el().className.indexOf('vjs-hidden') === -1, 'video element is visible'); + + return player.audioPosterMode(true) + .then(() => { + assert.ok(player.el().className.indexOf('vjs-audio-poster-mode') !== -1, 'vjs-audio-poster-mode class is present'); + assert.ok(player.tech_.el().className.indexOf('vjs-hidden') !== -1, 'video element is hidden'); + }); +}); + +QUnit.test('setting audioPosterMode() should trigger audiopostermodechange event', function(assert) { + const player = TestHelpers.makePlayer({ + audioPosterMode: true + }); + const done = assert.async(); + + player.trigger('ready'); + player.one('audiopostermodechange', () => { + assert.ok(true, 'audiopostermodechange event was triggered'); + assert.ok(player.audioPosterMode(), 'audioPosterMode is set to true'); + done(); + }); +}); + +QUnit.test('audioPosterMode is synchronous and returns undefined when promises are unsupported', function(assert) { + const originalPromise = window.Promise; + const player = TestHelpers.makePlayer({}); + + player.trigger('ready'); + window.Promise = undefined; + + const returnValTrue = player.audioPosterMode(true); + + assert.equal(returnValTrue, undefined, 'return value is undefined'); + assert.ok(player.audioPosterMode(), 'state synchronously set to true'); + + const returnValFalse = player.audioPosterMode(false); + + assert.equal(returnValFalse, undefined, 'return value is undefined'); + assert.notOk(player.audioPosterMode(), 'state synchronously set to false'); + + window.Promise = originalPromise; }); QUnit.test('should setScrubbing when seeking or not seeking', function(assert) { @@ -2807,23 +2850,24 @@ QUnit.test('playbackRates only accepts arrays of numbers', function(assert) { }); QUnit.test('audioOnlyMode can be set by option', function(assert) { - assert.expect(4); + assert.expect(3); const done = assert.async(); const player = TestHelpers.makePlayer({audioOnlyMode: true}); + player.trigger('ready'); player.one('audioonlymodechange', () => { assert.equal(player.audioOnlyMode(), true, 'asynchronously set via option'); assert.equal(player.hasClass('vjs-audio-only-mode'), true, 'class added asynchronously'); done(); }); - - assert.equal(player.audioOnlyMode(), false, 'defaults to false'); assert.equal(player.hasClass('vjs-audio-only-mode'), false, 'initially does not have class'); }); QUnit.test('audioOnlyMode(true) returns Promise when promises are supported', function(assert) { const player = TestHelpers.makePlayer({}); + + player.trigger('ready'); const returnValTrue = player.audioOnlyMode(true); if (window.Promise) { @@ -2835,6 +2879,7 @@ QUnit.test('audioOnlyMode(false) returns Promise when promises are supported', f const done = assert.async(); const player = TestHelpers.makePlayer({audioOnlyMode: true}); + player.trigger('ready'); player.one('audioonlymodechange', () => { const returnValFalse = player.audioOnlyMode(false); @@ -2848,6 +2893,7 @@ QUnit.test('audioOnlyMode(false) returns Promise when promises are supported', f QUnit.test('audioOnlyMode() getter returns Boolean', function(assert) { const player = TestHelpers.makePlayer({}); + player.trigger('ready'); assert.ok(typeof player.audioOnlyMode() === 'boolean', 'getter correctly returns boolean'); }); @@ -2855,6 +2901,7 @@ QUnit.test('audioOnlyMode(true/false) is synchronous and returns undefined when const originalPromise = window.Promise; const player = TestHelpers.makePlayer({}); + player.trigger('ready'); window.Promise = undefined; const returnValTrue = player.audioOnlyMode(true); @@ -2873,6 +2920,7 @@ QUnit.test('audioOnlyMode(true/false) is synchronous and returns undefined when QUnit.test('audioOnlyMode() gets the correct audioOnlyMode state', function(assert) { const player = TestHelpers.makePlayer({}); + player.trigger('ready'); assert.equal(player.audioOnlyMode(), false, 'defaults to false'); return player.audioOnlyMode(true) @@ -2885,6 +2933,7 @@ QUnit.test('audioOnlyMode() gets the correct audioOnlyMode state', function(asse QUnit.test('audioOnlyMode(true/false) adds or removes vjs-audio-only-mode class to player', function(assert) { const player = TestHelpers.makePlayer({}); + player.trigger('ready'); assert.equal(player.hasClass('vjs-audio-only-mode'), false, 'class not initially present'); return player.audioOnlyMode(true) @@ -2899,6 +2948,7 @@ QUnit.test('setting audioOnlyMode() triggers audioonlymodechange event', functio let audioOnlyModeState = false; let audioOnlyModeChangeEvents = 0; + player.trigger('ready'); player.on('audioonlymodechange', () => { audioOnlyModeChangeEvents++; audioOnlyModeState = player.audioOnlyMode(); @@ -2920,6 +2970,7 @@ QUnit.test('setting audioOnlyMode() triggers audioonlymodechange event', functio QUnit.test('audioOnlyMode(true/false) changes player height', function(assert) { const player = TestHelpers.makePlayer({controls: true, height: 600}); + player.trigger('ready'); player.hasStarted(true); const controlBarHeight = player.getChild('ControlBar').currentHeight(); @@ -2938,6 +2989,7 @@ QUnit.test('audioOnlyMode(true/false) changes player height', function(assert) { QUnit.test('audioOnlyMode(true/false) hides/shows player components except control bar', function(assert) { const player = TestHelpers.makePlayer({controls: true}); + player.trigger('ready'); player.hasStarted(true); assert.equal(TestHelpers.getComputedStyle(player.getChild('TextTrackDisplay').el_, 'display'), 'block', 'TextTrackDisplay is initially visible'); @@ -3016,6 +3068,7 @@ QUnit.test('audioOnlyMode(true/false) hides/shows video-specific control bar com // ChaptersButton only shows once cues added and update() called controlBar.getChild('ChaptersButton').update(); + player.trigger('ready'); player.hasStarted(true); // Show all control bar children @@ -3059,3 +3112,41 @@ QUnit.test('audioOnlyMode(true/false) hides/shows video-specific control bar com }) .catch(() => assert.ok(false, 'test error')); }); + +QUnit.test('setting both audioOnlyMode and audioPosterMode options to true will only turn audioOnlyMode', function(assert) { + const player = TestHelpers.makePlayer({audioOnlyMode: true, audioPosterMode: true}); + const done = assert.async(); + + player.trigger('ready'); + + player.one('audioonlymodechange', () => { + assert.ok(player.audioOnlyMode(), 'audioOnlyMode is true'); + assert.notOk(player.audioPosterMode(), 'audioPosterMode is false'); + done(); + }); +}); + +QUnit.test('turning on audioOnlyMode when audioPosterMode is already on will turn off audioPosterMode', function(assert) { + const player = TestHelpers.makePlayer({audioPosterMode: true}); + + player.trigger('ready'); + assert.ok(player.audioPosterMode(), 'audioPosterMode is true'); + return player.audioOnlyMode(true) + .then(() => { + assert.notOk(player.audioPosterMode(), 'audioPosterMode is false'); + assert.ok(player.audioOnlyMode(), 'audioOnlyMode is true'); + }); +}); + +QUnit.test('turning on audioPosterMode when audioOnlyMode is already on will turn off audioOnlyMode', function(assert) { + const player = TestHelpers.makePlayer({audioOnlyMode: true}); + + player.trigger('ready'); + assert.ok(player.audioOnlyMode(), 'audioOnlyMode is true'); + return player.audioPosterMode(true) + .then(() => { + assert.ok(player.audioPosterMode(), 'audioPosterMode is true'); + assert.notOk(player.audioOnlyMode(), 'audioOnlyMode is false'); + }); +}); +