Consider Exporting videojs.MenuButton

[dominic-p]

I'm working on a plugin that will add a new menu style button similar to the captions button. All of the other properties I need seem to be available (e.g. videojs.MenuItem), but videojs.MenuButton isn't exposed.

I'm sure I could recreate the functionality by extending videojs.Button, but that would mean a lot of copy/pasting from the core. Any reason not to export videojs.MenuButton that I'm missing?

[heff]

No, that was an oversight. If you make a pull request to add it to exports I'll pull it in.
https://github.com/videojs/video.js/blob/master/src/js/exports.js

On Jul 19, 2013, at 1:09 AM, Dominic [email protected] wrote:

I'm working on a plugin that will add a new menu style button similar to the text tracks. All of the other properties I need seem to be available (e.g. videojs.MenuItem), but videojs.MenuButton isn't exposed.

I'm sure I could recreate the functionality by extending videojs.Button, but that seems less than ideal. Any reason not to export videojs.MenuButton that I'm missing?

—
Reply to this email directly or view it on GitHub.

[dominic-p]

This might not be as simple as I thought actually. When creating the menu items, you have to call new videojs.MySpecialMenuItem(), but in the context of the createItems function you don't have a readily accessible player object to pass to the constructor of videojs.MenuItem(). In the core, there's this.player_, but that is obfuscated when plugins run.

Of course, I could add the player object as a property on my plugin's videojs.MenuButton, but that would mean duplicating it, and I don't know what issues that might create (performance?). Or, I could call my own create items function, pass it the player object and then manually call menu.addItem() for each item, but that would mean duplicating some core functionality.

Am I missing something here, or should I just suck it up and use one of the work-arounds above?

[heff]

Anywhere this.player_ exists there should also be this.player() exposed. Have you tried using that to reference the player?

[dominic-p]

Ahh, I missed this.player(), thanks for pointing that out.

[heff]

No problem

On Jul 22, 2013, at 6:41 PM, Dominic [email protected] wrote:

Ahh, I missed this.player(), thanks for pointing that out.

—
Reply to this email directly or view it on GitHub.

[dominic-p]

@heff, sorry to resurrect a dead issue, but I think I spoke too soon. I was developing using the unminified source before, but when I tested it on the minified version, this.player() was nowhere to be found.

I tried adding it to exports.js and it still came up undefined. This might be related to the issues I'm having building the project, but I'm not sure. It's almost as if, the subclasses aren't inheriting the player() function from vjs.Component. And, when I call videojs.Component.prototype.player (exactly as I exported it) I get a property undefined error. Strange.

[heff]

It looks like component.player was never exported. Just exported. Try it now?

[dominic-p]

Thanks for taking a look. For some reason this.player() is still undefined where I'm trying to use it. I need a player reference at two points in my code. First, when I define the MenuItems, and second when I define the click behaviors for the menu items. In both contexts, this.player() is not defined.

I would think that both of those classes should inherit the player function from vjs.Component, but they don't for some reason. As you can see, it's easy enough to work around the issue for now by just copying the player object to a class property in the init functions, but that doesn't seem right to me. Thoughts?

[heff]

Ok, I'm probably gonna need you show a full example of what you're doing. I just ran the following test and player() was available in both instances.

  videojs.TestComp = videojs.Component.extend({
    init: function(player, options){
      videojs.Component.call(this, player, options);

      ok(this.player());
    }
  });

  var asdf = new videojs.TestComp(player);
  ok(asdf.player());

[dominic-p]

That's interesting. In your first test, does the this.player() return undefined? That's what I'm getting, but that doesn't seem wrong given that a player hasn't been setup yet.

Did you have a chance to look at the 2 bits of code I linked in the previous message? That would be my full example. On both of those lines, I would like to change this.player to this.player(), but for some reason the function isn't defined in that context. I'll try to work up a more stripped down example. I'll post an update when I have that ready.

[heff]

This line could be breaking it. It's overwriting the player() method with a static ref to player.
https://github.com/dominic-p/videojs-resolution-selector/blob/v0.9/video-quality-selector.js#L165

[heff]

Same with this one.
https://github.com/dominic-p/videojs-resolution-selector/blob/v0.9/video-quality-selector.js#L76

When you run the super init _V_.MenuItem.call() it will eventually run the Component init which makes player() work, but before that's called player() won't return anything. And if you override this.player manually, this.player() will never work.

[dominic-p]

Oh man, I feel dumb. That was it. Thanks for taking the time to discover that. I really appreciate it. The code is working as expected now.

[heff]

Awesome! No problem.

[raidpipe]

hello guys i am about to add resolution selector to my site having video.js player, do i need to create different copies for each video resolution ???

[dominic-p]

@raidpipe, take a look at the example file for the plugin. You should setup your video files to match that.