Open iamtakashi opened 4 years ago
Will this cascade out to all the Varia child themes if we correct it here?
Unfortunately, the child themes need to recompile their CSS and to be deployed :/
Ah — but once fixed, child themes re-compiled and deployed, we'll have the fix across 20+ themes. Seems like a good priority fix since it affects the 1:1 fidelity of front-end vs Editor. cc @kjellr and @allancole for thoughts.
Looks like a fairly easy fix to sort out. Shouldn’t be too hard to recompile the fix into all the child-themes either (this can be automated). The hardest part will be getting all 20+ themes tested and deployed to dotcom. That part has to be done manually for each theme 1-by-1.
Something like this should make the editor view better.
[data-block][data-type="jetpack/layout-grid-column"] {
margin-top: 0;
margin-bottom: 0;
}
[data-block][data-type="jetpack/layout-grid-column"] .block-editor-block-list__layout *:first-child {
margin-top: 0;
}
[data-block][data-type="jetpack/layout-grid-column"] .block-editor-block-list__layout *:last-child {
margin-bottom: 0;
}
What do you think, @allancole? Any chance you could move this forward soon?
@kjellr Could we get some attention to this issue if it's possible?
As we're using more Layout Grid block for layouts and block patterns, more customers are having a difficult time to edit the page because of the disparity between the editor and the front of the site. Obviously, this affecting both of the previews for the layouts and the patterns.
For example:
When it should look like something like:
There will probably better way to fix this than my quick suggestion above :) Thanks in advance!
@iamtakashi We have some higher-priority items at the moment, but I'll try to get someone on this next week.
@kjellr Thank you! 🙇
Just a heads up that I looked into this briefly, and I'm actually seeing the same extra margins on the columns block (and all other nested blocks, frankly):
Tested with Gutenberg v8.0.0.
I think this is related to the fact that nested Gutenberg blocks aren't collapsing their margins — they end up building up like that, the deeper you go. I don't know that the solution is actually removing the margins entirely, since nested blocks should have margins to prevent them from bumping up against adjacent inner blocks. So this will take a little adjusting.
Thanks for looking into it @kjellr!
As you said, the core column block now also has the extra margins, and I understand why they are needed.
I agree that removing all of them wouldn't be a good solution, but what could we do to make this situation better? A quick comparison shows the one in the editor has a double the height that we see on the front of the site.
Could we reduce the margins for some blocks? I imagine we don't need the same vertical margins for the blocks that don't have the margin on the front of the site.
I wonder if this is something that should be handled in Gutenberg, since it technically effects all themes. I've also been burned in the past by targeting too many Gutenberg default styles in a theme... it often leads to breakage eventually.
cc @jasmussen for an extra set of eyes here — you tend to be much more in tune with all the margins in Gutenberg than I am.
Yes, I can confirm that the layout grid has 1 extra unnecessary instance of margin, which makes it not look the same as the frontend.
Because of that fact, I would consider it a bug in the Layout Grid block, and one that we should fix. Takashi is indeed right, in my preliminary testing, this fixes it:
[data-type="jetpack/layout-grid"] {
margin-top: 0;
margin-bottom: 0;
}
I will add that to the layout grid block, because I also have to think about how to handle this:
Permit me to We have the following rules in the editor:
.block-editor-block-list__block {
margin-top: 28px;
margin-bottom: 28px;
}
That is a "baseline margin" that is applied to every single block ever, regardless of nesting level. It is meant to be overridden and customize, but provide a "fallback minimum margin" in cases where a block does not provide any margin.
That also explains why the group block adds additional margins — it's also a block, even if it is often just an invisible nesting container. When margins don't collapse, as they don't do in cases of flex, that causes the extra issue. I will investigate what a good fix might be there, but it's nontrivial.
There's a farther out solution, it's going to get long-winded, but if you have time, I could use your advice. Ready?
👇 👇
Why not just remove that rule entirely, and allow every individual block, or the theme itself, to specify the margins of each block? Well, it's complicated. Outside of a rough transition (some blocks will break because they assume those margins, or even compensate for them), there are some big and fundamental questions, and I would love your themer thoughts on them.
Given a block can be wrapped in anything, a figure
, a p
, a div
or section
or span
or even nothing at all, are we sure we want to remove this margin entirely? You can imagine most of those blocks lying flush next to each other if we removed it, and others, like the figure
, might have wildly uneven margins.
If we did remove that margin — should each individual block really provide those margins instead? For example our Image Compare block is basically like an Image, so it should have the same margin as an image, right? But how would margins across blocks be consistent?
More problematically, if we did provide, say, this as part of the paragraph block:
p {
margin-top: 1em;
margin-bottom: 1em;
}
then we would effectively be creating new rules that themes need to override anyway, just vestigal stuff for the browser to parse, understand and then discard.
We also can't really provide a globally inherited CSS block margin rule, as this would require new CSS classes for the frontend, for every block, even paragraphs. And even then, it would just make it even harder to override.
Ultimately it seems like the path forward is to remove the "baseline margin" entirely, even if it will become uncomfortable. Because ultimately, it feels the purview of the theme alone to provide a vertical rhythm. And it will get uncomfortable, here's what TwentyTwenty does:
figure {
display: block;
margin: 0;
}
We'll probably need some new rule that applies for the vanilla editor style, the one that shows up when a theme does not register an editor style. That's its own challenge.
Here's a more visual example:
As background, this is in a theme that customizes the paragraph margin to be 1em top and bottom, which is less than the 28px top and bottom margin that the editor provides as a baseline.
When the GIF starts, it shows the in-progress fix for the layout grid, which removes top and bottom margins from the block. Things flow nicely.
Then that layout grid is turned into a group, and suddenly the margins that are applied to the group kick in — those are bigger than the default paragraph margin, so while they collapse, it does make for bigger spacing.
Question time. Do we:
It's worth clarifying, that baseline margin is editor only, it does not appear on the frontend.
Layout grid patch here: https://github.com/Automattic/block-experiments/pull/90
@jasmussen, thanks for your insight and moving this conversation forward. Also, thank you for pushing the fix to the Layout Grid block!
Removing the baseline margin is a very interesting idea. There seem to be a few steps to get there, but that's definitely something we want to keep discussing! But in the meantime, I'd +1 on removing the outermost margins from the Group block editor style.
Thanks for the feedback!
I've created https://github.com/WordPress/gutenberg/pull/22208 that removes the baseline margin entirely — I do not expect it to be merged, but hopefully it can generate some good discussion!
I also created https://github.com/WordPress/gutenberg/pull/22209, which should have a much better chance of getting merged, as it's a mostly harmless change to just the group block.
Thank you for digging into this so thoroughly, @jasmussen! I'll weigh in on those Gutenberg tickets.
Took another look at this today. Still appears to be a problem despite the items noted above being merged already. Doesn't appear to be theme-specific though and I would recommend against trying to overcome it in any of the themes. Seems to be specific to the block and should be reported there instead.
We're releasing a new version of Layout Grid soon, so I wanted to see if I could bundle a fix. But from what I can tell, it appears to be both an editor issue, and to a smaller extent, a theme issue (more like a consequence of the editor):
Notice how the group has padding + background color, and this correct between editor and frontend. But on the frontend, the p
has zero top and bottom margin, for some reason, whereas in the editor it does have top and bottom margin. So it's two issues:
Firstly, for classic themes, the editor outputs top and bottom margins for every block (as defined by the [data-block]
attribute). Recent classic themes take control of that in this way:
It's a bit clunky, and for that reason, while TwentyTwentyOne theoretically could output an editor style to match the frontend and fix this discrepancy, I understand why that's not really trivial to do.
The good news is: block themes don't have this issue, because default top and bottom margins aren't output by the editor. If the theme stylesheet is loaded in the editor, it's just the same with no difference. Here's editor and frontend in my block theme:
Here's editor and frontend in TT1 blocks:
The bad news is, we probably can't fix this in the Layout Grid plugin. The top and bottom margins for blocks inserted should be controlled by the theme, even if the theme is at the mercy of the editor styles. In that vein, I think the fix, ultimately, will be to move towards block themes.
Steps to replicate
Notice the difference in the vertical spacing. The one with the core column block is expected. This is how it looks on the front of the site.