Add AMP compatibility for Pinterest block

[westonruter]

See #14395.

This PR adds AMP compatibility for the Pinterest block.

Given this block content:

<!-- wp:jetpack/pinterest {"url":"https://www.pinterest.com/pin/140948663310535715/"} -->
<div class="wp-block-jetpack-pinterest"><a data-pin-do="embedPin" href="https://www.pinterest.com/pin/140948663310535715/"></a></div>
<!-- /wp:jetpack/pinterest -->

<!-- wp:jetpack/pinterest {"url":"https://www.pinterest.com/pin/834784480905384404/"} -->
<div class="wp-block-jetpack-pinterest"><a data-pin-do="embedPin" href="https://www.pinterest.com/pin/834784480905384404/"></a></div>
<!-- /wp:jetpack/pinterest -->

<!-- wp:jetpack/pinterest {"url":"https://www.pinterest.com/pin/1759287342038719/"} -->
<div class="wp-block-jetpack-pinterest"><a data-pin-do="embedPin" href="https://www.pinterest.com/pin/1759287342038719/"></a></div>
<!-- /wp:jetpack/pinterest -->

The before/after results are as follows:

Non-AMP	AMP before or non-AMP without JS 👎	AMP after 👍	AMP after without JS 👍

The behavior is even improved over the non-AMP version in that it also renders fallback content when JavaScript is turned off.

Note that the AMP plugin also has basic support for rendering Pinterest embeds by registering an oEmbed handler: https://github.com/ampproject/amp-wp/blob/develop/includes/embeds/class-amp-pinterest-embed-handler.php

Nevertheless, the Jetpack implementation in this PR is far superior in that it correctly sets the width/height of the component, includes metadata, and also placeholder/fallback content.

Changes proposed in this Pull Request:

• Add AMP compatibility for Pinterest block.

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

1. Create a post.
2. Activate the AMP plugin with any template mode active.
3. Add Pinterest block(s).
4. View AMP page and confirm Pins embed similarly to non-AMP pge.

Proposed changelog entry for your changes:

• Add AMP compatibility for Pinterest block.

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17086

Scheduled Jetpack release: October 6, 2020.
Scheduled code freeze: September 29, 2020

Generated by 🚫 dangerJS against b9e4f62

[jeherve]

This works well for pins. It won't work for boards or user profiles though. I wonder if this may be worth mentioning in the docblock of the render_amp function, so our future selves don't get surprised about this?

[jeherve]

If we do not have info about the pin (like when embedding a user profile such as https://pinterest.com/anapinskywalker/), is it worth using the amp-pinterest tag? Wouldn't we be better off just outputting a link?

[westonruter]

I'm trying to embed https://pinterest.com/anapinskywalker/ in the Pinterest block but it's not doing anything. Is that expected?

Clicking Embed does nothing. So a user wouldn't be able to embed non-pins anyway, right?

But yes, it would be better to show a link if a non-pin ended up getting embedded.

Otherwise, if the only issue is the info request fails, then it still seems a bit better to render amp-pinterest rather than just a bare URL link. While the size of the Pin may be not ideal, at least it should be showing the Pin.

I'm also going to open an issue in the amphtml repo to request that the width/height of amp-pinterest be optional in the same way as they are for amp-twitter or amp-gist. It is too tedious to get the dimensions exactly right. So once the embed initializes, it should resize itself to fit its contents even if this causes some CLS. This is an instance of where AMP needs to be pragmatic.

[westonruter]

So once the embed initializes, it should resize itself to fit its contents even if this causes some CLS. This is an instance of where AMP needs to be pragmatic.

See ampproject/amphtml#10318 (comment).

[westonruter]

If we do not have info about the pin (like when embedding a user profile such as https://pinterest.com/anapinskywalker/), is it worth using the amp-pinterest tag? Wouldn't we be better off just outputting a link?

I've added a link to render as a fallback in this case: b9e4f62.

[jeherve]

I'm trying to embed https://pinterest.com/anapinskywalker/ in the Pinterest block but it's not doing anything. Is that expected?

That's odd. It seems to work well on my end:

This is a test account set up by Pinterest themselves, but you could try with just about any other profile:

[jeherve]

r213307-wpcom