-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Try to make change image flow clearer #14142
Conversation
Some really creative ideas here, nice work! Would love some design input from @mapk or @melchoyce. Whatever we decide, it would be good to make these changes for every block that has a similar placeholder flow so that there's some consistency throughout the interface. (Let's wait to see what the design team thinks, first, though!) |
This is a great exploration, @draganescu. Upon testing, I have a few suggestions:
Here's a mockup for reference: I built a quick prototype to test the idea: |
Love that iteration, @kjellr — the action feels more intentional, if that makes sense? Plus, the added context of "this is the image you're replacing" makes the transition feel much less jarring, IMO. |
On second thought, perhaps we try a swap/replace sort of icon in the place of the image icon here? Here's an example: The SVG is here if you'd like to try it out in the PR: https://cloudup.com/cXuEmylGcCQ |
The new icon is excellent! It makes much more sense in the toolbar now. I also really like the points you've noted in number 3 above, @kjellr. @draganescu Thanks for getting us to think about this differently. Your explorations kicked off some fantastic ideas. |
This is nice but will not work when the image is smaller than the width of those buttons plus some padding or when the image is resized to be smaller than that. So there will be cases when we keep the image size as the edit mode box size and cases when we don't. Also full width images will show a huge edit mode UI in big screens. |
Good call. Let's implement the new icon and add the thumbnail + cancel link inside of the placeholder, and see how that feels. That alone will be a really solid improvement. |
1b2a82b
to
d486603
Compare
Hi @kjellr, I have updated this PR and it now behaves like this: So the PR adds:
@noisysocks could you take a peek at the changes starting here because they look kinda hackish to me and I am unsure of all that nesting :) |
@draganescu excellent! In the Replace Image screen, can we drop the image margin from 5% to 3%? I think it feels better at that size. I'd hate to add more, but can we add this to the Media+Text block as well? |
…exit the edit mode, also there is only one edit mode flow
Co-Authored-By: draganescu <me@andreidraganescu.info>
4e6b6a5
to
0bbf891
Compare
Looks like all the changes are addressed, merging this. |
This is excellent, thanks @draganescu! Such a helpful improvement. I've opened #14795 to help track efforts to roll this out to other blocks as well. 👍 |
Description
Trying to move #11952 forward by:
How has this been tested?
Localhost proof of concept
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist: