From 69dc1e8a8aff4693d975bb39d030e34f9d57b546 Mon Sep 17 00:00:00 2001 From: Vineet Date: Mon, 1 Aug 2016 18:54:07 -0400 Subject: [PATCH 01/10] # This is a combination of 3 commits. # The first commit's message is: Squashing commits and rebasing Making changes to take into consideration fullscreen change on component Making proposed changes Removing test commented code Making the necessary changes and adding a test # The 2nd commit message will be skipped: # Adding the correct style to the test # The 3rd commit message will be skipped: # Removing duplicate instance of same test --- src/js/control-bar/fullscreen-toggle.js | 19 ++++++++++++++++--- test/unit/controls.test.js | 12 ++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/js/control-bar/fullscreen-toggle.js b/src/js/control-bar/fullscreen-toggle.js index 81ac4f9e64..a66b3c73ba 100644 --- a/src/js/control-bar/fullscreen-toggle.js +++ b/src/js/control-bar/fullscreen-toggle.js @@ -12,6 +12,10 @@ import Component from '../component.js'; */ class FullscreenToggle extends Button { + constructor(player, options) { + super(player, options); + this.on(player, 'fullscreenchange', this.handleFullscreenChange); + } /** * Allow sub components to stack CSS class names * @@ -21,7 +25,18 @@ class FullscreenToggle extends Button { buildCSSClass() { return `vjs-fullscreen-control ${super.buildCSSClass()}`; } - + /** + * Handles Fullscreenchange on the component and change control text accordingly + * + * @method handleFullscreenChange + */ + handleFullscreenChange() { + if (this.player_.isFullscreen()) { + this.controlText('Non-Fullscreen'); + } else { + this.controlText('Fullscreen'); + } + } /** * Handles click for full screen * @@ -30,10 +45,8 @@ class FullscreenToggle extends Button { handleClick() { if (!this.player_.isFullscreen()) { this.player_.requestFullscreen(); - this.controlText('Non-Fullscreen'); } else { this.player_.exitFullscreen(); - this.controlText('Fullscreen'); } } diff --git a/test/unit/controls.test.js b/test/unit/controls.test.js index e2a8efc2fc..d5c3fec667 100644 --- a/test/unit/controls.test.js +++ b/test/unit/controls.test.js @@ -3,6 +3,7 @@ import VolumeControl from '../../src/js/control-bar/volume-control/volume-contro import MuteToggle from '../../src/js/control-bar/mute-toggle.js'; import PlaybackRateMenuButton from '../../src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js'; import Slider from '../../src/js/slider/slider.js'; +import FullscreenToggle from '../../src/js/control-bar/fullscreen-toggle.js'; import TestHelpers from './test-helpers.js'; import document from 'global/document'; @@ -110,3 +111,14 @@ QUnit.test('should hide playback rate control if it\'s not supported', function( QUnit.ok(playbackRate.el().className.indexOf('vjs-hidden') >= 0, 'playbackRate is not hidden'); }); + +QUnit.test('Fullscreen control text should be correct when fullscreenchange is triggered', function() { + const player = TestHelpers.makePlayer(); + const fullscreentoggle = new FullscreenToggle(player); + player.isFullscreen(true); + player.trigger('fullscreenchange'); + QUnit.equal(fullscreentoggle.controlText(), 'Non-Fullscreen', 'Control Text is correct while switching to fullscreen mode'); + player.isFullscreen(false); + player.trigger('fullscreenchange'); + QUnit.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); +}); \ No newline at end of file From 8db1807ec7c9b71813c8510f3793376b0eb65fc4 Mon Sep 17 00:00:00 2001 From: Vineet Date: Mon, 1 Aug 2016 18:54:07 -0400 Subject: [PATCH 02/10] Squashing commits and rebasing Making changes to take into consideration fullscreen change on component Making proposed changes Removing test commented code Making the necessary changes and adding a test --- test/unit/controls.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/controls.test.js b/test/unit/controls.test.js index d5c3fec667..64afba1925 100644 --- a/test/unit/controls.test.js +++ b/test/unit/controls.test.js @@ -121,4 +121,4 @@ QUnit.test('Fullscreen control text should be correct when fullscreenchange is t player.isFullscreen(false); player.trigger('fullscreenchange'); QUnit.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); -}); \ No newline at end of file +}); From 85c7872c03c0c1ee1bd48a0b39d5750337f9089e Mon Sep 17 00:00:00 2001 From: Vineet Date: Mon, 1 Aug 2016 18:54:07 -0400 Subject: [PATCH 03/10] Control text fix fullscreen --- src/js/player.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/js/player.js b/src/js/player.js index 97b031ee35..cff3ee3c59 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -1114,6 +1114,8 @@ class Player extends Component { this.addClass('vjs-fullscreen'); } else { this.removeClass('vjs-fullscreen'); + // Making sure the Control text reverts back after pressing ESC + this.controlBar.fullscreenToggle.controlText('Fullscreen'); } } From aac5e4472b4d0f99edb86421d7ea65de168a55eb Mon Sep 17 00:00:00 2001 From: Vineet Date: Tue, 2 Aug 2016 16:17:31 -0400 Subject: [PATCH 04/10] Making changes to take into consideration fullscreen change on component --- src/js/clickable-component.js | 6 ++++++ src/js/control-bar/fullscreen-toggle.js | 1 - src/js/player.js | 2 -- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/js/clickable-component.js b/src/js/clickable-component.js index 4b4c830165..cb26e296a0 100644 --- a/src/js/clickable-component.js +++ b/src/js/clickable-component.js @@ -28,6 +28,7 @@ class ClickableComponent extends Component { this.on('click', this.handleClick); this.on('focus', this.handleFocus); this.on('blur', this.handleBlur); + player.on('fullscreenchange', this.handleFullscreenChange); } /** @@ -167,6 +168,11 @@ class ClickableComponent extends Component { * @method handleClick */ handleClick() {} + + /** + + */ + handleFullscreenChange() {} /** * Handle Focus - Add keyboard functionality to element diff --git a/src/js/control-bar/fullscreen-toggle.js b/src/js/control-bar/fullscreen-toggle.js index a66b3c73ba..505d2035c4 100644 --- a/src/js/control-bar/fullscreen-toggle.js +++ b/src/js/control-bar/fullscreen-toggle.js @@ -35,7 +35,6 @@ class FullscreenToggle extends Button { this.controlText('Non-Fullscreen'); } else { this.controlText('Fullscreen'); - } } /** * Handles click for full screen diff --git a/src/js/player.js b/src/js/player.js index cff3ee3c59..97b031ee35 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -1114,8 +1114,6 @@ class Player extends Component { this.addClass('vjs-fullscreen'); } else { this.removeClass('vjs-fullscreen'); - // Making sure the Control text reverts back after pressing ESC - this.controlBar.fullscreenToggle.controlText('Fullscreen'); } } From 3bd02d7b2a204acdb21632efef691b150ecd63a0 Mon Sep 17 00:00:00 2001 From: Vineet Date: Tue, 2 Aug 2016 18:21:38 -0400 Subject: [PATCH 05/10] Making proposed changes --- src/js/clickable-component.js | 7 +------ src/js/control-bar/fullscreen-toggle.js | 3 ++- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/js/clickable-component.js b/src/js/clickable-component.js index cb26e296a0..e16f1a09c4 100644 --- a/src/js/clickable-component.js +++ b/src/js/clickable-component.js @@ -28,7 +28,7 @@ class ClickableComponent extends Component { this.on('click', this.handleClick); this.on('focus', this.handleFocus); this.on('blur', this.handleBlur); - player.on('fullscreenchange', this.handleFullscreenChange); + //player.on('fullscreenchange', this.handleFullscreenChange); } /** @@ -168,11 +168,6 @@ class ClickableComponent extends Component { * @method handleClick */ handleClick() {} - - /** - - */ - handleFullscreenChange() {} /** * Handle Focus - Add keyboard functionality to element diff --git a/src/js/control-bar/fullscreen-toggle.js b/src/js/control-bar/fullscreen-toggle.js index 505d2035c4..b5b31fb849 100644 --- a/src/js/control-bar/fullscreen-toggle.js +++ b/src/js/control-bar/fullscreen-toggle.js @@ -35,7 +35,8 @@ class FullscreenToggle extends Button { this.controlText('Non-Fullscreen'); } else { this.controlText('Fullscreen'); - } + } + } /** * Handles click for full screen * From 18974c45ec2b0e3824dea430381afbd2b1b108dd Mon Sep 17 00:00:00 2001 From: Vineet Date: Tue, 2 Aug 2016 18:25:54 -0400 Subject: [PATCH 06/10] Removing test commented code --- src/js/clickable-component.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/js/clickable-component.js b/src/js/clickable-component.js index e16f1a09c4..4b4c830165 100644 --- a/src/js/clickable-component.js +++ b/src/js/clickable-component.js @@ -28,7 +28,6 @@ class ClickableComponent extends Component { this.on('click', this.handleClick); this.on('focus', this.handleFocus); this.on('blur', this.handleBlur); - //player.on('fullscreenchange', this.handleFullscreenChange); } /** From 8053aabf4cfc2c75cd0cfc783754a5e2f9a03c7a Mon Sep 17 00:00:00 2001 From: Vineet Date: Fri, 5 Aug 2016 14:29:13 -0400 Subject: [PATCH 07/10] Making the necessary changes and adding a test --- test/unit/controls.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/unit/controls.test.js b/test/unit/controls.test.js index 64afba1925..9d859e6180 100644 --- a/test/unit/controls.test.js +++ b/test/unit/controls.test.js @@ -122,3 +122,14 @@ QUnit.test('Fullscreen control text should be correct when fullscreenchange is t player.trigger('fullscreenchange'); QUnit.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); }); + +test('Fullscreen control text should be correct when fullscreenchange is triggered', function() { + var player = TestHelpers.makePlayer(); + var fullscreentoggle = new FullscreenToggle(player); + player.isFullscreen(true); + player.trigger('fullscreenchange'); + equal(fullscreentoggle.controlText(), 'Non-Fullscreen', 'Control Text is correct while switching to fullscreen mode'); + player.isFullscreen(false); + player.trigger('fullscreenchange'); + equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); +}); From ea10b7857dbf8a7e86fb3df607af3cc6ebeffa0e Mon Sep 17 00:00:00 2001 From: Vineet Date: Mon, 8 Aug 2016 14:39:11 -0400 Subject: [PATCH 08/10] Adding the correct style to the test --- test/unit/controls.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/unit/controls.test.js b/test/unit/controls.test.js index 9d859e6180..273d8c081c 100644 --- a/test/unit/controls.test.js +++ b/test/unit/controls.test.js @@ -123,13 +123,13 @@ QUnit.test('Fullscreen control text should be correct when fullscreenchange is t QUnit.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); }); -test('Fullscreen control text should be correct when fullscreenchange is triggered', function() { - var player = TestHelpers.makePlayer(); - var fullscreentoggle = new FullscreenToggle(player); +Qunit.test('Fullscreen control text should be correct when fullscreenchange is triggered', function() { + const player = TestHelpers.makePlayer(); + const fullscreentoggle = new FullscreenToggle(player); player.isFullscreen(true); player.trigger('fullscreenchange'); - equal(fullscreentoggle.controlText(), 'Non-Fullscreen', 'Control Text is correct while switching to fullscreen mode'); + QUnit.equal(fullscreentoggle.controlText(), 'Non-Fullscreen', 'Control Text is correct while switching to fullscreen mode'); player.isFullscreen(false); player.trigger('fullscreenchange'); - equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); + QUnit.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); }); From 7afa4a0c230f1394ae9f1cdf256d74ef80f1c3c2 Mon Sep 17 00:00:00 2001 From: Vineet Date: Mon, 8 Aug 2016 14:45:25 -0400 Subject: [PATCH 09/10] Removing duplicate instance of same test --- test/unit/controls.test.js | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/test/unit/controls.test.js b/test/unit/controls.test.js index 273d8c081c..d5c3fec667 100644 --- a/test/unit/controls.test.js +++ b/test/unit/controls.test.js @@ -121,15 +121,4 @@ QUnit.test('Fullscreen control text should be correct when fullscreenchange is t player.isFullscreen(false); player.trigger('fullscreenchange'); QUnit.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); -}); - -Qunit.test('Fullscreen control text should be correct when fullscreenchange is triggered', function() { - const player = TestHelpers.makePlayer(); - const fullscreentoggle = new FullscreenToggle(player); - player.isFullscreen(true); - player.trigger('fullscreenchange'); - QUnit.equal(fullscreentoggle.controlText(), 'Non-Fullscreen', 'Control Text is correct while switching to fullscreen mode'); - player.isFullscreen(false); - player.trigger('fullscreenchange'); - QUnit.equal(fullscreentoggle.controlText(), 'Fullscreen', 'Control Text is correct while switching back to normal mode'); -}); +}); \ No newline at end of file From 7ea58fe3a646e2572380a224e46ad5528bb3f994 Mon Sep 17 00:00:00 2001 From: Vineet Date: Tue, 9 Aug 2016 16:59:20 -0400 Subject: [PATCH 10/10] Adding a new line as requested --- src/js/control-bar/fullscreen-toggle.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/js/control-bar/fullscreen-toggle.js b/src/js/control-bar/fullscreen-toggle.js index b5b31fb849..fe03c72f8c 100644 --- a/src/js/control-bar/fullscreen-toggle.js +++ b/src/js/control-bar/fullscreen-toggle.js @@ -16,6 +16,7 @@ class FullscreenToggle extends Button { super(player, options); this.on(player, 'fullscreenchange', this.handleFullscreenChange); } + /** * Allow sub components to stack CSS class names *