search

brandly / angular-youtube-embed

:tv: Embed a YouTube player with a simple directive
http://brandly.github.io/angular-youtube-embed/
MIT License
510 stars 147 forks source link

Avoid destroying old player when video changes #145

Open ashe540 opened 7 years ago

ashe540 commented 7 years ago

This avoids destroying the player when changing the current video. It also prevents the player to stop playing when the video is finished and when user is on a different tab or window. There are a couple of pull requests open for this, but as mentioned in issue #84 this can be solved by simply checking if the player exists before creating a new one. Hopefully we can get this solution to this old issue merged soon.

brandly commented 7 years ago

does this handle playlists?

ashe540 commented 7 years ago

Added support for playlists. I also included the updated demo in my fork in the gh-pages branch. Let me know what you think.

brandly commented 7 years ago

nice! i'm gonna check this out in a bit. at first glance, this looks great!

i think this will affect how events are emitted, so i wanna verify that i understand how things behave now.

if it is changing how events are emitted, we have to treat this update as a change in functionality, and release a new major version, so there's a good chance we're looking at a 2.0.0 release with this code in it.

thanks for helping with this! i know this has been an issue for a while, and i haven't addressed it. other implementations i've seen have been a lot less direct than this one, so i appreciate you taking the time to sort things out 🌟

ashe540 commented 7 years ago

Hey, no problem! And yeah that sounds like a good idea. Let me know if I can help out in any way! :smile:

brandly commented 7 years ago

had some time to look:

looking at the docs, i think we should use the "object syntax" for both videos and playlists.

for videos, this allows us to support startSeconds and endSeconds video playerVars.

take a look at the docs for this repo. start and end can supported here, but there are other parameters, like autoplay, that once you set, you can't take back. does that make sense?

before this PR, every video change created a brand new player object with the given playerVars. i still think this PR is a step in the right direction, but we should consider these things and see how much functionality we can continue to support.

want to take a stab at supporting startSeconds and endSeconds for videos and startSeconds for playlists?

ashe540 commented 7 years ago

Yeah I get what you mean. I'll see what I can do about keeping those features intact while keeping the player object. You'll be hearing back from me 😄.

brandly commented 7 years ago

@ashe540 hey! any updates?

ashe540 commented 7 years ago

@brandly so sorry for the long silence. Unfortunately I have been terribly busy and haven't had a chance to work on this. I'm going to try to make some time during the next week or so. I'll keep you posted.

brandly commented 7 years ago

No worries! I've been rather busy too. I appreciate the help, but don't feel pressured if you don't have the time.

On Wed, Apr 12, 2017 at 6:12 AM ashe540 notifications@github.com wrote:

@brandly https://github.com/brandly so sorry for the long silence. Unfortunately I have been terribly busy and haven't had a chance to work on this. I'm going to try to make some time during the next week or so. I'll keep you posted.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/brandly/angular-youtube-embed/pull/145#issuecomment-293533531, or mute the thread https://github.com/notifications/unsubscribe-auth/AAyF2Gh43v4h5U5fuouc0dqQHJG0jWcIks5rvKNtgaJpZM4MMLWl .