Skip to content
This repository has been archived by the owner on Sep 2, 2020. It is now read-only.

Recognition of Interactive elements too restrictive #27

Open
rbraun75 opened this issue Aug 24, 2017 · 1 comment
Open

Recognition of Interactive elements too restrictive #27

rbraun75 opened this issue Aug 24, 2017 · 1 comment

Comments

@rbraun75
Copy link

rbraun75 commented Aug 24, 2017

The recognition of interactive elements in the instant article is too restrictive.

else if ((Type::is($child, Interactive::getClassName()) || Type::is($child, SocialEmbed::getClassName())) && !Type::isTextEmpty($child->getSource())) {
                    if (!$containsIframe) {
                        $containsIframe = true;
                        $context->getHead()->appendChild($this->buildCustomElementScriptEntry('amp-iframe', 'https://cdn.ampproject.org/v0/amp-iframe-0.1.js', $context));
                    }
                    $childElement = $this->observer->applyFilters('IA_INTERACTIVE', $this->buildIframe($child, $context, 'interactive', true), $child, $context);
                }

Filters with tag "IA_INTERACTIVE" are only applied when the iframe has a "src" attribute, because of && !Type::isTextEmpty($child->getSource()).

Standard Twitter, Facebook, Instagram elements for example are not getting recognized (@see https://developers.facebook.com/docs/instant-articles/reference/embeds)

<figure class="op-interactive">
  <iframe>
    <!-- Include Twitter embed code here. Your embed code should look like <blockquote class="twitter-tweet" ... -->    
  </iframe>
</figure>

Please change the recognition so that iframes without src within "op_interactive" figures are getting recognized too.

@timjacobi
Copy link
Contributor

I agree with this. Care to submit a PR?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants