Pym.js AMP-compatibility

[claudiulodro]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR adds basic AMP-compatibility to Pym.js embeds. This is a "good enough" solution until native pym.js AMP compatibility. In general, embeds are designed rectangular, so there will be a little extra padding underneath the amp-iframe, as this renders a square container. We don't have access to the intended height of the iframe from only the src url, and we don't know the exact rectangle ratio so square is a safe choice.

An alternate approach I considered that would always "work correctly" is to detect whether a page is using Pym.js embeds, and if the page is using Pym.js embeds force the page to render in non-AMP. That is also not super ideal, but let me know if you think that's a better solution.

How to test the changes in this Pull Request:

1. Install and activate Pym.js plugin through the Newspack area of the Plugins screen.
2. Add the following raw content (in details) to a page:

<!-- wp:paragraph -->
<p><a title="Atque." href="http://hills.com/soluta-corrupti-inventore-in-nulla-nisi-fuga">Minima assumenda sed est natus ut.</a> quidem optio neque eum Odio sed sapiente ut recusandae molestiae. Sed laboriosam quas veniam. Error soluta vel Est eum dicta nesciunt quaerat saepe. Quaerat aut et officia Esse deserunt voluptate ea possimus Nihil quia ut doloribus dicta repudiandae. Dolorem soluta perferendis odit. Quia ut consequuntur eum ea harum. Quia expedita voluptates repellat omnis est. Corporis cum adipisci ut. Laboriosam voluptatibus eligendi recusandae ducimus molestias. voluptas adipisci ipsam. Corporis voluptatibus unde facilis repellendus quia neque <a title="Expedita ipsam alias nisi tenetur dolorem." href="http://www.hahn.com/nemo-tenetur-debitis-porro-aut-perferendis-esse-facere">Impedit illo non dignissimos nemo</a></p>
<!-- /wp:paragraph -->

<!-- wp:html -->
<div id="senate" data-pym-src="https://s3.us-west-2.amazonaws.com/projects.oklahomawatch.org/oklahoma-legislature/senate.html"></div>
<!-- /wp:html -->

<!-- wp:paragraph -->
<p>Quasi nihil in assumenda qui amet eum Et ut qui iusto maiores. Rem sint unde quasi. Earum pariatur sunt sed consequatur <a href="http://luettgen.com/enim-molestias-hic-officiis-incidunt-corporis-vel">Expedita</a> omnis harum <a href="http://fay.com/dolor-libero-enim-ducimus-ut.html">dolores. Blanditiis</a> temporibus rerum debitis quia ipsam Quaerat distinctio est voluptatem aut expedita. <a href="http://hane.info/">rerum ut numquam laborum</a> Aut temporibus harum Sed suscipit ut nam. Excepturi aut assumenda accusantium voluptates. Veniam amet est optio harum et Ut voluptatem inventore enim velit itaque. qui delectus deleniti corporis. <a href="http://price.biz/dolor-dolor-hic-vel-ea-unde-architecto-nesciunt.html">molestiae</a> sed non. Et voluptate sunt dicta sit voluptas deserunt in. beatae natus quos ad. Necessitatibus iste molestiae harum <a href="https://anderson.org/eum-ad-minima-error-numquam-alias.html">Eaque et eum et nulla. Est est ab</a> magnam. Eos eaque eligendi ut illum molestias. Omnis totam quisquam temporibus sit sit. Cum consequatur omnis nobis officia.</p>
<!-- /wp:paragraph -->

<!-- wp:pym-shortcode/pym {"src":"https://s3.us-west-2.amazonaws.com/projects.oklahomawatch.org/oklahoma-legislature/senate.html"} -->
<a href="https://s3.us-west-2.amazonaws.com/projects.oklahomawatch.org/oklahoma-legislature/senate.html" class="wp-block-pym-shortcode-pym">https://s3.us-west-2.amazonaws.com/projects.oklahomawatch.org/oklahoma-legislature/senate.html</a>
<!-- /wp:pym-shortcode/pym -->

