jfujita / videojs-http-source-selector

VideoJS plugin that leverages videojs-contrib-quality-levels plugin to offer manual user-select able level selection options for adaptive http streams.
MIT License
62 stars 49 forks source link

Fix SourceMenuItem not being selectable #33

Closed leonklingele closed 5 years ago

leonklingele commented 5 years ago

The menu item was not selectable as the super-constructur of SourceMenuItem (i.e. MenuItem) was called before the 'selectable' property on the 'options' object was set.

Fixes https://github.com/jfujita/videojs-http-source-selector/issues/7.

omarroth commented 5 years ago

Buttons that were previously selected won't update themselves: image

A couple additional changes appear to fix the issue if you'd like to update this PR:

diff --git a/src/components/SourceMenuButton.js b/src/components/SourceMenuButton.js
index f557c11..d810276 100644
--- a/src/components/SourceMenuButton.js
+++ b/src/components/SourceMenuButton.js
@@ -26,9 +26,9 @@ class SourceMenuButton extends MenuButton
       }
     }

     // Bind update to qualityLevels changes
-    //this.player().qualityLevels.on(['change', 'addqualitylevel'], videojs.bind( this, this.update) );
+    this.player().qualityLevels().on(['change', 'addqualitylevel'], videojs.bind(this, this.update));
   };

   createEl() {
     return videojs.dom.createEl('div', {
diff --git a/src/components/SourceMenuItem.js b/src/components/SourceMenuItem.js
index 2a30602..40cf058 100644
--- a/src/components/SourceMenuItem.js
+++ b/src/components/SourceMenuItem.js
@@ -1,20 +1,21 @@
 import videojs from 'video.js';
 const MenuItem = videojs.getComponent('MenuItem');
+const Component = videojs.getComponent('Component');

 class SourceMenuItem extends MenuItem
 {
   constructor(player, options) {
     options.selectable = true;
+    options.multiSelectable = false;
+
     super(player, options);
   }

   handleClick() {
     var selected = this.options_;
     console.log("Changing quality to:", selected.label);
-
-    this.selected_ = true;
-    this.selected(true);
+    super.handleClick();

     var levels = this.player().qualityLevels();
     for(var i = 0; i < levels.length; i++) {
       if (selected.index == levels.length) {
@@ -28,12 +29,11 @@ class SourceMenuItem extends MenuItem
     }
   }

   update() {
-    var levels = this.player().qualityLevels();
-    var selection = levels.selectedIndex;
-    this.selected(this.options_.index == selection);
-    this.selected_ = (this.options_.index === selection);
+    var selectedIndex = this.player().qualityLevels().selectedIndex;
+    this.selected(this.options_.index == selectedIndex);
   }
 }

+Component.registerComponent('SourceMenuItem', SourceMenuItem);
 export default SourceMenuItem;
leonklingele commented 5 years ago

Updated, thanks!

leonklingele commented 5 years ago

@jfujita thanks for merging this. Can you please push a new release soon? :)

jfujita commented 5 years ago

@leonklingele should be up at videojs-http-source-selector@1.1.6