Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oembed Update - Added instagram, better display for rich/photo #3880

Merged
merged 4 commits into from Jan 25, 2013

Conversation

Projects
None yet
6 participants
@Ruxton
Copy link
Contributor

commented Jan 22, 2013

  • Added Instagram provider
  • Added support for soundcloud https (also pull requested to ruby-oembed)
  • Switched the view to only display click to play for videos, now photos and rich embeds will work out of the box. Happy to discuss this (but whats the point in supporting oEmbed if you have to click to use your TRUSTED providers).
@Raven24

This comment has been minimized.

Copy link
Member

commented Jan 22, 2013

Yeah, the additional click to show images wasn't supposed to be there in the first place... my bad.

I think this is looking really good. Could you just add a jasmine test for the types attribute you added.
Also, would you reference the PR to ruby-oembed here somewhere, please.

@DeadSuperHero

This comment has been minimized.

Copy link
Member

commented Jan 22, 2013

Wow, thanks for the great PR!

On Tuesday, January 22, 2013, Florian Staudacher wrote:

Yeah, the additional click to show images wasn't supposed to be there in
the first place... my bad.

I think this is looking really good. Could you just add a jasmine test for
the types attribute you added.
Also, would you reference the PR to ruby-oembed here somewhere, please.


Reply to this email directly or view it on GitHubhttps://github.com//pull/3880#issuecomment-12547441.

@Flaburgan

This comment has been minimized.

Copy link
Member

commented Jan 22, 2013

Can we have a screenshot of how it looks?

@Ruxton

This comment has been minimized.

Copy link
Contributor Author

commented Jan 23, 2013

The PR to ruby oEmbed is merged now - ruby-oembed/ruby-oembed#30

Instagram
Instagram oEmbed
Soundcloud
Soundcloud oEmbed
YouTube
YouTube oEmbed
flickr
flickr oEmbed

@Ruxton

This comment has been minimized.

Copy link
Contributor Author

commented Jan 23, 2013

I'll take a look at the failing rSpec & write the jasmine test tonight

@DeadSuperHero

This comment has been minimized.

Copy link
Member

commented Jan 23, 2013

This looks terrific. Can't wait to have it merged in!

@jhass jhass referenced this pull request Jan 23, 2013

Open

Gem updates #3855

5 of 5 tasks complete
@Flaburgan

This comment has been minimized.

Copy link
Member

commented Jan 24, 2013

Btw, I don't know if it's available but if you can include DailyMotion, it's something really used in France :)

@Ruxton

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2013

Still working on sorting tests, dailymotion is good, so that'll come with the PR update too.

Daily Motion

@Flaburgan

This comment has been minimized.

Copy link
Member

commented Jan 25, 2013

Thank you !

@Ruxton

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2013

ok that should cover it, I added twitter oEmbed also.

this might start an onslaught of requests for oEmbed providers, one thing to remember is that every provider added must be trusted. It's totally possible for them to say "embed this drive-by exploit javascript" and it will, and that's the point of the click to play thing, to avoid rich/html/video embeds from embedding bad crap.

Twitter is probably the worst and you may wish to separate the adding of it along with the rest of this, without a local style for the tweet embed.
diaspora1

@movilla

This comment has been minimized.

Copy link
Contributor

commented Jan 25, 2013

0_0

Thanks

jhass added a commit that referenced this pull request Jan 25, 2013

Merge pull request #3880 from Ruxton/feature/oembed_providers
Oembed Update - Added instagram, better display for rich/photo

@jhass jhass merged commit cb640b9 into diaspora:develop Jan 25, 2013

1 check was pending

default The Travis build is in progress
Details
@jhass

This comment has been minimized.

Copy link
Member

commented Jan 25, 2013

Thank you very much!

@Flaburgan

This comment has been minimized.

Copy link
Member

commented Jan 25, 2013

We have a word for that : AWESOME.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.