Merge pull request #764 from ooyala/player-1257

[PLAYER-1257] Perceived Player Performance: Disabling loading spinner during player load
ooyala · Jul 18, 2017 · 279b524 · 279b524
2 parents 4c8f6f6 + 4fe0799
commit 279b524
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 2 deletions.
diff --git a/js/constants/constants.js b/js/constants/constants.js
@@ -26,6 +26,10 @@ module.exports = {
     ERROR_SCREEN: "errorScreen"
   },
 
+  CUSTOM_EVENTS: {
+    INITIAL_PLAY_REQUESTED: 'html5SkinInitialPlayRequested'
+  },
+
   SKIN_TEXT: {
     LEARN_MORE: "Learn More",
     CLOSED_CAPTION_PREVIEW: "CLOSED CAPTION PREVIEW",

diff --git a/js/controller.js b/js/controller.js
@@ -164,7 +164,9 @@ OO.plugin("Html5Skin", function (OO, _, $, W) {
       this.mb.subscribe(OO.EVENTS.ASSET_UPDATED, 'customerUi', _.bind(this.onAssetUpdated, this));
       this.mb.subscribe(OO.EVENTS.PLAYBACK_READY, 'customerUi', _.bind(this.onPlaybackReady, this));
       this.mb.subscribe(OO.EVENTS.ERROR, "customerUi", _.bind(this.onErrorEvent, this));
+      this.mb.subscribe(CONSTANTS.CUSTOM_EVENTS.INITIAL_PLAY_REQUESTED, "customerUi", _.bind(this.onInitialPlayRequested, this));
       this.mb.addDependent(OO.EVENTS.PLAYBACK_READY, OO.EVENTS.UI_READY);
+      this.mb.addDependent(CONSTANTS.CUSTOM_EVENTS.INITIAL_PLAY_REQUESTED, OO.EVENTS.PLAYBACK_READY);
       this.state.isPlaybackReadySubscribed = true;
     },
 
@@ -258,7 +260,7 @@ OO.plugin("Html5Skin", function (OO, _, $, W) {
       }
 
       this.accessibilityControls = new AccessibilityControls(this); //keyboard support
-      this.state.screenToShow = CONSTANTS.SCREEN.LOADING_SCREEN;
+      this.state.screenToShow = CONSTANTS.SCREEN.START_SCREEN;
     },
 
     onVcVideoElementCreated: function(event, params) {
@@ -669,6 +671,17 @@ OO.plugin("Html5Skin", function (OO, _, $, W) {
       }
     },
 
+    /**
+     * Handles the custom INITIAL_PLAY_REQUESTED skin event. This event is used in
+     * order to defer INITIAL_PLAY until PLAYBACK_READY has been fired. Doing this allows
+     * us to display the big play button before the player finishes loading. If the user
+     * clicks on the button before PLAYBACK_READY, the player will simply show the loading
+     * spinner and INITIAL_PLAY will be automatically fired after PLAYBACK_READY itself fires.
+     */
+    onInitialPlayRequested: function() {
+      this.mb.publish(OO.EVENTS.INITIAL_PLAY, Date.now());
+    },
+
     /********************************************************************
       ADS RELATED EVENTS
     *********************************************************************/
@@ -1119,6 +1132,8 @@ OO.plugin("Html5Skin", function (OO, _, $, W) {
       this.mb.unsubscribe(OO.EVENTS.PLAYBACK_READY, 'customerUi');
       this.mb.unsubscribe(OO.EVENTS.ERROR, "customerUi");
       this.mb.unsubscribe(OO.EVENTS.SET_EMBED_CODE_AFTER_OOYALA_AD, 'customerUi');
+      // custom skin events
+      this.mb.unsubscribe(CONSTANTS.CUSTOM_EVENTS.INITIAL_PLAY_REQUESTED, "customerUi");
     },
 
     unsubscribeBasicPlaybackEvents: function() {
@@ -1226,7 +1241,7 @@ OO.plugin("Html5Skin", function (OO, _, $, W) {
     togglePlayPause: function() {
       switch (this.state.playerState) {
         case CONSTANTS.STATE.START:
-          this.mb.publish(OO.EVENTS.INITIAL_PLAY, Date.now());
+          this.mb.publish(CONSTANTS.CUSTOM_EVENTS.INITIAL_PLAY_REQUESTED);
           break;
         case CONSTANTS.STATE.END:
           if(Utils.isAndroid() || Utils.isIos()) {

diff --git a/tests/controller-test.js b/tests/controller-test.js
@@ -180,6 +180,7 @@ OO = {
       exitFullWindow: function() {},
       exitFullWindowOnEscKey: function() {},
       onBuffered: function() {},
+      onInitialPlayRequested: function() {},
       unsubscribeBasicPlaybackEvents: function() {},
       resetUpNextInfo: function(a) {},
       showUpNextScreenWhenReady: function(a,b) {},
@@ -554,6 +555,43 @@ OO = {
     Html5Skin.findMainVideoElement.call(controllerMock, div);
     Html5Skin.findMainVideoElement.call(controllerMock, {0:videoElement});
 
+    describe('Controller testing skin initialization', function() {
+      // TODO
+      // This gets cleared up below in the "destroy" tests due to the way this
+      // test suite is currently set up. We need a lot of refactoring to fix this,
+      // so we're working around it for now
+      var mb = controllerMock.mb;
+      var messageBusSpy;
+
+      beforeEach(function() {
+        controllerMock.mb = mb;
+        messageBusSpy = sinon.spy(controllerMock.mb, 'publish');
+      });
+
+      afterEach(function() {
+        messageBusSpy.restore();
+      });
+
+      it('should show Start Screen after player created', function() {
+        Html5Skin.onPlayerCreated.call(controllerMock, 'customerUi', 'elementId', {});
+        expect(controllerMock.state.screenToShow).toBe(CONSTANTS.SCREEN.START_SCREEN);
+      });
+
+      it('should raise INITIAL_PLAY_REQUESTED event when toggling play/pause on Start Screen', function() {
+        Html5Skin.onPlayerCreated.call(controllerMock, 'customerUi', 'elementId', {});
+        Html5Skin.togglePlayPause.call(controllerMock);
+        expect(messageBusSpy.calledOnce).toBe(true);
+        expect(messageBusSpy.calledWith(CONSTANTS.CUSTOM_EVENTS.INITIAL_PLAY_REQUESTED)).toBe(true);
+      });
+
+      it('should publish INITIAL_PLAY after INITIAL_PLAY_REQUESTED', function() {
+        Html5Skin.onInitialPlayRequested.call(controllerMock);
+        expect(messageBusSpy.calledOnce).toBe(true);
+        expect(messageBusSpy.args.length).toBe(1);
+        expect(messageBusSpy.calledWith(OO.EVENTS.INITIAL_PLAY)).toBe(true);
+      });
+    });
+
     describe('Controller testing Ooyala Ads', function () {
       it('test after Ooyala ad state', function() {
         expect(controllerMock.state.afterOoyalaAd).toBe(false);