Add a margin block support and enable in group block #30371
Conversation
youknowriad
added
[Feature] Block API
youknowriad
requested review from
ajitbohra,
ellatrix,
oandregal and
spacedmonkey
as code owners
youknowriad
requested review from
kjellr,
scruffian,
mtias,
MaggieCabrera,
aristath,
carolinan and
jasmussen
|
Size Change: +152 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
|
This is a bit tricky to review as it's mostly under the hood as I understand. In general the abstraction of complexity that theme.json has accomplished for block themes is amazing. I was thrilled to be able to rip out big chunks of code for alignments in my theme, it lets me focus on what's important. So in principle it seems okay to allow controlling margins in that same file. But if I wanted to do that in my CSS file instead, that would presumably still be possible? The thing about margins is that they are likely going to change contextually. Maybe you want smaller margins for paragraphs inside a footer, or you're creating a very special subpage that has an entirely different design, prompting something like On the auto value — I know we have space-between properties for some of the horizontal blocks at the moment, and that gets a fair bit of the way. But if we add an option to the group block to display its contents horizontally (make it a flex container in other words), allowing the "auto" value would be very handy for creating some advanced patterns. |
of course, it's possible but why would you want to do that instead of using theme.json? Theme.json is more "controlled" and loaded just where you need it.
Why can't we absorb these use cases in theme.json, it has contexts for that IMO.
how is this any different than the default alignment where "auto" is applied? |
Oh I'm probably just missing the longer view here! Some of my thinking is also shared in #27315 (comment). I'm a big theme.json fan, in principle the more it can take over, the more I can focus on design in my stylesheet. That's good. Just need to stress test the edgecases.
The immediate use case that comes to mind is a navigation menu which has 3 menu items on the left, with consistent 1em spacing between those menu items, but then on the far right have a social links block, with consistent 1em spacing between them. Something like this, in other words: In the above case, because the Navigation block is a flex container, if I set margin-right: auto; on the "About" menu, this would be easily accomplished. Space-between on the other hand, would evenly space the margins between all items. |
That looks more like a hack to me, for me, this just means these are two navigation blocks or a single one with two containers inside. |
|
This is looking good. A few thoughts:
|
|
I should've suggested a "margin-left: auto" on the "social links" inside navigation. I agree flex values can be a bit non-intuitive with how "auto" works, but it's no different than how we center blocks using margin: 0 auto;. |
@nosolosw any help with that. I have no idea how this UI works, I'll check though.
This is the same for all controls and not just margin, we don't have a solution for that yet.
My thinking is that instead of showing these things to the user, which is very hard to understand even for devs, we should instead provide them with better abstractions. Alignments are an abstraction that apply margin: auto and that users can understand. |
I took a quick look at this and I understand the margin panel is something added to the block sidebar by the layout hook, correct? We'd need to port that component to the global styles sidebar as well (over here). |
|
This issue came up in the block-based themes meeting earlier today. It looks like this and #30333 are similar — is there one we should be testing over the other? |
|
Let's close this and review @aaronrobertshaw's PRs instead. |
Related #28356
This PR adds margin support to the Group block and a margin block support. At the moment, it's more a POC to see where we take this. Here are some open questions:
!importantfor these. I think it's a decent solution for this use-case but would love thoughts on this.with this PR, you'll also be able to set "margin" styles in theme.json for blocks.
Testing instructions
spacing.customMargin: trueto your theme.json config.