search

Semantic-Org / Semantic-UI-Angular

Semantic UI Angular Integrations
MIT License
557 stars 117 forks source link

feat(video): add video directive #18

Closed mxth closed 8 years ago

mxth commented 9 years ago

This one is simple, so I decided to do it first. @m0t0r @caitp Please review

TODO; integrate Player API

mxth commented 9 years ago

@m0t0r @caitp Player API added

caitp commented 9 years ago

I haven't reviewed this yet (on mobile), but it sounds like a neat thing to have. One suggestion though, is to put media-oriented stuff in a separate script/module, similar to angular core's extra modules.

I'll give this a look in a few days

jlukic commented 9 years ago

Video is being renamed to embed and getting an overhaul in 2.0 (launching June 1st). Apologies if you had to dig through previous code, It was poorly documented and feature incomplete.

m0t0r commented 9 years ago

this PR is waiting only @caitp 's review and good to be merged

caitp commented 9 years ago

I left a bunch of comments on the first commit, I dunno if any of those were changed in subsequent commits. The directives are what I care about mainly

caitp commented 9 years ago

LGTM, but the comments should be considered for incremental improvements

m0t0r commented 9 years ago

@mxth, will you update this PR with @caitp 's comments ?