-
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
Group: Zero out the intrinsic margin set to the Group block #22209
Conversation
This PR is a much smaller alternative to #22208. It sets the top and bottom margin of a group block to zero, in the editor only, to unset the margins that are otherwise inherited by the baseline margin that every block in the editor has. This helps a great deal for when people create block patterns, where you might have several groups in a row, that should not have additional margin above and below. Important also to remember that the margins of contents inside the group can still "bleed out" of the group, and even collapse with adjacent margins. It would be nice to not have to unset these margins, and simply not have any margins in the first place. But per the discussions in #22208 removing those is nontrivial.
Size Change: +9 B (0%) Total Size: 824 kB
ℹ️ View Unchanged
|
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.
This makes a lot of sense. It removes the double-margins that were visible in the editor before, and gets things looking much more similar to the way they are in the front end. The group block becomes a lot more invisible, which I think is the way it was intended to be.
My only followup question is whether we should do something similar for the Columns block? That also appears to have "double" margins — the margins from its child blocks, as well the parent block's margins:
@@ -1,7 +1,11 @@ | |||
/** | |||
* Group: All Alignment Settings | |||
*/ | |||
|
|||
.wp-block[data-type="core/group"] { |
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.
why not .wp-block-group
?
.wp-block[data-type="core/group"] { | ||
// Zero out the baseline margin that is set for every block in the editor. | ||
margin-top: 0; | ||
margin-bottom: 0; |
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.
Should this apply to the first and last block inside groups instead of the group itself? I'm having hard time understanding what's specific about the group block here compared to say "cover".
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.
Honestly I think Cover should also zero out margins. #22208 is kind of the nuclear option, this "per-block zeroing out" is the more conservative alternative.
The thing is, both cover and group are div
s so they should be "born without margins" just like their HTML counterparts. This wasn't always feasible for the editor, but as we get closer to FSE and such, we really are seeing some opportunities play out here.
Specifically, you might see a Layout Grid block inside a Group block, and the Layout Grid uses flex so its margins can't collapse, so those margins are zeroed out as well. In those cases, it really helps that the group becomes kind of "invisible". When we reset the margin on the block itself, there's nothing to compensate for, and no complicated first/last child rules that can break in various ways.
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.
Conceptually speaking, I still wonder whether this should apply to first and last inner blocks or the parent one. It's not clear to me what's different between a container block and a non container block in terms of margins when shown at the same level while I can understand that the first and last block of a "Block list" don't have top and bottom margins.
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.
But the first and last blocks of a block list very probably should have margins in some cases, and not have margins in others. That's why I like to think of this same as html an css. If we add a margin and padding control to virtually every block through global styles, going the basic route will make for predictable behavior, whereas going the first child/last child route might add confusion because of the imperceptible heuristic.
To flip it on its head, what would be the benefit if the first/last child approach?
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.
If we add a margin and padding control to virtually every block through global styles, going the basic route will make for predictable behavior, whereas going the first child/last child route might add confusion because of the imperceptible heuristic.
For me, both approaches will have the same result if a padding control is added since both will override the padding for the group (or its first/last block). I don't think there's any big difference tbh between both approaches, my cartesian mind don't like that different blocks on the same block lists have different margins that's all. (feel free to ignore, it's not that important)
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.
I would never ignore someone who's so often right. On weekends, I'm redesigning my website, and finding a need for adjusting margins and paddings on many blocks. I expect this to become very common when FSE efforts mature, and that we will increasingly be looking a a canvas where most blocks are born with no margin at all, which we can then adjust on a per-block basis.
We'll absolutely see how it plays out — but just due to how flexible CSS has been with its approach, I feel the most comfortable mimicking that going forward.
Yes, I think we should, and to Riad's point above — should we do it also for the Cover block? Any others? Of course at some point we veer close to simply wanting to embrace #22208. But I'm happy to followup with any blocks we agree should follow this lead. |
This PR is a much smaller alternative to #22208. It sets the top and bottom margin of a group block to zero, in the editor only, to unset the margins that are otherwise inherited by the baseline margin that every block in the editor has.
This helps a great deal for when people create block patterns, where you might have several groups in a row, that should not have additional margin above and below.
Important also to remember that the margins of contents inside the group can still "bleed out" of the group, and even collapse with adjacent margins.
It would be nice to not have to unset these margins, and simply not have any margins in the first place. But per the discussions in #22208 removing those is nontrivial.
Editor, before:
Editor: after, after:
Frontend before:
Frontend, after:
2 appenders in a row before:
After: