Add prompt to zoom out separator #65392
Conversation
getdave
added
[Type] Enhancement
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
@WordPress/gutenberg-design this will need your expertise 🙇 |
|
Size Change: +1.29 kB (+0.07%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
A couple of quick GIFs:
Nice exploration. There are some slight details to refine if we are to land this, the drag and drop behavior feels a bit flickering, it seems as if it's interfering with the sibling inserters and/or the text. It's subtle and mostly a feeling, but I feel like if you are dragging, during the act of dragging, everything else should be basically inert. You're only interacting with what's under the grabby-hand, so the animation to make space between sections should respond only to that. Make sense? The second thing is the font size, weight, and family itself, ideally this prompt is "UI", i.e. system font, 13px font size. I recognize this will be very non-trivial to get right, since you're essentially dealing with an already-scaled interface at that point, so you'd need to counteract that. It may even be unfeasible, but it feels worth seeing what we can do here to ensure that whatever text we do show, shows up in a legible font size that doesn't look like the theme itself, but as part of the editor. There may be nuance here. In general, and especially during drag operations, we can be much better at changing the visual appearance of the editor, highlighting dropzones, showing "Drop here" prompts, etc. So the instinct is good enough; this is overall a space where the editor is lacking. While dragging is essentially a mode, or a context, and the editor itself can light up like fireworks when we're in this mode. |
|
Thank you for reviewing 👍 You've picked up on a number of good points which I'd love to improve. Some of these are bugs and some are wider enhancements to drag and drop. I'd love to get those improved - but likely not in this particular PR. I expect you know that anyway. I will collate the feedback and to the wider zoom out issue.
I think this is the most immediately actionable feedback that is specific to this PR. Let's see if we can make the font "feel" right proportional to the zoom level. |
|
Nice work 👏🏻 My initial thoughts:
|
@jasmussen I've now applied the correct font family and also set the font size to be proportional to the zoom scale factor variable. The base size is This is the result: 
Please note: there appears to be a general UI bug with "zooming" whereby the first time you activate zoom out things aren't scaled correctly. Then when you open the pattern inserter and toggle zoom out it works again. This is unrelated to this PR but requires fixing. Perhaps by #63390? |
|
Flaky tests detected in fcea97e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11016627453
|
draganescu
force-pushed
the
try/add-prompt-to-zoom-out-separators
branch
from
fcea97e to
88983d3
Compare
| font-size: $default-font-size; | ||
| font-family: $default-font; | ||
|
|
||
| .is-zoomed-out & { |
There was a problem hiding this comment.
@draganescu Maybe this class got removed?
|
That's quite a scaling value :D This is working nicely:
There's a curious width issue, not sure if it's related to this branch or not:
Also while you're here, can you add an exlicit
Here's how it looks with the default font weight, compare to the chip:
Separately, I wonder if it isn't best to have just a single message, always, one that doesn't change when you're hovering over the drop-zone. Perhaps simply: "Drop here."? |
| duration: 0.1, | ||
| } } | ||
| > | ||
| { __( 'Drop pattern.' ) } |
There was a problem hiding this comment.
@jasmussen @draganescu Shall we run with this and then iterate? We have until RC 1 (October 22, 2024) to change this prior to the hard string freeze.
There was a problem hiding this comment.
Fine from me, assuming we can polish up the little things.
There was a problem hiding this comment.
...can polish up the little things.
You mean overall on Zoom Out mode? Yes we're need to polish during the Beta/RC phase for 6.7.
I think all the items you requested that are applicable directly to this PR have been addressed no? If not happy to tweak them.
There was a problem hiding this comment.
Overall zoom mode, though also figuring out the width issue. But yes, it's not clear to me if that's a side effect of this PR or not. But yeah, mostly mean the experience overall.
There was a problem hiding this comment.
I'm going to ask folks working on Zoom Out to start testing everything on TT5 👍
I'll raise an Issue to track the width thing.
There was a problem hiding this comment.
What is wrong with two labels?
| justify-content: center; | ||
| font-size: $default-font-size; | ||
| font-family: $default-font; | ||
| font-weight: normal; |
There was a problem hiding this comment.
@jasmussen Updated to force normal weight 👍
Which Theme is that? Almost certainly not related to changes in this PR but would be good to be able to replicate that Issue for tracking separately.
✅ Done in 1d35d21
I updated this as you suggested. I think we should now ship this "as is" and then iterate in response to (the inevitable) feedback. What do you think? |
This is WIP Twenty Twenty-Five. Happy to try this. |
|
@draganescu Are you happy to approve this one? |
…acground after rebase on trunk
| @@ -416,6 +416,7 @@ _::-webkit-full-page-media, _:future, :root .has-multi-selection .block-editor-b | |||
| justify-content: center; | |||
| font-size: $default-font-size; | |||
| font-family: $default-font; | |||
| color: $black; | |||
There was a problem hiding this comment.
This needs to be $gray-900.
There was a problem hiding this comment.
Or maybe you used $black because it needed that level of contrast?
If so let's add a comment to explain why we're using it.
There was a problem hiding this comment.
It feels on that gray we need black.
There was a problem hiding this comment.
Yeh the contrast would be insufficient otherwise. Thanks for clarifying 👍
|
@draganescu this for 6.7? |
|
@getdave yes |
draganescu
added
the
Backport to WP 6.7 Beta/RC
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
* Add prompt to separator * updated the text to be animated too and to have contextual messageing - drop or insert * Fix font size * Scale font size based on zoom scale * Force font weight reset * Remove dynamic text * adds a color to the test which somehow gets the same color as the babacground after rebase on trunk * bogus change for our pipeline * revert bogus change for our pipeline to restart --------- Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org>
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: ef84f51 |
What?
Adds a user prompt to "drag and drop patterns" to Zoom Out separators.
Why?
To encourage users to inserter patterns / drag and drop into the canvas.
How?
Adds prompt.
I fully expect this will need design input
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast