Cover Block: Show spinner while uploading

[cpapazoglou]

Description

Fixes #23206

• Adds a spinner while uploading media in Cover Block

How has this been tested?

• Throttle network to Slow 3G (use devtools).
• Add new Post
• Add Cover block - see placeholder

Image

• Click "Upload" and upload an image.
• You should be able to see the image being uploaded with a spinner and opacity
• After the image is uploaded, spinner should be removed and opacity returned to normal

Video

• Click "Upload" and upload a video.
• You only see a spinner and opacity
• After the video is uploaded, spinner should be removed and opacity returned to normal

Screenshots

Image	Video

Types of changes

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR.

[ntsekouras]

Thanks for working on this @cpapazoglou 👍

I haven't checked the changes, but If I recall correctly there have been discussions about inner spinners, that could make the UI 'weird/confusing' if there are lots of them.

Probably not the case here since it's shown on Image block as well, but..

@jasmussen @mapk - can you share your thoughts here? Thanks.

[jasmussen]

Thank you for the PR!

The spinner conflicts a little bit with the text in the cover block, but given it's the same behavior for the image block when uploading, this may be an acceptable interim step towards a better spinner/upload design overall (such as showing an upload progressbar as a blue line at the top of the block).

Question, though: does #25171 conflict with this?

[cpapazoglou]

@jasmussen Since we have included the global spinner when / if that changes it will be inherited in the cover block too.
Regarding #25171 , yeah we may have a conflict in packages/block-library/src/cover/edit.js. It can be resolved easily though.

[mapk]

I agree the visual conflict with the text placeholder makes this a bit odd. Is there any CSS positioning that can be done to move the spinner 12px down without adding to the complexity too much?

[cpapazoglou]

I agree the visual conflict with the text placeholder makes this a bit odd. Is there any CSS positioning that can be done to move the spinner 12px down without adding to the complexity too much?

Hey @mapk, since we don't know whether the user has added some text, would you prefer positioning the spinner to the top of the image? This seems to be the safest position.

[jasmussen]

In the shipping product right now you can upload an image and it just stays in the placeholder state until it's uploaded. To address Mark's point, perhaps we could stay in that placeholder state, in a dimmed fashion (perhaps fully white overlay) with a spinner in the center, and then only invoke the actual Cover block once the image is uploaded? I'm not sure if that would be an onerous amount of work.

[cpapazoglou]

@jasmussen In that case we would need to account for image replacements too, in which case showing the placeholder and thus hiding all the text editing being already done might confuse the user and lead them to abandon / cancel? Showing the image with a spinner:

• indicates that there is an upload in progress
• lets user use / edit the block

How would you feel placing the spinner in the top?

Personally, I prefer it behind the text because the user can perceive it easily ;-)!

[jasmussen]

Good point. Though the spinner is barely visible in that case.

The preview of the image is less important in the transient state, what happens if you increase the opacity of the scrim radically? I.e. what if the white layer above is 80% opaque? (Or, if you'r making the image transparent, what if the image in the transient state is only 20% visible)?

[getdave]

I tested this using both image and video and using a variety of backgrounds and opacity settings. Overall it's a better experience than what's currently in master so I'm all for merging it and iterating.

Obviously there will be instances where the spinner won't show up (eg: if you set the overlay color to white and the opacity to 100%) but on the whole it covers most use cases.

Screenshot (Image)

Screencapture (Video)

It would be good to get a 👍 from @jasmussen on the Design side before this merges however.

[cpapazoglou]

Thanks a lot @getdave for the review!

Overall it's a better experience than what's currently in master so I'm all for merging it and iterating.

That is what I believe too!

[jasmussen]

This is what I see:

This is okay, and it does not preclude further iteration.

As a larger undertaking, I would very much like to see an entirely new loading experience, perhaps a progressbar at the top in the spot color of wp-admin, moving from the left of the block all the way to the right, unified across all blocks that let you upload. And perhaps a similar progress-bar inspired "spinner" replacement that works with that, for when the state is just transient without any defined temporal end. But in the mean time, this makes it the same as the image block, which is fine.

Thanks so much for the PR. Ship it!