Make posts responsive

Videos should play I guess as we don't do anything custom with that. We just print the embed code.

With regards to the responsiveness I am not sure. Never really looked at it, but it would be very nice to support that. I assume this would only require a CSS tweak?

Would you upload the patch for that change that you did on your site?

Something like this I guess....

I got the same error like toni4i with multiple images. So i found this very small js https://github.com/ryanburnette/responsive-instagram, which worked very fine for me.

Here is a new patch

Damn it. Missed one test

Why you are using iFrame for a responsive embed of Instagram, when instagram API gives us a responsive oEmbed solution without requirement of API Key or Client ID for free?

https://www.instagram.com/developer/embedding/

Perhaps it would be better to re-factor the instagram embed field formatter to use oEmbed instead of iFrame?

Our old dirty way without using media_entity_instagram with an instagram paragraph:

  /* @var Drupal\paragraphs\Entity\Paragraph $paragraph */
  $paragraph = $variables['paragraph'];
  if ($paragraph->getType() === 'instagram' && !$paragraph->hasField('field_media')) {
    try {
      $instagram_url = $paragraph->field_link->getValue();
      $instagram_url = $instagram_url[0]['uri'];

      $client = \Drupal::httpClient();
      $request = $client->request('GET', 'http://api.instagram.com/oembed?url=' . $instagram_url . '&omitscript=true');

      if ($request->getStatusCode() === 200) {
        $instagram_json = json_decode($request->getBody()->getContents());
        $instagram = array(
          '#markup' => (string) $instagram_json->html,
        );
        $variables['content']['instagram'] = $instagram;
        $variables['#attached']['library'][] = 'paragraphs_starterkit_instagram/instagram.embeds';
      }
    } catch(Exception $e) {}

I talked about that with slashrsm on IRC. We both think it makes sense to switch to the API approach. Would be nice if you could provide a patch for that

I will try to start working on a patch for this today. I think main problem is, that we does not request API when we render the instagram embed. Perhaps we could hold JSON response in some text field or so? Because when you have many instagram media entities in a node it blocks rendering.

Perhaps we can use https://www.drupal.org/project/json_field for storing JSON data from embed API call of each pinterest media entity. When we have only a source field we can fetch JSON directly as fallback?

I take a look at media_entity_twitter and I think it would be enoug to cache data from embedding JSON request. I will sprint on this before DrupalCamp in Munich next week if someone wants to join at Hubert Burda Media :)

No caching of requested JSON from instagram embed API yet. Hide caption enable/disable per view mode. Hope I have time to work on JSON caching service on next days.

Re-factor patch from #20. Hope now tests does'n fail.

Updated patch omitting test for iframe that has been removed and added interdiff.

Tomorrow I tink I've add a InstagramFetcher and caching like in media_entity_twitter module. Patch will follow as soon as possible.

Add a InstagramEmbedFetcher (some parameters missing yet: see - https://www.instagram.com/developer/embedding/#oembed) for getting embed for InstagramEmbedFormatter field formatter with caching.

Display of caption is configurable. InstagramEmbedFormatterTest not testet yet.

Add better error handling for InstagramEmbedFetcher. I think tests will currently fail.

I fixed some coding styles, the tests and implemented the ContainerFactoryPluginInterface for InstagramEmbedFormatter.

The hide option should also work now.

Another round of big changes. I adjusted the MediaEntity\Type\Instagram to also fetch everything from the oembed api. With that we lose the ability to get instagram tags. But we win, that all the other fields are now available without any credentials.

Nice work everyone! This is realty nice improvement. I did code review and everything looks good, I have just few nitpicks.

1. +++ b/src/InstagramEmbedFetcher.php
   @@ -0,0 +1,86 @@
   +  public function fetchInstagramEmbed($shortcode, $hidecaption = FALSE) {
   

   Nitpick: $hidecaption should be added in Interface as option too, or make it required argument.

2. +++ b/src/InstagramEmbedFetcher.php
   @@ -0,0 +1,86 @@
   +    $client = \Drupal::httpClient();
   

   We should inject this service: 'http_client'

3. +++ b/src/InstagramEmbedFetcher.php
   @@ -0,0 +1,86 @@
   +      watchdog_exception('media_entity_instagram', $e, "Could not retrieve Instagram post $shortcode.");
   

   I think it would be better to inject logger channel and use that, instead of watchdog_ function.

4. +++ b/src/Plugin/Field/FieldFormatter/InstagramEmbedFormatter.php
   @@ -96,13 +128,9 @@ class InstagramEmbedFormatter extends FormatterBase {
   +      $this->t('Caption: @hidecaption', ['@hidecaption' => $settings['hidecaption'] ? 'Hidden' : 'Visible']),
   

   "Hidden" and "Visible" should be translated too.

5. +++ b/src/InstagramEmbedFetcher.php
   @@ -0,0 +1,86 @@
   +    if ($this->cache && $cached_instagram_post = $this->cache->get($shortcode . "_" . $hidecaption)) {
   

   Nitpick: it would be nice to set cache key in custom variable and use it in both places + wrap $hidecaption in intval(), so that we have nicer format for key when $hidecaption is FALSE ("key_0" instead of "key_").

6. +++ b/src/InstagramEmbedFetcher.php
   @@ -0,0 +1,86 @@
   +      'url' => 'http://instagr.am/p/' . $shortcode . '/',
   ...
   +      $response = $client->request('GET', 'http://api.instagram.com/oembed?' . $queryParameter);
   

   Small consideration. Maybe static urls can be set in protected static properties ('http://instagr.am/p/' and 'http://api.instagram.com/oembed'). Additionally, I'm not sure can we use https instead of http?

7. Additionally, maybe we should add update hook. That should remove all width/height configuration options and add default "hidecaption", so that configuration is in sync with schema.

But overall, everything looks great and clean.

Adjusted all the comments from #37

Also:

• Implemented the maxWidth property from the instagram api.
• Adjusted the README file

The patch is now working pretty good. One thing i noticed, the layout of the posts changed. I have no clue why. Maybe these iframe method is deprecated.

New oembed implementation:

Old iframe implementation:

Perhaps Instagram change or optimize currently styling of embeded posts. We also see this or other layouts on our projects sometimes.

Also see https://www.instagram.com/developer/embedding/

New patch to get the correct extension from the filename

It would be nice to pass the shortcode to the template, too. That way, you have more options, to override it, for example for AMP or Facebook Instant Articles.
This patch adds that.

Patch works like a charm for us. Can we commit it to the module ?

LoggerChannelFactory should be defined as LoggerChannelFactoryInterface

Looks goog

Looks good

The oembed request should have a pretty short timeout on it. If the instagram oEmbed endpoint is unresponsive, you don't want to nuke your site performance waiting for eEmbed requests to timeout at the default time (prolly 30 seconds or more).

Oops, wrong patch.

Thanks everyone for the hard work.