sampotts / plyr

A simple HTML5, YouTube and Vimeo player
https://plyr.io
MIT License
26.63k stars 2.93k forks source link

*Announcement*: v3 beta testing #766

Closed sampotts closed 6 years ago

sampotts commented 6 years ago

Hey,

I'm happy to say that the illusive v3 is now available for beta testing. I'm sorry it's taken so long but it turned into a full blown re-write in the end. The changelog is massive as you can imagine and there's quite a few breaking changes. The docs are also updated.

It also includes an ads plugin (in partnership with https://vi.ai) which we are in the final stages of finalising. It's currently undocumented but once we're happy with it then it'll be documented and publicised accordingly.

If you find any bugs then please prefix them with [v3] so I know it's the new version. Once all testing is complete and bugs fixed, then it'll go to master.

There's a preview up here: https://plyr.io/beta

Also, if you have any questions, feel free to duck into the slack workspace

Cheers Sam

panoti commented 6 years ago

Do you intend to support quality level selector for hls source? and I can't add custom control by function passing in controls option. Is it a bug?

sampotts commented 6 years ago

I'll have to investigate the HLS option. It won't be in v3 but can follow after.

With the controls issue, I'll have to take a look. Will get back to you.

adantzler commented 6 years ago

Congratulations man! This is looking really great! Just a note to say we are thankful for people like you that willing to take the time to build this. We are looking at implementing this into a new e-learning platform we are building called LearnSocially.

dacostafilipe commented 6 years ago

Great work! I integrated this in one of our sites and seems to work fine.

friday commented 6 years ago

At this time you also have to set iconUrl to get the updated svg icon sprite containing for the settings. Ex:

new Plyr(video, {iconUrl: 'https://cdn.plyr.io/3.0.0-beta.8/plyr.svg'});
sampotts commented 6 years ago

Oh good catch @friday . I'll address that.

sampotts commented 6 years ago

The icon url is fixed in the latest version. I wasn't updating the defaults.js as part of my deployment scripts. Resolved now πŸ‘

Still need to look at the controls issue. Will do ASAP.

omzi commented 6 years ago

Nice update!

intelligence commented 6 years ago

Nice work Sam! Can't wait to use this on production sites!

aolko commented 6 years ago

slack bit.ly link is dead :(

dacostafilipe commented 6 years ago

This new version does not seem to work on Internet Explorer without the polyfills.

Maybe this should be added to the documentation?

sampotts commented 6 years ago

Sorry, will check the bit.ly link.

Yep, the new version uses ES6 which IE obviously doesn’t support so the polyfills are required if you need to support IE. Will add a section on polyfilling. It’s going to be the case with most libs these days.

dacostafilipe commented 6 years ago

@sampotts Yeah, makes sense, but I use your lib on computers that don't have access to the internet. Would it make sense to add a "build with polyfills" option to your build process?

aolko commented 6 years ago

wait, how about babel? Should work.

friday commented 6 years ago

@dacostafilipe, @sampotts, @aolko

I've been thinking about this as well. For example, fetch was added as a plyr dependency after I initially started using it. I'm not using fetch due to the lack of some features XHR handles. It also isn't part of any of the es6/7/8-sets on polyfill.io. So when I updated it broke IE support.

It's beta, so these things should be expected. But it would be nice to ensure that future stable releases don't require additional undocumented polyfills. If I understand it correctly eslint-plugin-compat can be used to make the build fail if the additional polyfills aren't explicitly listed in a whitelist. In that case this could be used to make the dev aware of any additional polyfill requirements introduced since last release (tried it now, but the plugin is based on a limited blacklist and can't be trusted to report all or even most browser incompatibilities)

This could in turn be used to document additional requirements in the changelog, release notes and main page. But also possibly to maintain an always updated set of polyfills needed at polyfill.io. Blissfuljs for example does this. I think this makes it much easier for devs.

Alternatively, building with polyfills included as suggested by @dacostafilipe makes sense too. This should be less complicated with babel, but adds more of an overhead. I'm not sure how well babel and rollup actually handles pruning unused polyfills.

sampotts commented 6 years ago

I've been working a lot with ES6 recently and haven't really found an ideal solution to this. polyfill.io means relying on a third party which is never ideal and babel-polyfill / core-js massively bloat the bundle for 95% of users who don't need the polyfills. I also found babel-polyfill included a fair bit I didn't need even with it set up correctly to apparently include all that's required rather than everything. My plan, as lazy as it sounds, was to let the end users decide since inevitably you're going to have to polyfill for other libs as ES6 and newer become more prevalent. I could probably do a polyfilled bundle somehow with gulp/rollup but it was like 50k gzipped when I tried which is insane.

I can probably roll back fetch to use XHR as I'm not sure I need fetch as such anyway. Polyfill.io does include a polyfill but it's 7k and as I say, I'll work on replacing it with XHR.

sampotts commented 6 years ago

Slack link updated ... it's https://bit.ly/plyr-slack πŸ‘

Maqsyo commented 6 years ago

i have Problems with fetch too -> Safari 10.1 (mobile) doesn't know this function -> core-js won't like to implement a polyfill

Also there's a Problem with "find" -> IE11 doesn't support this function. But for this problem i can use core-js

v3.0.11-beta used with babel and ES6

sampotts commented 6 years ago

I'll document the polyfills but it's pretty common practice now with ES6. As I mentioned above, you'll notice the demo uses polyfill.io...

sampotts commented 6 years ago

OK, just pushed 3.0.0-beta.12 with:

Next up is to fix captions on IE11 and the custom controls.

For anyone that wants the easy polyfill option...

<script src="https://cdn.polyfill.io/v2/polyfill.min.js?features=es6,Array.prototype.includes,CustomEvent"></script>

I'll make a more comprehensive list of the polyfill features required at some point before release but that string works well for now.

@dacostafilipe I will look at making a polyfilled built JS option. This should be possible.

@aolko Babel can handle polyfills and that's probably what I'll end up using for the polyfilled build. The problem seems to be most of their polyfills are huge. It at least doubled the size of Plyr when I tried it. Bit of a shame when ~90% of users won't need most of the polyfills.

sampotts commented 6 years ago

Hey,

I've just pushed v3.0.0-beta.13 which contains:

Will look at IE11 captions next.

Cheers πŸ‘

gehaktmolen commented 6 years ago

following polyfills could fix the fetch and promise problems: npm install whatwg-fetch --save-dev npm install es6-promise --save-dev

friday commented 6 years ago

@gehaktmolen: Fetch isn't used anymore.

dacostafilipe commented 6 years ago

After @sampotts post regarding polyfills, I did some tests of my own.

I saw a 75% overhead when using babel-polyfill, obviously, this is not usable in production. (think about the idea of all libs/frameworks including polyfills ... scary)

For now, we will keep v2 for our "offline" sites and use v3 + polyfill.io on our online sites.

That said, I would add an build option for v3+polyfills for those special cases. I could create a PR if needed.

sampotts commented 6 years ago

Yeah I will work out a build that includes polyfills. I'll probably try and use core-js and include just the polyfills that are needed or use babel-polyfill and babel-preset-env to try and limit the polyfills. I'll work something out...

sampotts commented 6 years ago

Just pushed v3.0.0-beta.14 which contains:

Will look at IE11 captions today or tomorrow.

sampotts commented 6 years ago

Oh, thanks to @friday for the work on the unminified builds πŸ‘

friday commented 6 years ago

Glad I could help :)

Forgot to mention, if someone has gotten a [object%20Object] network request error (trying to get the icon svg), then this could be due to uglifyjs. It's not really a plyr bug since the error is introduced later. It has only happened to me using uglifyjs with mangle enabled (the default) and using the minified plyr build as a source. Either using the non-minified build which is now available in v3.0.0-beta.14 as dist/plyr.js, disabling mangle or switching to uglify-es (although it's significantly slower) seems to work.

mariusgnicula commented 6 years ago

Awesome job @sampotts !! Really appreciate this player, man! I saw you added a quality switcher for Youtube, any idea when that's coming for HTML5? Thanks again, bro!

friday commented 6 years ago

Small issue: The progress bar is growing and shrinking as the time changes. This is seemingly because the new font isn't monospace and hence digits 1 and 7 are not as wide as 0 and 8, causing noticeable jumps.

Switching back to the old font fixes it, but obviously doesn't look quite the same:

.plyr--video .plyr__time {
  font: 600 13px 'avenir';
}

You can also force the new font to be "monospace" for modern browsers it seems (this was news to me). If I understand it correctly this can also use different glyphs within the same font (and does):

.plyr--video .plyr__time {
  font-variant-numeric: tabular-nums;
}

https://css-tricks.com/almanac/properties/f/font-variant-numeric/

sampotts commented 6 years ago

This last solution seems like a good one. Good find! πŸ‘ I'll give it a go. The new font is really only for use in the demo as that's all it's licensed for anyway.

sampotts commented 6 years ago

@friday I included your typeface change. I actually set it as a global declaration on the whole player as we present timestamps in a few spots and I actually preferred now the numerics looked with it.

3.0.0-beta.18 contained several fixes for issues with .destroy() leaving event bindings on the document causing issues (as you can imagine). Also fixes for trying to set menus when there's no menus in the controls and some improvements to the ad plugin from @friday and @gehaktmolen πŸ‘

3.0.0-beta.19 contains the above fix for the timestamps font spacing, thanks to @friday.

friday commented 6 years ago

Nice! You've been working hard lately :) I think you have fixed all known v3 issues now? and obviously some that were present in v2 before.

sampotts commented 6 years ago

I think so. Closing all the tickets related to bugs fixed in v3 might take me a while. I think I'll push current master to a v2 branch and then merge beta to master tomorrow night and prepare for the backlash πŸ˜‚

Maqsyo commented 6 years ago

3.0.0-beta.19 still Problems with destroy() on HTMLAudioElement

Uncaught TypeError: Cannot read property 'display' of null
    at e.timeUpdate (plyr.min.js:1)
    at HTMLAudioElement.<anonymous> (plyr.min.js:1)
sampotts commented 6 years ago

@Maqsyo Try 3.0.0-beta.20 - I found in some cases I had to delay the garbage collection of old elements.

Maqsyo commented 6 years ago

Thanks! this worked for me :)

sampotts commented 6 years ago

Thanks for all your feedback and help. I'm gonna close this for now but if you find anything, please don't hesitate to grab me in Slack or raise an issue if need be! πŸ‘