search

silvermine / videojs-quality-selector

MIT License
183 stars 54 forks source link

fix: Change the way we update the quality selector item to solve an issues with VideoJS 7... #30

Closed mcarriere closed 5 years ago

mcarriere commented 5 years ago

...where the menu bar would not fadeout after the quality selector was used. Also updated all the libs in package.json to the latest available.

mcarriere commented 5 years ago

Tested with VideoJS 7 and 6

mcarriere commented 5 years ago

sure, no problem! I'll split everything in multiple commits and fix my commit messages

mcarriere commented 5 years ago

hopefully, my 3 commits are now compliant with your commitlint rules. I also added a more detailed explanation to all the commits onto what I changed and why.

mcarriere commented 5 years ago

@mcarriere Thank you for the PR! Nice job. I have just a couple of feedback items for you in this review. Also, could you update the commit message subject on this commit: b17858a to be 72 characters or less?

Thanks! commit: [b17858a] subject has been fixed, apparently I can't count :/

mcarriere commented 5 years ago

thanks for accepting this :) Would it be possible to create a new release?

yokuze commented 5 years ago

Yes, we should do that soon. However, I'd like to fix #31 first.

jthomerson commented 5 years ago

@mcarriere this is available in @silvermine/videojs-quality-selector@1.2.1

mcarriere commented 5 years ago

@mcarriere this is available in @silvermine/videojs-quality-selector@1.2.1 @jthomerson Thanks a lot! Do you know why it's not on https://www.npmjs.com/package/silvermine-videojs-quality-selector ?

yokuze commented 5 years ago

@mcarriere That's the old version of the package. We moved the package to the @silvermine org a while back: https://www.npmjs.com/package/@silvermine/videojs-quality-selector

mcarriere commented 5 years ago

ok thx!