NamPNQ / bower-videogular-youtube

Videogular `youtube` plugin repository for distribution on `bower`
MIT License
41 stars 32 forks source link

Broadcasting Youtube Player event #55

Closed JenjisKhan closed 8 years ago

JenjisKhan commented 8 years ago

These changes add the broadcasting of 2 events:

Elecash commented 8 years ago

$broadcast is not good for performance reasons, you should change it to $emit.

Additionally, it would be desirable to not use events. Instead of, I suggest you to use scope callbacks defined in this way on your DDO:

scope: {
    vgVideoReady: '&',
    vgVideoStateChange: '&'
}

That allow you to subscribe to a callback and you don't need to send events, which should be avoided always it's possible in AngularJS.

JenjisKhan commented 8 years ago

I'll change $broadcasting in $emit, but i can not declare an isolated scope in the directive, 'cause the needed vgSrc already has an isolated scope in DDO... ... ...right? (i'm not so skilled in angular!)

Just to get the point: you mean to share the vgVideoReady and vgVideoStateChange so in controller you can $watch the value and fire a callback when needed, don't you?

Thank you!

Elecash commented 8 years ago

Oh! Yes, you're right!

This is declared as an attribute inside vg-media which already haves an isolated scope and therefore we will have a scope collision.

What I pretend is to pass a function reference (or callback) and call it from vg-youtube. In Angular world that's usually a better scenario than events.

BTW, I corrected the code in my previous comment, I put @ when I want to put &.

JenjisKhan commented 8 years ago

Ok, I'll fix $emit so i can request a new pull in soon. It's ok?

Thank you very much @Elecash!

Elecash commented 8 years ago

Sure, that's perfect :smile:

2fdevs commented 8 years ago

Closing.

Reopen when you have this fixed and up-to-date.