External Media: Don't append to Coblocks replace button

[obenland]

Fixes #17476.

Changes proposed in this Pull Request:

• Very rudimentarily checks if the media button render function contains the coblocks textdomain and doesn't append external media links if it does.

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

No

Before	After

Testing instructions:

• Go to the Editor
• Insert an image block and select an image.
• Click on "Replace" in the block toolbar and make sure External Media is still available there.
• Check the sidebar and make sure External Media links are no longer there.

Proposed changelog entry for your changes:

• External Media: Fixed a conflict with CoBlock's image replace feature.

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello obenland! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D51230-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

[jetpackbot]

Scheduled Jetpack release: November 10, 2020.
Scheduled code freeze: November 3, 2020

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

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 393a16b

[mreishus]

• ✔️ It works correctly.
• (Minor) The .indexOf() compared to -1 stuff can be replaced with .includes() in both forms:

> "hamburger".includes("burger")
true
> [1, 10, 15].includes(10)
true

• (Minor) If there isn't a render prop, it crashes when trying to call .toString() on null. But probably not an issue since you need a render prop to use <MediaLibrary>.
• ❔ Using render.toString() and looking for 'coblocks' in the string seems questionable (brittle, potentially slow), but there aren't a lot of other options.

The coblocks call looks like this:

								<MediaUpload
									allowedTypes={ [ 'image' ] }
									onSelect={ onSelectImage }
									value={ props.url }
									render={ ( { open } ) => (
										<Button
											isSmall
											isSecondary
											onClick={ open }>
											{ __( 'Replace Image', 'coblocks' ) }
										</Button>
									) }
								>
								</MediaUpload>

So it finds the 'coblocks' they happen to use in a translation function. But what if they replaced the text with an icon? It would stop working. But there isn't enough information elsewhere to go off of.

• Possibility 1: Change the coblocks call to <MediaUpload> to add a prop like context="coblocks". Doesn't seem likely to happen since it's not our code.
• Possibility 2: Go the other direction and make all of our calls to <MediaUpload> add a includeExternalSource={true} prop. Also, doesn't seem that good.

So I see the shortcomings but I can't really think of anything better. 🤷‍♂️ .

In my testing with performance.now(), converting the string to a function and looking for the word 'coblocks' in it took 0.01 milliseconds most of the time, and ~2 milliseconds sometimes.

[marekhrabe]

(Minor) The .indexOf() compared to -1 stuff can be replaced with .includes() in both forms

IE11 doesn't support this. I'm not 100% sure whether we polyfill this or not, that would need to be checked in the repository or the Gutenberg project

[obenland]

r216518-wpcom