<!-- wp:paragraph -->
<p>Quasi nihil in assumenda qui amet eum Et ut qui iusto maiores. Rem sint unde quasi. Earum pariatur sunt sed consequatur <a href="http://luettgen.com/enim-molestias-hic-officiis-incidunt-corporis-vel">Expedita</a> omnis harum <a href="http://fay.com/dolor-libero-enim-ducimus-ut.html">dolores. Blanditiis</a> temporibus rerum debitis quia ipsam Quaerat distinctio est voluptatem aut expedita. <a href="http://hane.info/">rerum ut numquam laborum</a> Aut temporibus harum Sed suscipit ut nam. Excepturi aut assumenda accusantium voluptates. Veniam amet est optio harum et Ut voluptatem inventore enim velit itaque. qui delectus deleniti corporis. <a href="http://price.biz/dolor-dolor-hic-vel-ea-unde-architecto-nesciunt.html">molestiae</a> sed non. Et voluptate sunt dicta sit voluptas deserunt in. beatae natus quos ad. Necessitatibus iste molestiae harum <a href="https://anderson.org/eum-ad-minima-error-numquam-alias.html">Eaque et eum et nulla. Est est ab</a> magnam. Eos eaque eligendi ut illum molestias. Omnis totam quisquam temporibus sit sit. Cum consequatur omnis nobis officia.</p>
<!-- /wp:paragraph -->

<!-- wp:shortcode -->
[pym src='https://s3.us-west-2.amazonaws.com/projects.oklahomawatch.org/oklahoma-legislature/senate.html']
<!-- /wp:shortcode -->

3. View on the frontend in AMP and regular mode. You should see an interactive map each place there is a Pym.js embed.

.

Note: If you're getting errors because of double-defined newspack_is_amp, check out this branch of the theme.

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

[claudiulodro]

Offered this PR upstream to the pym.js plugin: Automattic/pym-shortcode#59 (comment)

[claudiulodro]

@westonruter if you have a chance, I'd appreciate any thoughts on this approach. It's pretty straightforward. I'd also be curious if you know a good solution to making amp-iframe height equal to the height of the inner iframe's content; something like layout='fit' would be cool (good feature idea for the next version of AMP! :D).

[westonruter]

I'd also be curious if you know a good solution to making amp-iframe height equal to the height of the inner iframe's content; something like layout='fit' would be cool (good feature idea for the next version of AMP! :D).

Yes, that is supported… but it depends on the document in the iframe window sending this message:

window.parent.postMessage({
  sentinel: 'amp',
  type: 'embed-size',
  height: document.body.scrollHeight
}, '*');

Without the embedded window sending such a message, there is no way for the host page to “see” into the window to determine its dimensions. This is why Pym.js exists, actually.

For more, see the documentation on amp-iframe resizing.

[westonruter]

It may be possible for you to inject that logic yourself if you have the interactives on the local filesystem:

https://gist.github.com/westonruter/73412f3e35cdeaade808f3b8da9d9586#file-pymjs-amp-injected-js
https://gist.github.com/westonruter/73412f3e35cdeaade808f3b8da9d9586#file-pymjs-testing-php-L96-L119

If the file is located on a remote server requiring HTTP request to fetch, then this would not be a good approach, because the interactive would need to be cached in a transient. When embedding an interactive for things like election results, such added latency is not ideal 😄

[claudiulodro]

Thanks for the feedback @westonruter! I've implemented the changes you requested and verified the resizing works well on pages that include your above JS snippet.

@jeffersonrabb ping me for instructions if you want to test the resizing.

[claudiulodro]

Don't merge this; I'm going to move the functionality to the Pym.js plugin. Automattic/pym-shortcode#59 (comment)

[claudiulodro]

Automattic/pym-shortcode#62

[benlk]

We've merged Automattic/pym-shortcode#62 and are proceeding to get that released to wp.org.

[benlk]

Released: https://github.com/INN/pym-shortcode/releases/tag/v1.3.2.3

[claudiulodro]

Thanks for getting that AMP-compatibility work released @benlk!

I've removed the handling from here since it's merged upstream, and this PR now just adds Pym.js Embeds to the list of supported plugins.

[matticbot]

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