Fix: Remove editor usage from media blocks.

[jorgefilipecosta]

Description

Blocks should not use editor module directly and the editor module may not available in all edition environments (e.g. in the widget screen).

This PR makes sure the mediUpload function used in the image, audio, video, and file blocks is the one provided in the block-editor settings instead of being the one from wordpress/editor.

How has this been tested?

I tried to insert all the changed blocks by dragging files in the editor and I verified thigs worked as expected.

[gziolo]

What about Gallery block? It uses mediaUpload in transforms file.

[gziolo]

I personally don't like this pattern in the current shape as I anticipate that it's going to be copied to 3rd party blocks even though we prefix the method with __experimental.

[jorgefilipecosta]

What about Gallery block? It uses mediaUpload in transforms file.

All the blocks changed in this PR had the same change. The gallery bock is a different case, so I will try to address it in a different PR.

I personally don't like this pattern in the current shape as I anticipate that it's going to be copied to 3rd party blocks even though we prefix the method with __experimental.

I think an option is to remove __experimental prefix. Another option is to add the behavior of uploading temporary URLs to MediaPlaceholder or to a new component part of block-editor, so blocks will not have a need to call a mediaUpload function directly.
Not sure what is the best option to follow here. cc @gziolo, @youknowriad.

[youknowriad]

This __experimentalMediaUpload is already existing before this PR. It's still new and for me it should stay this way for some more releases but it raises the question that we should probably do an audit of the experimental APIs and "stabilize" the ones that are here for some time and proved to be good enough.

[jorgefilipecosta]

It is also important to note that while import { mediaUpload } from '@wordpress/editor'; does not contain any experimental flag, third-party blocks may use this option instead of something similar to what I did in this PR (that contains the experimental flag), and if that's the case, we may have a problem as these blocks will not work on the widget screen.

[youknowriad]

we may have a problem as these blocks will not work on the widget screen.

By that time, both the widget screen and this API won't be considered experimental I think

[jorgefilipecosta]

This __experimentalMediaUpload is already existing before this PR. It's still new and for me it should stay this way for some more releases but it raises the question that we should probably do an audit of the experimental APIs and "stabilize" the ones that are here for some time and proved to be good enough.

What about using this __experimentalMediaUpload setting in our own blocks, is it something we are ok with for now? Or should we try to abstract this call inside a new component or inside MediaPlaceholder?

[youknowriad]

I was saying, I'm ok with us using it. For third party blocks, The experimental APIs are documented clearly, why this one should be an exception.

[gziolo]

It’s fine to merge it as a stopgap solution but it would be nice to address it before WordPress 5.3 release.

I think that the biggest downside of this approach is that it introduces a lot of complexity which would be nice to hide for plugin developers as @jorgefilipecosta mentioned thus making it an implementation detail.

[jorgefilipecosta]

This PR was updated, and now we are also addressing the gallery case, this makes the gallery way of handling file drops in line with the approach used by all other blocks.
I agree with @gziolo that there may be an opportunity to abstract this upload logic and simplify bock code, but I think we can advance with these changes which fix an existing problem and make blocks code more consistent and then we can try to extract this blob upload logic.

[youknowriad]

What about useSelect ;)

[jorgefilipecosta]

Hi @youknowriad I did not use useSelect because these are class components (where hooks could not be used), and most of the cases already had a withSelect usage. I guess if there are some benefits of the usage of the hook, we can create specific PR's with a refactor to hooks before the next block change.

[youknowriad]

Makes sense

[youknowriad]

It looks like with this change, the only other parts where we use the "core/editor" store or the editor package in the block library are:

• The "date" of the post object in the calendar block (this should probably default to "no date" if the store is not available.
• Reusable blocks APIs, two options here: maybe the solution ehre is to move these to the block editor settings (like mediaUpload).