Add option to use Post Featured Image with consistent height #27545
Conversation
ntsekouras
added
[Feature] Full Site Editing
[Block] Post Featured Image
ntsekouras
requested review from
scruffian,
aristath,
gziolo,
jasmussen and
ItsJonQ
| min-height: 300px; | ||
| background-position: center center; | ||
| background-size: cover; | ||
| a { |
There was a problem hiding this comment.
@jasmussen is there a better way to achieve full height/width link with no content?
There was a problem hiding this comment.
would it make more sense to just wrap the image in an a tag?
|
Size Change: +654 B (0%) Total Size: 1.19 MB
ℹ️ View Unchanged
|
|
I'm a little confused... Is the purpose of this PR to allow consistent height for all featured image blocks when used in a grid? |
Yes. I had the css |
ntsekouras
changed the title
|
Yes, I'd echo Ari, the use of "Cover" is a little misleading. I understand this to be that you are using the I do agree, this is a nice feature for the Featured Image block to have. But having just worked on #27331, one thing that became clear to me from that effort was that we have a lot of very very different UI across sidebars, and it's making it very inconcistent and confusing. So my first question is: how urgent is the need for this feature (perhaps @kjellr who's working on block themes can help answer)? If it's very urgent, we can probably make some small tweaks and land this as a small solution. If it's not urgent, I think we might want to wait for some of the efforts to address #27331 to land, so we can make a consistent UI for this. Because what it boils down to is two things:
2 is where it gets really tricky — how deep do we go? Here's a mockup of the controls for the Cover block in the new layout:
The thing is, ideally the UI here should be identical between any block that allows background images, give or take controls that get hidden on a per-block basis if they don't make sense. And for every new variation of this UI we create, we have to go back and fix up the old ones once we finally do get to a point. So while I deeply appreciate your work here, the thing is, I can see this UI being used in many places that allow backgrounds, and it might be nice to get this UI super duper right and consistent for the cover block, before we expand it further. |
I wouldn't describe it as urgent, though I do think this sort of thing should make its way in eventually. |
|
A simpler if perhaps interim alternative is to use object-fit, which the gallery block does at the moment. |
Yeah, I know. I saw that we already use this in a couple of places in GB, but since it doesn't have full browser support (IE) I didn't go for it. Anyway, it seems from the discussions that this PR will not go through and we'll wait for the new designs/implementations. |
|
While we still support IE11, everything beyond that supports object fit, and I would suggest that given the feature image will still work in IE11, just not cover the area, that it's a decent solution that should be somewhat simpler to implement, with this toggle in the sidebar:
But okay to wait if you prefer the background approach. |
It's not about the specific approach - using I'll wait a day or two for any other comments and will close the PR.. |
|
I've added a label to get some additional thoughts. |
|
It could be interesting to work on the link aspect of this separately, so that when we are ready for this the link is already in place. |
You mean making the Post Featured Image a link? Because it's implemented already on master. |
|
Ah OK, I wasn't aware of that. I guess the part that seemed challenging in the PR was ensuring that the image was wrapped in the a tag, but I don't know how that's working on master... |
|
I opened an issue here to continue tracking this feature: #27620. |
Description
This PR adds support for setting consistent height for the Post Featured Image block, using css
background-imageproperty. I think this is a very important thing for Post Featured Image to support, so as to enable basic/common designs mostly used throughQueryblock.I intentionally handled this specifically in this block by providing minimum functionality which provides value ( IMO :) ), as there are discussions for creating a generic
backgroundcontrol to handle featured images, along with other options (#24660).How has this been tested?
Manually
Screenshots
Types of changes
Checklist: