Extended Blocks: add Loom as oEmbed provider

[retrofox]

This PR adds loom.com as a new oembed provider, allowing embed resources from this service.

Changes proposed in this Pull Request:

• Add loom.com as a new oembed provider

Jetpack product discussion

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

No

Testing instructions:

• Edit/Add a post
• Confirm you see the Loom variation among block options.

• Add Loom embed variation to the editor. You can use this link https://www.loom.com/share/e3dcec661c37487b818b8e3b8225ec27

• Finally, confirm the resource is properly rendered.

• Confirm, you can paste the resource and it will be automatically transformed in a core/embed block, Loom variation.

Confirm it works with WordPress greater and lower than 5.5

Proposed changelog entry for your changes:

• Add loom.com as a new oembed provider

[jetpackbot]

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

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

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.

Generated by 🚫 dangerJS against 7dfea16

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello retrofox! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D49389-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

[creativecoder]

Note, I tested with different versions...

WP 5.5 plus latest Gutenberg plugin - works as expected

WP 5.5 - works as expected

WP 5.4 (previous version) - registerBlockVariation is available, but the embed block did not use variations in this version, so the block isn't available. But the embed still works through the embed block, or by pasting a loom link.

[creativecoder]

I'm having trouble getting the embed to preview within the editor. I works fine on the front end of the site within the post/page, though.

Seems to be related to the sandbox attribute on the iframe. The iframe embed works fine pasted into an html block when I remove that attribute.

[retrofox]

Note, I tested with different versions...

WP 5.5 plus latest Gutenberg plugin - works as expected

WP 5.5 - works as expected

WP 5.4 (previous version) - registerBlockVariation is available, but the embed block did not use variations in this version, so the block isn't available. But the embed still works through the embed block, or by pasting a loom link.

I've added a fallback for versions lower than 5.5, using the editor.registerBlockType filter. It works very well for me. it would be nice if you can check it.

[retrofox]

Seems to be related to the sandbox attribute on the iframe. The iframe embed works fine pasted into an html block when I remove that attribute.

Can you provide the Loom resource (URL), please? I think it was a matter of your testing flow. Keep in mind that the HTML markup is cached and if it was not properly generated in the first attempt it will keep returning the wrong code beyond the app adds the Loop provider properly.
I say this because we are seeing the sandbox attribute. It shouldn't appear once the provider is added.

[creativecoder]

Keep in mind that the HTML markup is cached and if it was not properly generated in the first attempt it will keep returning the wrong code beyond the app adds the Loop provider properly.

I think that was it. My issues were resolved when I switched to a different test site. The loom url I'm using for testing is https://www.loom.com/share/0281766fa2d04bb788eaf19e65135184

[jeherve]

Do you happen to have some background on this addition?

• Shouldn't this be added to Core directly instead?
• In the past, we've added embeds without dealing with the block editor UI.
  

  jetpack/modules/shortcodes/others.php

  Lines 1 to 10 in f438233

  	<?php
  	/**
  	* Extra oEmbed providers that we know about and use on wpcom for feature parity.
  	*
  	* This file will be loaded even when you don't use the Shortcodes feature,
  	* as these embeds are considered safe to use on any site
  	* (and may end up embedded in Core in the future).
  	*
  	* @package Jetpack
  	*/

  
  (also see D49210-code for where this is managed on WordPress.com)

My first thought would be that the embed addition would be a good fit for Jetpack / WordPress.com, without a UI, until the embed makes it way into Core.

If we decide to add a UI in the editor, maybe we should do it for all our other embeds as well?

[retrofox]

Keep in mind that the HTML markup is cached and if it was not properly generated in the first attempt it will keep returning the wrong code beyond the app adds the Loop provider properly.

I think that was it. My issues were resolved when I switched to a different test site. The loom url I'm using for testing is https://www.loom.com/share/0281766fa2d04bb788eaf19e65135184

It works for me. 👍

[retrofox]

Thanks, Jeremy.

• Shouldn't this be added to Core directly instead?

We discussed a little bit about this and concluded adding this provider to Jetpack could be the whole implementation faster. It doesn't mean that we couldn't start adding Loom straight in the core but maybe in the following step?
We'd really like to implement Loom as soon as possible, and since it doesn't seem to have technical blockers for its implementation, Jetpack was the best candidate.

• In the past, we've added embeds without dealing with the block editor UI.

Well... maybe in the past the block editor didn't exist :-D.
I put the backend code in ./extensions/extended-blocks/core-embed because, basically, we are extending the core/embed block.
Something similar happens with core/video, core/cover and core/audio. Although the extending reasons are different, in the end, we modifying blocks that are not registered by Jetpack.

jetpack/modules/shortcodes/others.php

Lines 1 to 10 in f438233

	<?php
	/**
	* Extra oEmbed providers that we know about and use on wpcom for feature parity.
	*
	* This file will be loaded even when you don't use the Shortcodes feature,
	* as these embeds are considered safe to use on any site
	* (and may end up embedded in Core in the future).
	*
	* @package Jetpack
	*/

I didn't want to edit this file because it belongs to shortcodes. It isn't a big deal moving changes straight there.

(also see D49210-code for where this is managed on WordPress.com)

Yeah, but it's for Simple sites, correct? In that case, we'll need to add the provider to two different files?

My first thought would be that the embed addition would be a good fit for Jetpack / WordPress.com, without a UI, until the embed makes it way into Core.

That was our first thought too, but then we changed in order to reduce the landing time. Again, we can start to work on moving the whole implementation to the core. It means, if I'm not wrong, add the provider to WordPress core and also add the block variation in Gutenberg core. Is it correct?

If we decide to add a UI in the editor, maybe we should do it for all our other embeds as well?

Do you mean to add a block variation for all oEmbed blocks in Jetpack? If so, I'd happy to take over this task.

[jeherve]

Related discussion:
p1599826045429400-slack-CKZHG0QCR

adding this provider to Jetpack could be the whole implementation faster. It doesn't mean that we couldn't start adding Loom straight in the core but maybe in the following step?

👍

it's for Simple sites, correct? In that case, we'll need to add the provider to two different files?

Correct, unless we were to bring files in sync in the future. Here was an exploration:

D27182-code

It means, if I'm not wrong, add the provider to WordPress core and also add the block variation in Gutenberg core. Is it correct?

Exactly. Adding an embed to Core is usually done in two patches these days: one to add it to Gutenberg, one to add it to Core. Here is an example:

• 49083-core
• Add support for TikTok video Embeds WordPress/gutenberg#19345

Do you mean to add a block variation for all oEmbed blocks in Jetpack? If so, I'd happy to take over this task.

Yes, exactly. If we think it's a valuable addition for the Loom provider, and if you've developed the basic structure for one provider, we may as well add more in the future.

[jeherve]

This tests well. I only have one suggestion.

[jeherve]

r214751-wpcom