-
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
Post Featured Image is double border marked up. #52349
Comments
Hi @shimotmk - thanks for reporting that. Could you please let us know the steps you took to create that Featured Image block in the first place - so the issue can be recreated from scratch rather than pasting the code in. It will hopefully give some insight into why/if the code was generated that way. Could you please also clarify what you mean when you say 'double border marked up.' ? |
@jordesign Missed steps from the beginning. I'm sorry. The steps to reproduce are as follows. |
Thanks @shimotmk - I can confirm I've been able to recreate this. It looks as though the border is applied to both the I've also confirmed this is present without the Gutenberg plugin active - so it may have been around for a little while. |
@jordesign |
I feel like the padding control is unnecessary. Post Featured Image doesn't have content like Cover block, so I don't think it's necessary to set padding. Can I send a pull request with padding supports removed? |
@shimotmk That's true - it does seem like there's not much use for the padding control. Feel free to submit a PR - in the meantime I've flagged it for design feedback to consider the UX of the block. |
I can imagine situations where padding can be useful, on any image, so I'm not convinced we should remove that control. In terms of the border, I'd expect it to produce the same result as adding padding and border to and img in html. That is to say: the border would at the outer edge of the overall footprint rather than around the image explicitly. |
It's interesting how there are two borders showing up, which is inconsistent with how I'd think about the CSS box model. Following this model, I would expect the border to only appear outside the padding. Not on the content (1) and the padding (2), which is shown in a previously shared screenshot: In other words, I agree with @jameskoster that the border would only appear on the outer edge (outside the padding). One of the other things to consider is that when a user adds an overlay on their Post Featured Image, do they expect the overlay to only apply to the image? Or do they expect the color overlay to also apply to the padding? |
This pull request ( #42847 ) explains why the border was added to the It is considered natural that borders are added to |
Interesting challenge! Can the solution be something as simple as this?
|
Thanks for your comment! post-featured-image.mp4 |
Oh I see the issue. In any case, it seems like this needs to be solved in the same way as it is solved for the Cover image, since effectively the overlay changes it to have more or less the same structure. That is, it doesnt' seem like there are any fixes that will scale properly until we apply border properties on the container itself, as it is done for Cover. |
That's right. It would be better to use a cover block configuration. |
The padding can, when combined with a border, make a "picture frame"-like appearance: But I see the challenge given the Cover comparison I made, because on Cover, the padding does not have the same effect: Definitely a tricky issue, especially because the "picture frame" argument seems to be why the padding option was added in the first place, and since it's out there shipping, it's likely a bunch of themes are leveraging this exact thing. To expand the ping range, @WordPress/gutenberg-design, any good thoughts on what to do here? |
Certainly, a design like a "picture frame" might be possible. And I think it's definitely a tricky issue. The currently rendered HTML looks like this: <figure style="padding-top:var(--wp--preset--spacing--50);padding-bottom:var(--wp--preset--spacing--50);padding-left:var(--wp--preset--spacing--50);padding-right:var(--wp--preset--spacing--50);" class="wp-block-post-featured-image">
<img width="320" height="180" src="https://placehold.jp/150x150.png" class="has-border-color wp-post-image" alt="" decoding="async" style="border-color:#0693e3;border-radius:30px;border-width:30px;object-fit:cover;">
<span class="wp-block-post-featured-image__overlay has-border-color has-background-dim has-background-dim-50 has-black-background-color" style="border-color:#0693e3;border-radius:30px;border-width:30px" aria-hidden="true">
</span>
</figure> |
If you write CSS like this, the border to the image may be cured. .wp-block-post-featured-image[style*=padding] img {
border: none;
} |
Description
Post Featured Image is double border marked up.
Step-by-step reproduction instructions
<!-- wp:post-featured-image {"overlayColor":"black","dimRatio":50,"style":{"spacing":{"padding":{"top":"clamp(1.875rem, 1.303rem + 1.83vw, 2.813rem)","bottom":"clamp(1.875rem, 1.303rem + 1.83vw, 2.813rem)","left":"var:preset|spacing|50","right":"var:preset|spacing|50"}},"border":{"width":"20px"}},"borderColor":"vivid-cyan-blue"} /-->
Setting both Overlay and Padding makes it easy to see that the border is double marked up.
Screenshots, screen recording, code snippet
Environment info
Please confirm that you have searched existing issues in the repo.
Yes
Please confirm that you have tested with all plugins deactivated except Gutenberg.
Yes
The text was updated successfully, but these errors were encountered: