Don't recreate controls object on configure() calls.

Issue #1948 Change-Id: I86a10c61d704688b2830e7f08b894a98faaf68e7
shaka-project · May 30, 2019 · 5723324 · 5723324
1 parent 78d9fc8
commit 5723324
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 56 deletions.
diff --git a/ui/controls.js b/ui/controls.js
@@ -54,7 +54,7 @@ shaka.ui.Controls = class extends shaka.util.FakeEventTarget {
     /** @private {boolean} */
     this.overrideCssShowControls_ = false;
 
-    /** shaka.extern.UIConfiguration */
+    /** @private {shaka.extern.UIConfiguration} */
     this.config_ = config;
 
     /** @private {shaka.cast.CastProxy} */
@@ -145,13 +145,14 @@ shaka.ui.Controls = class extends shaka.util.FakeEventTarget {
     /** @private {shaka.ui.Localization} */
     this.localization_ = shaka.ui.Controls.createLocalization_();
 
-    this.createDOM_();
-
-    this.updateLocalizedStrings_();
-
     /** @private {shaka.util.EventManager} */
     this.eventManager_ = new shaka.util.EventManager();
 
+    // Configure and create the layout of the controls
+    this.configure(this.config_);
+
+    this.updateLocalizedStrings_();
+
     this.addEventListeners_();
 
     /**
@@ -345,6 +346,28 @@ shaka.ui.Controls = class extends shaka.util.FakeEventTarget {
   }
 
 
+  /**
+   * @param {!shaka.extern.UIConfiguration} config
+   * @export
+   */
+  configure(config) {
+    this.config_ = config;
+
+    if (this.controlsContainer_) {
+      // Deconstruct the old layout if applicable
+      shaka.util.Dom.removeAllChildren(this.controlsContainer_);
+    } else {
+      this.addControlsContainer_();
+    }
+
+    // Create the new layout
+    this.createDOM_();
+
+    this.addEventListeners_();
+  }
+
+
+  /**
   /**
    * Enable or disable the custom controls. Enabling disables native
    * browser controls.
@@ -590,8 +613,6 @@ shaka.ui.Controls = class extends shaka.util.FakeEventTarget {
     this.videoContainer_.classList.add('shaka-video-container');
     this.video_.classList.add('shaka-video');
 
-    this.addControlsContainer_();
-
     this.addPlayButton_();
 
     this.addBufferingSpinner_();
@@ -624,6 +645,14 @@ shaka.ui.Controls = class extends shaka.util.FakeEventTarget {
     this.controlsContainer_ = shaka.util.Dom.createHTMLElement('div');
     this.controlsContainer_.classList.add('shaka-controls-container');
     this.videoContainer_.appendChild(this.controlsContainer_);
+
+    this.controlsContainer_.addEventListener('touchstart', (e) => {
+      this.onContainerTouch_(e);
+    }, {passive: false});
+
+    this.controlsContainer_.addEventListener('click', () => {
+      this.onContainerClick_();
+    });
   }
 
 
@@ -639,8 +668,8 @@ shaka.ui.Controls = class extends shaka.util.FakeEventTarget {
     /** @private {!HTMLElement} */
     this.playButton_ = shaka.util.Dom.createHTMLElement('button');
     this.playButton_.classList.add('shaka-play-button');
-    this.playButton_.setAttribute('icon', 'play');
     this.playButtonContainer_.appendChild(this.playButton_);
+    this.onPlayStateChange_();
   }
 
 
@@ -755,14 +784,6 @@ shaka.ui.Controls = class extends shaka.util.FakeEventTarget {
       });
     }
 
-    this.controlsContainer_.addEventListener('touchstart', (e) => {
-      this.onContainerTouch_(e);
-    }, {passive: false});
-
-    this.controlsContainer_.addEventListener('click', () => {
-      this.onContainerClick_();
-    });
-
     // Elements that should not propagate clicks (controls panel, menus)
     const noPropagationElements = this.videoContainer_.getElementsByClassName(
         'shaka-no-propagation');

diff --git a/ui/ui.js b/ui/ui.js
@@ -40,12 +40,6 @@ shaka.ui.Overlay = class {
     /** @private {shaka.Player} */
     this.player_ = player;
 
-    /** @private {!HTMLElement} */
-    this.videoContainer_ = videoContainer;
-
-    /** @private {!HTMLMediaElement} */
-    this.video_ = video;
-
     /** @private {!shaka.extern.UIConfiguration} */
     this.config_ = this.defaultConfig_();
 
@@ -59,6 +53,10 @@ shaka.ui.Overlay = class {
       videoContainer.classList.add('shaka-mobile');
     }
 
+    /** @private {shaka.ui.Controls} */
+    this.controls_ = new shaka.ui.Controls(
+        player, videoContainer, video, this.config_);
+
     // Run the initial setup so that no configure() call is required for default
     // settings.
     this.configure({});
@@ -122,28 +120,6 @@ shaka.ui.Overlay = class {
 
     goog.asserts.assert(typeof(config) == 'object', 'Should be an object!');
 
-    const DomUtils = shaka.util.Dom;
-    // Deconstruct the old layout.
-
-    // Save the text container, so subtitles can be displayed with
-    // the new layout.
-    const textContainer =
-      this.videoContainer_.querySelector('.shaka-text-container');
-
-    // Remember whether the controls were shown
-    let shown = false;
-    let controlsContainer =
-        this.videoContainer_.querySelector('.shaka-controls-container');
-    if (controlsContainer != null) {
-      shown = controlsContainer.getAttribute('shown');
-    }
-
-    // Destroy the old layout.
-    shaka.util.Dom.removeAllChildren(this.videoContainer_);
-
-    // Add the video back in
-    this.videoContainer_.appendChild(this.video_);
-
     shaka.util.ConfigUtils.mergeConfigObjects(
         this.config_, config, this.defaultConfig_(),
         /* overrides (only used for player config)*/ {}, /* path */ '');
@@ -156,18 +132,7 @@ shaka.ui.Overlay = class {
 
     goog.asserts.assert(this.player_ != null, 'Should have a player!');
 
-    /** @private {shaka.ui.Controls} */
-    this.controls_ = new shaka.ui.Controls(
-        this.player_, this.videoContainer_, this.video_, this.config_);
-
-    controlsContainer = DomUtils.getElementByClassName(
-        'shaka-controls-container', this.videoContainer_);
-    controlsContainer.setAttribute('shown', shown);
-
-    // Add the text container back.
-    if (textContainer) {
-      this.videoContainer_.appendChild(textContainer);
-    }
+    this.controls_.configure(this.config_);
 
     this.controls_.dispatchEvent(new shaka.util.FakeEvent('uiupdated'));
   }