search

dominic-p / videojs-resolution-selector

Adds a resolution selector button to Video.js to allow users to manually adjust the video quality.
MIT License
88 stars 31 forks source link

changeRes not triggering #29

Closed electronicadept closed 9 years ago

electronicadept commented 9 years ago

I have used the setup recommended in your example.html file, with video.js v4.5 and the latest build available here as a zip, but all that happens when I select a different resolution is that item turns black ("selected"). I've added console logging and nothing shoes. By contrast, I get console logging events triggered for volumechange events. It's as though the clicks aren't registering in the appropriate way.

dominic-p commented 9 years ago

Interesting. I wonder if that's a compatibility issue with Video.js 4.5. Could you put together a jsFiddle, so I can take a look at it?

electronicadept commented 9 years ago

Sure, I'll work on that. Should I downgrade my video.js? Does it work with a different version? I tested it with 4.2 and that didn't work, since the mergeOptions function wasn't available.

On Wed, May 27, 2015 at 11:59 AM, Dominic notifications@github.com wrote:

Interesting. I wonder if that's a compatibility issue with Video.js 4.5. Could you put together a jsFiddle, so I can take a look at it?

— Reply to this email directly or view it on GitHub https://github.com/dominic-p/videojs-resolution-selector/issues/29#issuecomment-106036630 .

dominic-p commented 9 years ago

I haven't heard of any compatibility issues other than this one. You can try Video.js 4.4.0. That's the first version that exported the util.mergeOptions method. I have updated the minimum required version in the readme.

Let me know when that jsFiddle is ready, so I can take a look at it.

electronicadept commented 9 years ago

I'm having a tough time getting the jsfiddle working. Doesn't want to for some reason. Still working on it, but you can see the page live at http://kimochis.electronicadept.com/watch/?token=5565ea5b900c35.95705467 if you want.

On Fri, May 29, 2015 at 9:57 AM, Dominic notifications@github.com wrote:

I haven't heard of any compatibility issues other than this one. You can try Video.js 4.4.0. That's the first version that exported the util.mergeOptions method. I have updated the minimum required version in the readme.

Let me know when that jsFiddle is ready, so I can take a look at it.

— Reply to this email directly or view it on GitHub https://github.com/dominic-p/videojs-resolution-selector/issues/29#issuecomment-106870480 .

electronicadept commented 9 years ago

Here's the fiddle, but I can't find secure video samples to use, so you may need to supply your own samples. The ones I'm using on the site I'm working on are proprietary, so I can't open them up to general use.

https://jsfiddle.net/9kkncjz9/4/

Cheers, Matt

On Mon, Jun 1, 2015 at 4:06 PM, Matt Stewart matt@electronicadept.com wrote:

I'm having a tough time getting the jsfiddle working. Doesn't want to for some reason. Still working on it, but you can see the page live at http://kimochis.electronicadept.com/watch/?token=5565ea5b900c35.95705467 if you want.

On Fri, May 29, 2015 at 9:57 AM, Dominic notifications@github.com wrote:

I haven't heard of any compatibility issues other than this one. You can try Video.js 4.4.0. That's the first version that exported the util.mergeOptions method. I have updated the minimum required version in the readme.

Let me know when that jsFiddle is ready, so I can take a look at it.

— Reply to this email directly or view it on GitHub https://github.com/dominic-p/videojs-resolution-selector/issues/29#issuecomment-106870480 .

dominic-p commented 9 years ago

There seem to be some issues with the jsFiddle (it doesn't look like you are loading the plugin .js file at any point for one thing). I will see if I can get them straightened out. In the mean time though, I see that you have a typo in your changeRes handler. You are using a dot (.) instead of a plus (+) which will throw an error. I don't think this is the only error, but it's something.

dominic-p commented 9 years ago

Ok, I did some testing with the fiddle you posted. It turns out that the plugin won't function on anything less than Video.js 4.7.3. That seems to be your main issue (although once you correct that, the dot issue I mentioned before will throw an error as well). I'm sorry I had out dated info on the readme. I have bumped the minimum required versions to 4.7.

The latest stable Video.js release is 4.12.7. I would suggest that you update to that. Here is your corrected fiddle. All I did was move this plugin to a separate file (probably not necessary), reference Video.js 4.12 instead of 4.4, and fix the changeRes handler.

https://jsfiddle.net/9kkncjz9/7/

electronicadept commented 9 years ago

Thanks, man; I appreciate it. It's working fine now with 4.12.7 of video.js.

On Tue, Jun 2, 2015 at 1:35 PM, Dominic notifications@github.com wrote:

Closed #29 https://github.com/dominic-p/videojs-resolution-selector/issues/29.

— Reply to this email directly or view it on GitHub https://github.com/dominic-p/videojs-resolution-selector/issues/29#event-320474709 .

dominic-p commented 9 years ago

No problem. I'm glad it's working for you.