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

Improve media placeholders during upload #1757

Closed
wants to merge 1 commit into from

Conversation

cameronvoell
Copy link
Contributor

@cameronvoell cameronvoell commented Jan 7, 2020

Fixes #1544

Companion PR's

Gutenberg PR: WordPress/gutenberg#19497

Changes

  • This change is introducing deliberate, consistent styles for both light and dark mode for image and video placeholder icons and backgrounds during upload.
  • There is also some small refactoring for fetching icons in an easier to comprehend way and removing unreachable code.

To Test:

Uploading images can be tested using the WordPress Android and iOS apps or the Example App using the following steps:

  1. Add new Image or Video block
  2. Click to upload a new image from wordpress media library or from device
  3. Note that during upload we briefly see the improved placeholder.

Screenshots

Before:
image

After:
image

Video:
image-upload-gif

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@cameronvoell cameronvoell marked this pull request as ready for review January 7, 2020 22:30
@cameronvoell
Copy link
Contributor Author

@iamthomasbishop Hello! Question for you regarding #1544 - I followed almost all your "proper" styling recommendations.

One difference was that the existing video icons during upload on video block where 40x40 not 32x32, so I left them and made image icons that same size. Let me know if you think it looks alright. Also, in case it is useful for reference, the icon on the Image/Video block before an image/video is added is 28x28 which you can briefly see in the video in this PR, but that icon also has text underneath it. Thanks!

@iamthomasbishop
Copy link
Contributor

@cameronvoell This is looking much better, thank you!

One difference was that the existing video icons during upload on video block where 40x40 not 32x32, so I left them and made image icons that same size. Let me know if you think it looks alright.

That works for me — I wasn't sure if 40 would feel too large but let's roll with that for consistency.

the icon on the Image/Video block before an image/video is added is 28x28 which you can briefly see in the video in this PR, but that icon also has text underneath it.

Do you mean in between when you tap to insert and the upload starts? Is this 28x28 icon you mention different from the standard block icon (shown on the placeholder state), or are all of those 28? (For reference, I think they are all supposed to be 24, but not sure if that's what you meant).

The very brief "flash" that shows in between when the picker dismisses and the upload starts is a little jarring still, but w/ the colors feeling nicer it is much less obvious.

There is also that moment in-between when the upload finishes and the image is "rendered" on in the space, where we don't show a background at all — are we able to fill that w/ the same color as you applied in the other places?

@cameronvoell
Copy link
Contributor Author

cameronvoell commented Jan 8, 2020

Thanks for feedback @iamthomasbishop

Do you mean in between when you tap to insert and the upload starts? Is this 28x28 icon you mention different from the standard block icon (shown on the placeholder state), or are all of those 28? (For reference, I think they are all supposed to be 24, but not sure if that's what you meant).

Oops, yea I just double-checked, I meant 24x24 for the standard block icon:
image

The very brief "flash" that shows in between when the picker dismisses and the upload starts is a little jarring still, but w/ the colors feeling nicer it is much less obvious.
There is also that moment in-between when the upload finishes and the image is "rendered" on in the space, where we don't show a background at all — are we able to fill that w/ the same color as you applied in the other places?

Yea I debated trying to address all these upload UI awkward transitions at once, but I think making upload smoother overall (addressing your two points above) is different enough in functionality and the code base to warrant a separate ticket / PR from #1544 which specifically addresses the incorrect style of the background placeholder on upload, which is probably the most jarring part of the upload UI at the moment.

Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Approved via WordPress/gutenberg#19497 (review)
Great job @cameronvoell ! 🎉

Copy link
Contributor

@marecar3 marecar3 left a comment

Choose a reason for hiding this comment

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

Nice work!!! 🎉

@cameronvoell
Copy link
Contributor Author

Closing since this PR only updates gutenberg ref on master which was updated past the relevant gutenberg commit on master with this recent PR #1770

@cameronvoell cameronvoell deleted the issue/1544-upload-media-placeholder branch January 14, 2020 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refine background/icon of image block transitional state
4 participants