wp-shortcake / shortcake-bakery

A fine selection of shortcodes for WordPress
42 stars 16 forks source link

instagram embeds cut off at small screen sizes #162

Open montchr opened 8 years ago

montchr commented 8 years ago

[instagram url="https://instagram.com/p/96cJ_hgNHH/?taken-by=knostikat"]

Running on a fresh site, latest versions of Shortcake and Shortcake Bakery.

Here's the embed at a desktop size:

And at a mobile screen size:

montchr commented 8 years ago

And it's worse for vertical images. This is a desktop size:

danielbachhuber commented 8 years ago

Thanks for the report.

I suppose they have some media queries in their CSS, so retaining the same proportion when scaling causes problems.

I've filed it as a bug. I don't have any great ideas on how to fix right now. Might be worth seeing if Jetpack has addressed this through some means.

montchr commented 8 years ago

I suppose they have some media queries in their CSS, so retaining the same proportion when scaling causes problems.

Thanks, makes sense.

Would it be possible to use the Instagram API oEmbed endpoint? Apparently the embed HTML is included in the response.

danielbachhuber commented 8 years ago

Would it be possible to use the Instagram API oEmbed endpoint?

Could. Related #89

IIRC, we only get the image and some basic text out of the oEmbed endpoint (e.g. no likes, branding, user avatar, etc.). As such, the iframe embed is preferable.

montchr commented 8 years ago

Actually, Jetpack is using the API oEmbed endpoint. But there's a lot of extra stuff related to caching, which probably would be good to keep.

IIRC, we only get the image and some basic text out of the oEmbed endpoint (e.g. no likes, branding, user avatar, etc.). As such, the iframe embed is preferable.

Endpoint does provide HTML, which, in this example, would be:

<blockquote class="instagram-media" data-instgrm-captioned data-instgrm-version="6" style=" background:#FFF; border:0; border-radius:3px; box-shadow:0 0 1px 0 rgba(0,0,0,0.5),0 1px 10px 0 rgba(0,0,0,0.15); margin: 1px; max-width:658px; padding:0; width:99.375%; width:-webkit-calc(100% - 2px); width:calc(100% - 2px);"><div style="padding:8px;"> <div style=" background:#F8F8F8; line-height:0; margin-top:40px; padding:50.0% 0; text-align:center; width:100%;"> <div style=" background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACwAAAAsCAMAAAApWqozAAAAGFBMVEUiIiI9PT0eHh4gIB4hIBkcHBwcHBwcHBydr+JQAAAACHRSTlMABA4YHyQsM5jtaMwAAADfSURBVDjL7ZVBEgMhCAQBAf//42xcNbpAqakcM0ftUmFAAIBE81IqBJdS3lS6zs3bIpB9WED3YYXFPmHRfT8sgyrCP1x8uEUxLMzNWElFOYCV6mHWWwMzdPEKHlhLw7NWJqkHc4uIZphavDzA2JPzUDsBZziNae2S6owH8xPmX8G7zzgKEOPUoYHvGz1TBCxMkd3kwNVbU0gKHkx+iZILf77IofhrY1nYFnB/lQPb79drWOyJVa/DAvg9B/rLB4cC+Nqgdz/TvBbBnr6GBReqn/nRmDgaQEej7WhonozjF+Y2I/fZou/qAAAAAElFTkSuQmCC); display:block; height:44px; margin:0 auto -44px; position:relative; top:-22px; width:44px;"></div></div> <p style=" margin:8px 0 0 0; padding:0 4px;"> <a href="https://www.instagram.com/p/96cJ_hgNHH/" style=" color:#000; font-family:Arial,sans-serif; font-size:14px; font-style:normal; font-weight:normal; line-height:17px; text-decoration:none; word-wrap:break-word;" target="_blank">Taking a vacation with The Doctor in the #TARDIS for a few months, see you in a couple minutes</a></p> <p style=" color:#c9c8cd; font-family:Arial,sans-serif; font-size:14px; line-height:17px; margin-bottom:0; margin-top:8px; overflow:hidden; padding:8px 0 7px; text-align:center; text-overflow:ellipsis; white-space:nowrap;">A photo posted by Chris Montgomery (@knostikat) on <time style=" font-family:Arial,sans-serif; font-size:14px; line-height:17px;" datetime="2015-11-10T17:44:03+00:00">Nov 10, 2015 at 9:44am PST</time></p></div></blockquote>
<script async defer src="//platform.instagram.com/en_US/embeds.js"></script>

Tested, looks great, no proportion issues.

montchr commented 8 years ago

Shall I open a PR using the oEmbed endpoint to address this issue?

danielbachhuber commented 8 years ago

Tested, looks great, no proportion issues.

Can you share comparison screenshots?

Shall I open a PR using the oEmbed endpoint to address this issue?

Sure — we'd consider it.

montchr commented 8 years ago

Here's the output of the Shortcake Bakery shortcode:

[instagram url="https://instagram.com/p/_FFvtTDwSG/"]

Note that the bottom of the embed is cut off, as this is a vertical image.

And here's the same Instagram post using the HTML from the oEmbed endpoint (different than the HTML from my previous comment, for comparison's sake):

<blockquote class="instagram-media" data-instgrm-captioned data-instgrm-version="6" style=" background:#FFF; border:0; border-radius:3px; box-shadow:0 0 1px 0 rgba(0,0,0,0.5),0 1px 10px 0 rgba(0,0,0,0.15); margin: 1px; max-width:658px; padding:0; width:99.375%; width:-webkit-calc(100% - 2px); width:calc(100% - 2px);"><div style="padding:8px;"> <div style=" background:#F8F8F8; line-height:0; margin-top:40px; padding:62.5% 0; text-align:center; width:100%;"> <div style=" background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACwAAAAsCAMAAAApWqozAAAAGFBMVEUiIiI9PT0eHh4gIB4hIBkcHBwcHBwcHBydr+JQAAAACHRSTlMABA4YHyQsM5jtaMwAAADfSURBVDjL7ZVBEgMhCAQBAf//42xcNbpAqakcM0ftUmFAAIBE81IqBJdS3lS6zs3bIpB9WED3YYXFPmHRfT8sgyrCP1x8uEUxLMzNWElFOYCV6mHWWwMzdPEKHlhLw7NWJqkHc4uIZphavDzA2JPzUDsBZziNae2S6owH8xPmX8G7zzgKEOPUoYHvGz1TBCxMkd3kwNVbU0gKHkx+iZILf77IofhrY1nYFnB/lQPb79drWOyJVa/DAvg9B/rLB4cC+Nqgdz/TvBbBnr6GBReqn/nRmDgaQEej7WhonozjF+Y2I/fZou/qAAAAAElFTkSuQmCC); display:block; height:44px; margin:0 auto -44px; position:relative; top:-22px; width:44px;"></div></div> <p style=" margin:8px 0 0 0; padding:0 4px;"> <a href="https://www.instagram.com/p/_FFvtTDwSG/" style=" color:#000; font-family:Arial,sans-serif; font-size:14px; font-style:normal; font-weight:normal; line-height:17px; text-decoration:none; word-wrap:break-word;" target="_blank">🔛🔝 #Philadelphia</a></p> <p style=" color:#c9c8cd; font-family:Arial,sans-serif; font-size:14px; line-height:17px; margin-bottom:0; margin-top:8px; overflow:hidden; padding:8px 0 7px; text-align:center; text-overflow:ellipsis; white-space:nowrap;">A photo posted by @odettablue on <time style=" font-family:Arial,sans-serif; font-size:14px; line-height:17px;" datetime="2015-12-09T17:31:15+00:00">Dec 9, 2015 at 9:31am PST</time></p></div></blockquote>
<script async defer src="//platform.instagram.com/en_US/embeds.js"></script>

Here, the entire embed is intact.

montchr commented 8 years ago

And if you're curious here's the API response for https://api.instagram.com/publicapi/oembed/?url=http://instagr.am/p/_FFvtTDwSG/:

{
  "provider_url": "https://www.instagram.com",
  "media_id": "1136339749667013766_179420960",
  "author_name": "odettablue",
  "height": null,
  "thumbnail_url": "https://scontent-lga3-1.cdninstagram.com/hphotos-xfa1/t51.2885-15/sh0.08/e35/p640x640/12362117_573167109503565_528046734_n.jpg",
  "thumbnail_width": 640,
  "thumbnail_height": 800,
  "provider_name": "Instagram",
  "title": "🔛🔝 #Philadelphia",
  "html": "<blockquote class=\"instagram-media\" data-instgrm-captioned data-instgrm-version=\"6\" style=\" background:#FFF; border:0; border-radius:3px; box-shadow:0 0 1px 0 rgba(0,0,0,0.5),0 1px 10px 0 rgba(0,0,0,0.15); margin: 1px; max-width:658px; padding:0; width:99.375%; width:-webkit-calc(100% - 2px); width:calc(100% - 2px);\"><div style=\"padding:8px;\"> <div style=\" background:#F8F8F8; line-height:0; margin-top:40px; padding:62.5% 0; text-align:center; width:100%;\"> <div style=\" background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACwAAAAsCAMAAAApWqozAAAAGFBMVEUiIiI9PT0eHh4gIB4hIBkcHBwcHBwcHBydr+JQAAAACHRSTlMABA4YHyQsM5jtaMwAAADfSURBVDjL7ZVBEgMhCAQBAf//42xcNbpAqakcM0ftUmFAAIBE81IqBJdS3lS6zs3bIpB9WED3YYXFPmHRfT8sgyrCP1x8uEUxLMzNWElFOYCV6mHWWwMzdPEKHlhLw7NWJqkHc4uIZphavDzA2JPzUDsBZziNae2S6owH8xPmX8G7zzgKEOPUoYHvGz1TBCxMkd3kwNVbU0gKHkx+iZILf77IofhrY1nYFnB/lQPb79drWOyJVa/DAvg9B/rLB4cC+Nqgdz/TvBbBnr6GBReqn/nRmDgaQEej7WhonozjF+Y2I/fZou/qAAAAAElFTkSuQmCC); display:block; height:44px; margin:0 auto -44px; position:relative; top:-22px; width:44px;\"></div></div> <p style=\" margin:8px 0 0 0; padding:0 4px;\"> <a href=\"https://www.instagram.com/p/_FFvtTDwSG/\" style=\" color:#000; font-family:Arial,sans-serif; font-size:14px; font-style:normal; font-weight:normal; line-height:17px; text-decoration:none; word-wrap:break-word;\" target=\"_blank\">🔛🔝 #Philadelphia</a></p> <p style=\" color:#c9c8cd; font-family:Arial,sans-serif; font-size:14px; line-height:17px; margin-bottom:0; margin-top:8px; overflow:hidden; padding:8px 0 7px; text-align:center; text-overflow:ellipsis; white-space:nowrap;\">A photo posted by @odettablue on <time style=\" font-family:Arial,sans-serif; font-size:14px; line-height:17px;\" datetime=\"2015-12-09T17:31:15+00:00\">Dec 9, 2015 at 9:31am PST</time></p></div></blockquote>\n<script async defer src=\"//platform.instagram.com/en_US/embeds.js\"></script>",
  "width": 658,
  "version": "1.0",
  "author_url": "https://www.instagram.com/odettablue",
  "author_id": 179420960,
  "type": "rich"
}
danielbachhuber commented 8 years ago

Cool, let's do it. The global $post needs to be set in order for the oEmbed call to be appropriately cached. You might need to dig into WP internals a bit to see how that's done.

montchr commented 8 years ago

Cool, let's do it.

Ok, great. I'll get started on that.

The global $post needs to be set in order for the oEmbed call to be appropriately cached.

Looks like Jetpack isn't using global $post in its Instagram shortcode at all — they're calling wp_cache_get() and wp_cache_set() independent of WP_Embed. I was planning on modelling this after the Jetpack implementation unless you think there's a better way to do caching here.

Also, Jetpack is using wp_enqueue_script() to load the embed script, then it strips the <script> tag from the embed HTML. This seems like a good way to do it, but I know that there are a few other shortcodes in Bakery that could benefit from that approach too. Is that something I should do here, or would it be better to just leave the <script> and tackle the enqueue method in another PR? I don't think there's currently an issue open for it, but that would prevent us from loading the embed scripts for each and every embed from the same provider (such as in an article with several Twitter embeds).

danielbachhuber commented 8 years ago

I was planning on modelling this after the Jetpack implementation unless you think there's a better way to do caching here.

We should follow core's caching implementation — storing the result in post meta — so we aren't dependent on a persistent object cache.

Is that something I should do here, or would it be better to just leave the <script> and tackle the enqueue method in another PR?

Sure — just for the Instagram embed on this PR.

montchr commented 8 years ago

@danielbachhuber can you clarify the end? Looks like your comment got cut off.

danielbachhuber commented 8 years ago

Oh, <script> was treated as HTML. I've updated

montchr commented 8 years ago

:+1: