search

danielcebrian / rangeslider-videojs

This is a plugin to create a range slider to select a region of the video
Apache License 2.0
79 stars 69 forks source link

Update rangeslider to videojs v5 #22

Open Billybobbonnet opened 8 years ago

Billybobbonnet commented 8 years ago

Hello Daniel,

First, congratulations for this plugin. I plan to update it to videojs v5. I already modified the new skin control bar layout to have a full width progress bar, like in pre v5 skin.

Do you already have started to port rangeslider to videojs v5 or should I start from scratch?

By the way, I use your (modified) plugin in a video analysis webapp that you might be interested to see. Let me know if it is the case.

Keep up the good work :)

danielcebrian commented 8 years ago

Hello Billy,

I am not planning to upgrade the rangeslider, because I am in other project, but It would be great if you do it and share your results here to be merged.

Please, let me know if you need something to do it.

Best, Daniel

Billybobbonnet commented 8 years ago

In fact I could use some help on a specific point. I struggle on this issue: http://stackoverflow.com/questions/33389458/how-can-i-access-videojs-seekbar-and-controlbar-in-version-5

I am still new to Javascript/videoJs so I could misunderstood how you designed the plugin. Can you please have a look?

Billybobbonnet commented 8 years ago

I made some progress, as you might see on the SO question. It seems that it will be necessary to change completely the way the plugin load its components. Besides, there is a couple errors popping due to, I assume, deprecated function calls. I'll look further into it and keep you posted.

danielcebrian commented 8 years ago

great! I have seen your issue on stackoverflow and I did not taken time for looking into the 5 version of videojs. But I have seen that there is a person that has created a fork with the solution that you are looking for. Please, take a look into this: https://github.com/mikaelnousiainen/rangeslider-videojs/commit/9e98433e881e18285301b6026621ef5588094917 and if this fix your problem, I will problably do something similar to upgrade.

Billybobbonnet commented 8 years ago

@danielcebrian Thanks for the feedback. I gave a quick look into @mikaelnousiainen fork but I guess he did a quick and dirty try (no offense :smile: ) or it is a work in progress. However, he did add the videojs.registerComponent I planned to add tomorrow.

By the way, I also added several events related to the selection bar left and right handles (clicked and grabbed events). I will include them in a PR if I successfully make this work (as I hope).

Also note that in terms of layout, it is necessary to change v5 default skin to have a full width progress bar and thus fit into your plugin approach. I did the changes and can provide the CSS if needed (the final skin is a youtube player clone).

Have a nice evening!

mikaelnousiainen commented 8 years ago

@Billybobbonnet None taken :) Well, I guess the dirty part of the code are the videojs utility functions that were removed from v5 API and had to be re-implemented (in the beginning of the file) to make the code work. I really just tried to make the plugin work with a minimal set of changes. Anyway, I can verify that the plugin works perfectly with my changes using the default skin on Video.js v5, so you can use the changes as a basis for your PR. Note that there is another commit in my repo that changes videojs.extends() to videojs.extend(), which was an API change made later in the v5 release candidates.