-
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
[Block Library - Post Featured Image]: Add basic dimension controls #31634
Conversation
Size Change: +533 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
What an incredibly strong first offering. To me. this PR feels like it validates the idea of the object-fit size controls appearing after you've set both dimensions. I'm seeing some weirdness with the sizing, though: Percent here doesn't seem to be working correctly, is it? Maybe it is. But vw seems to be faring better: For some reason the scale values don't appear to work at small dimensions: A few small things:
Excellent work! Excited to see this land. |
+1. Technical users will understand these terms, otherwise they have little meaning. Sharing for potential inspiration there – YouTube playback quality options are listed as "1080p, 720p, 480p", etc on desktop. But I recently noticed that the mobile apps have a different take on that: Maybe we can do something similar? |
To clarify I actually like the segmented control for this one. I was referring more to making it match the mockup. That said, a dropdown would afford additional help text, that does make sense too. But the thing I like the most about the segmented control is that its effect is so instant — you just click through the three options and the results are immediately visually evident, teaching you what each option does. Combined with the fact that there is a default, I think this would be a good place to start — that would also let us make headway on the control itself, which is needed elsewhere also. One thing we could do is to just add some help text below, to clarify, and to change the "Fill" to "Stretch": |
Totally! I wasn't suggesting to change that. My point was mostly around the language. A label like "Contain" is going to be meaningless to the vast majority of people. You're correct that the canvas gives visual feedback – but only in the current context – it doesn't actually teach you what "Contain" means holistically. A few more words of description (or even just better labels, I like the "Stretch" example) might help make these more technical controls more digestible, and better assist visually impaired users? Sorry for de-railing this PR, we can discuss this in a separate issue :D |
Not derailing at all, in fact very much on point. Stretch is one such step. But I didn't find a good single-word term to replace "contain". Something that encompasses "don't crop and don't stretch". |
Yes I don't think a single word will really cut it. We have this area control in the template part creation flow: Maybe we could use something like that, with similar descriptions to those found at https://www.w3schools.com/css/css3_object-fit.asp ? Or, could we display a thumbnail preview, kind of like the style picker? That would totally eliminate the mystery and the need to click through each option to visualise its effects. A tooltip could reveal the detailed description 🤔 |
The thumbnail preview might work. But we'd need a great image where the differential between the three styles is clear. |
I addressed some of the feedback:
I tried finding a nice way to do this but found some problems with the label behavior.
I think this is resolved now. I added
Feel free to push any enhancements you like 😄 |
I'm tinkering with some things and encountered this situation: Observe how if you add only a height, the image immediately gets super stretched. If, however, you only add a width, the image looks fine and non-stretched. This is because we have the following 100% width rule: In other words, if you add only a height but no width, you actually do have a width — 100%. Which would also benefit from the "cover" scaling property. Can we try a change so that the cover/contain/stretch control also becomes available if you add just a height? I.e. show the control if there's a height, or a height and a width. |
Another question, I'm looking at the segmented control: It looks like a one-off for the featured control here for now, and that might be a fine way to dip our toes in? Alternately, should we already now start to extract this one as its own control that can be used in other places? We'll need it for the nav block also #30370 Otherwise let me know and I'll just style according to the |
That I believe is more of a design folks decision. IMO this functionality will be much needed and 5.8 is really close 😄 Either way I'm happy to help creating the new control if needed now. --edit:
If we are okay to style this one, your help would be great for sure 💯 |
4abc6d1
to
2c4edcc
Compare
Let's try to get the version that is already implemented for the g2-repo here if possible. |
This is nice! |
packages/block-library/src/post-featured-image/dimension-controls.js
Outdated
Show resolved
Hide resolved
702f2d3
to
c40bd8f
Compare
Good catch! Thanks! Fixed.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the reset. This works well in my testing, and seems like a good v1 in case we want to iterate further later on. 👍
This is great, I love it, but this now differs when compared to images blocks yet they are in most respects doing the same thing (full disclosure I think the image block is poor) back to my main point though, this lack of consistency confuses users. |
I am wondering where we can extend these new controls to other blocks where it is natural to add them? |
Consistency is absolutely critical. Decisions needs to be considered prior to implementation and to meet a predefined specification otherwise you end up with what Gutenberg currently is. An inconsistent perpetual beta. |
I agree 100% here and improvements are always being implemented. This is a big and difficult task and we have to tackle this with discussion, new issues and iterations. While some blocks have similarities, the consolidation of them is not so simple. There are already some issues about consistency for UX and API like this one and we will get there step by step 😄 . |
Can anybody tell me in which version of Gutenberg this feature has landed? It is not available in wordpress 5.8, even though it was mentioned in the PR description, is that correct? |
Nice, thanks! Is there also an option to select a WordPress predefined image format (like Middle, Large, Thumbnail etc.)? If not, is this issue being tracked somewhere? Sorry for being a bit lost here. |
It's tracked here: #33789 |
Is there a way to get this feature with the current WordPress 5.8 and maybe a hidden hook? I would like to set all feature images of the query loop to a specific size. I am not sure if using the additional gutenberg plugin is a good idea for a stable running site. |
I don't think so. It's in the plugin |
There's no way to select which registered image size is to be used for the base of this block; e.g. in the post query loop, we need to be able to choose that e.g. |
That's the existing issue for your request. Thanks for commenting there. |
Have already done so: #33789 (comment) |
Description
Resolves: #27620
This PR enables basic dimension controls (
width
,height
) and allowobject-fit
properties to be configured.I believe landing at least a basic handling for now like this would be okay for now and gives value, in order to make it to 5.8.
Notes
scale
control from the design does not exists now, so I've added some adhoc styles. We might improve those styles or consider adding a new component to match the design.object-fit
properties we need to have set bothwidth
andheight
(see comment).scale
control I've implementedtoggle/unset
functionality. We can keep this or add areset/clear
button.height
in percentage (%
) feels a bit unintuitive IMO - any suggestions?Testing instructions
Post Featured Image
block (single or in aQuery
block) and setwidth
,height
andscale
. What feels natural to me is havingwidth
in%
andheight
inpx
, but this is just subjective.