Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update image loading state to use a spinner and a faded static image. #11876

Merged
merged 3 commits into from
Nov 15, 2018

Conversation

mtias
Copy link
Member

@mtias mtias commented Nov 14, 2018

The current implementation of "this image is being uploaded" uses a heavy pulsating effect that has not proven to be very clear but it has been distracting.

This simply removes that effect and shows a spinner at the center — consistent with how Embeds are working — while leaving the image faded out until upload completes.

screenshot 2018-11-14 at 15 38 09

Related #8810.

@mtias mtias added [Feature] Media Anything that impacts the experience of managing media [Block] Image Affects the Image Block labels Nov 14, 2018
@mtias mtias added this to the 4.4 milestone Nov 14, 2018
@noisysocks noisysocks requested a review from talldan November 15, 2018 01:02
@noisysocks
Copy link
Member

Nice! I pushed up c76ec28 which applies the same effect to the Gallery block.

@noisysocks noisysocks added the [Block] Gallery Affects the Gallery Block - used to display groups of images label Nov 15, 2018
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good improvement 👍

There's also the media and text block that could do with this improvement (that doesn't seem to have any loading state at all, which I found confusing as a user).

Thinking into the future, I wonder if there's more that can be abstracted out of the media blocks (in terms of placeholders, loading state, toolbar buttons) so that they don't have separate but very similar implementations.

@noisysocks
Copy link
Member

There's also the media and text block that could do with this improvement (that doesn't seem to have any loading state at all, which I found confusing as a user).

Ah, interesting. It looks like MediaContainer ignores the blob URL because the mime type isn't set yet. I think let's tackle that separately.

Thinking into the future, I wonder if there's more that can be abstracted out of the media blocks (in terms of placeholders, loading state, toolbar buttons) so that they don't have separate but very similar implementations.

Moving the <img> part out into its own component might be a good way of tackling #11650, too.

@noisysocks noisysocks merged commit 7db334b into master Nov 15, 2018
@noisysocks noisysocks deleted the update/image-pulsing branch November 15, 2018 05:13
@mtias
Copy link
Member Author

mtias commented Nov 15, 2018

Thanks for including galleries and merging!

I wonder if there's more that can be abstracted out of the media blocks (in terms of placeholders, loading state, toolbar buttons) so that they don't have separate but very similar implementations.

I agree. Ideally a block developer only has to use a MediaArea or something and all the consistent UX comes with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images [Block] Image Affects the Image Block [Feature] Media Anything that impacts the experience of managing media
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants