Restore the margins of blocks relying on the figure element #29517
Conversation
youknowriad
added
[Type] Enhancement
youknowriad
requested review from
scruffian,
aristath,
kjellr,
jasmussen and
carolinan
| @@ -1,7 +1,5 @@ | |||
| .wp-block-image { | |||
| // The image block is in a `figure` element, and many themes zero this out. | |||
| // This rule explicitly sets it, to ensure at least some bottom-margin in the flow. | |||
| margin-bottom: 1em; | |||
There was a problem hiding this comment.
The image used to have this opinionated margin and I'm not sure why it's there specially since it's not added to the other blocks, I believe being consistent between blocks is better. cc @jasmussen
There was a problem hiding this comment.
I vaguely recall this being related to image captions. But any spacing issues there could presumably be handled by the figcaption, for which we have these rules:
.wp-block-image figcaption {
margin-top: 0.5em;
margin-bottom: 1em;
}
|
Size Change: +6.99 kB (+1%) Total Size: 1.4 MB
ℹ️ View Unchanged
|
|
In principle, I'm a fan of this change. And it's one of the changes that we need to land in order for something like #22208 to be successful, i.e. each block specify their own margins, or no margins at all, and the theme is able to style both using the same stylesheet. My first instinct in seeing this one, though, was that it might cause headaches for themes that create the centered column using For that reason, yes it does make sense to level the playing field for I'll let other folks chime in, but I'm leaning in favor of this one 👍 👍 |
There was a problem hiding this comment.
I like this change too. Like Joen said, it sets the stage for adding margin controls to blocks. A couple questions:
Could these lines be removed too?
gutenberg/packages/block-library/src/embed/style.scss
Lines 29 to 31 in da8555d
I noticed that the blocks in the editor assume that the "figure" doesn't have margins but by default in the DOM
In the editor, won't the block-editor-block-list__block margins still apply to these elements?
Yes, but this is a separate issue, these margins are about having margins between all kind of blocks which is also something we want to absorb declaratively (the margin controls).
Yes, I think it makes sense to remove that if we remove the equivalent style in the image block. |
|
Truthfully, I am struggling to understand the PR description. For a theme with no styling but the Before: With PR: And here is the front on top of the editor. If the purpose is for them to look similar, it is not working. What about this margin for the pullquote? |
|
@carolinan The first goal of this PR is more about margin-left/right than bottom ones. So, the figure element by default has margins and if you have a theme with no stylesheet and you insert any of the block above, they're not going to be aligned properly with the rest of the blocks because of these margins. But in the editor, there's a global Now for the "bottom" margins, it's something I noticed while working on the PR, some blocks have styles adding bottom margins, essentially because the idea is that blocks should have some space between them. In the editor, there's a global margin that is added between all blocks consistently, while in the frontend, that margin is actually added to some blocks but not others. So what I tried to do in this PR is to actually remove these adhoc margin bottom styles added to some blocks. Ideally though, these margins should also be removed from the editor (to match the frontend) or the frontend should have margins between all blocks. The problem is that we want that to be controlled by a "margin" style per block potentially, so in order to keep this PR contained, I decided that I'd just make things consistent:
This means the margin bottoms don't match between the editor and the frontend but that's something I'm leaving for a separate PR/work. |
|
And yes, that pullquote margin should be removed too IMO |
|
I'm starting to think that we should avoid impactful changes until we introduce the "margin control" for the bottom margins, so, I'm going to revert the margin bottom styles, and only fix the left/right for now |
|
Any ✅ here :) ? |
|
🎉 |
|
The list of tested themes above does not include Twenty Twenty. Setting side margins to zero makes at least the image block off-centered in that theme. Update: This Twenty Twenty issue seems to have been fixed between Gutenberg plugin versions 10.5.2 and 10.5.4 |
Our goal for the block editor should be that the frontend matches the backend by default. Meaning that if a theme has no stylesheet, all blocks should look similar in the frontend and the backend. We tend to break this rule in some small details here and there.
I noticed that the blocks in the editor assume that the "figure" doesn't have margins but by default in the DOM, all figure elements have margins, this PR restores the margins for the blocks relying on that element so that themes don't have to make these adjustments themselves.
Testing instructions
Check the image, embed, video, audio, pullquote and table block margins in several themes and ensure there's no visual regression.