Skip to content
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

Global styles: Add a global spacing variable #27315

Closed
scruffian opened this issue Nov 26, 2020 · 23 comments
Closed

Global styles: Add a global spacing variable #27315

scruffian opened this issue Nov 26, 2020 · 23 comments
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json

Comments

@scruffian
Copy link
Contributor

scruffian commented Nov 26, 2020

Is your feature request related to a problem? Please describe.
In TwentyTwenty one there is a CSS variable called --global-spacing-unit. I think it would be useful to add this to the default values in Global Styles, so that blocks can set their default spacing (padding/margins) to use this default value. This will allow users to update this spacing across their site in one place.

When we set a default padding (for example when a background color is set), this unit should be used.

@scruffian scruffian added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Nov 26, 2020
@scruffian
Copy link
Contributor Author

scruffian commented Nov 27, 2020

This is connected to #27154

@kjellr
Copy link
Contributor

kjellr commented Nov 30, 2020

+1 — whether it was a sass or a css variable, something like this has existed in every theme I've worked on since Gutenberg was released. It's essential for ensuring that the margins and spacing in the front end and the editor are correct. Placing it in a single, easy-to-change variable would help avoid problems like this one: WordPress/twentytwentyone#859

@jasmussen
Copy link
Contributor

I was working on an FSE theme this weekend, and had to tackle margins to get the editor to behave more like the front-end. This bit was a headache:

.block-editor-block-list__block {
	margin-top: 28px;
	margin-bottom: 28px;
}

There's a long history to why those margins exist, some recounted in this ticket. But as themes increasingly style the editor, they are starting to cause more problems than they solved.

Through an implementation in global styles, we have an opportunity to refactor that. Many themes attach the margin to wp-block directly, like so:

.wp-block {
	margin-top: 1em;
	margin-bottom: 1em;
}

But that means anything that is registered as a "block", gets those margins:

  • passthrough blocks that should be invisible containers, like column
  • all horizontal blocks such as Social Links, Buttons, or Navigation

So as a blanket rule, it doesn't work too well. By supporting a vertical spacing variable in theme.json, we have an opportunity to be smarter about which blocks employ the variable and which blocks do not.


But nearer term, even just a theme.json variable to opt out of those margins entirely, without a UI, could be a quick win. As an experiment, I wrote this CSS to unset the margin variables in the editor:

// Unset the intrinsic block margin.
.block-editor-block-list__block {
	margin-top: unset;
	margin-bottom: unset;
}

I had to load the CSS into the site editor through a roundabout way to ensure unprefixed low specificity, but it works.

The thing to keep in mind is that these margins are output only in the editor, and not on the frontend. Especially for FSE themes, there's no benefit unless it's the same in both. By resetting them, I can control the margins and achieve perfect parity between editor and frontend. This theme uses the same stylesheet for both:

editor and frontend

Providing a theme.json opt-out would be a great way to move forward in a progressive way.

@mtias
Copy link
Member

mtias commented Mar 1, 2021

@nosolosw this seems like a good candidate to start as theme.json only while we work on the UI for Spacing:

image

@oandregal
Copy link
Member

Coincidentally, @youknowriad and I talked about this the past days, and adding support for margin in theme.json is part of the dimension panels todo list #28356

As I understand this and related threads, though, block supports for margin is only part of the issue. Another part is that we want to remove the existing margin CSS but we can't. So, would it be a good first step to enqueue that margin CSS only for themes that don't have theme.json? @mtias @scruffian

@scruffian
Copy link
Contributor Author

So, would it be a good first step to enqueue that margin CSS only for themes that don't have theme.json?

Yes I think that would be good. @kjellr @jffng @pbking @MaggieCabrera

@youknowriad
Copy link
Contributor

youknowriad commented Mar 1, 2021

So, would it be a good first step to enqueue that margin CSS only for themes that don't have theme.json?

I'm doing something similar for the alignments FYI #29335 https://github.com/WordPress/gutenberg/pull/29335/files#diff-e0ee63e1380dc04eba95c99d0d5587304ec37fbaa50fb030beb0ba75576f27fbR120

@kjellr
Copy link
Contributor

kjellr commented Mar 1, 2021

So, would it be a good first step to enqueue that margin CSS only for themes that don't have theme.json?

Yeah, that seems like it's worth a try. 👍

@mtias
Copy link
Member

mtias commented Mar 1, 2021

So, would it be a good first step to enqueue that margin CSS only for themes that don't have theme.json?

Maybe there's a way to set the current margin as the default for the variable and use that?

@jasmussen
Copy link
Contributor

Maybe there's a way to set the current margin as the default for the variable and use that?

One of the use cases for me is to have different margins for some blocks, and no margins for other blocks. For example, column (singular), should arguably never have any margin. And child blocks in horizontal blocks, such as Buttons or Social Links, do not benefit from top/bottom margins.

It would be nice with a single default spacing unit that I could configure in one place, instead of having to change the defaults of 50 blocks, but then having the ability to customize that further, say I wanted a larger margin for Image blocks. But it would still be key that not every block gets this margin by default, it should probably be per-block opt-in.

@scruffian
Copy link
Contributor Author

it should probably be per-block opt-in.

The way I see it, each block would decide how to respond to this setting. Some might use it for left/right margins, others for padding, some might do calculations on it etc.

@carolinan
Copy link
Contributor

Something to consider:
After working with these margins in Twenty Twenty-One, there has been feedback that overriding

.block-editor-block-list__block {
	margin-top: 28px;
	margin-bottom: 28px;
}

and adding different spacing to different WordPress core blocks and inner blocks only causes a lot of problems for plugins, because their custom blocks suddenly has no spacing.

A standard solution (and a standard variable name) for this in the theme JSON file would help.

@youknowriad
Copy link
Contributor

I've started working on the margin issue and I've been thinking about this issue too.

The global variable proposal is interesting, it's a convenient one for sure, but it's also an opinionated one.
It seems to me what we want is:

1- Being able to apply a consistent "margin" to all blocks. With the "margin" block support, we can do that by targeting specific blocks in theme.json but we lack a way of targetting all blocks. We could introduce a dedicated context for that, alternatively, if we get this in #29714 we could potentially use the CSS variable produced by the "root margin style".

2- Potentially reuse the same "spacing value" used for margins in other places like "gap" configs, "paddings"... For me this should be left to the theme and instead of forcing an opinionated choice, theme authors should use the "custom" config to come up with their own variable and use it in these places. In terms of UI, we need a declarative approach to say "show a numeric control for this variable in the global styles UI".

@jasmussen
Copy link
Contributor

1- Being able to apply a consistent "margin" to all blocks. With the "margin" block support, we can do that by targeting specific blocks in theme.json but we lack a way of targetting all blocks. We could introduce a dedicated context for that, alternatively, if we get this in #29714 we could potentially use the CSS variable produced by the "root margin style".

To me the biggest problem with the block margins as they exist (see Carolina's comment) is that they apply to every single block in existance, including passthrough blocks like "Column", and including horizontal blocks like Button, which arguably should have an entirely different set of margins.

In other words, the current margin variable suffers from having been created in context of an editor that did not support nesting, or horizontal blocks.

A big step forward would be a way to simply unset those margins, and let themes handle them entirely in CSS.

@youknowriad
Copy link
Contributor

A big step forward would be a way to simply unset those margins, and let themes handle them entirely in CSS.

or in theme.json which is what I'm proposing here and we don't really need to unset them since it will take over and will be backward compatible.

@jasmussen
Copy link
Contributor

we don't really need to unset them since it will take over and will be backward compatible.

It's a bit tricky and similar to #30337 (comment). Is there now way we can output those margin rules only for themes, and opt-out or simply not load them for block-themes?

@youknowriad
Copy link
Contributor

Is there now way we can output those margin rules only for themes, and opt-out or simply not load them for block-themes?

It is possible yes, we can do like "layout" styles and only load them for themes without theme.json.

--
Thinking more here, regardless of whether #29714 lands or not, if we do want to allow themes to target all blocks and define margin for them, we need one of these two:

  • an all blocks context somehow in theme.json that excludes the "root"
  • an opinionated variable (which I don't like :p )
  • another property in the "root" block (and containers) that is different than "margin", more something like "blockMargins" and only applies to children and not the context itself.

The reason the CSS variable can't work is because it applies to both the context and its children.

@jasmussen
Copy link
Contributor

Right, the key bit is to not repeat the mistakes of the past and assume that every block should have a margin. It can get extra complicated, for example, if you want to zero out the top margin of the first paragraph in a group, and the bottom margin of the last paragraph in a group. So it's key that whatever margin values we output from theme.json values have as low specificity as possible. I.e. p { margin-top: 1em; margin-bottom: 1em; } is preferable to inline style.

@youknowriad
Copy link
Contributor

I.e. p { margin-top: 1em; margin-bottom: 1em; } is preferable to inline style.

Global styles don't generate inline styles, they generate exactly that.

@scruffian
Copy link
Contributor Author

In terms of UI, we need a declarative approach to say "show a numeric control for this variable in the global styles UI".

This would be great!

@mtias
Copy link
Member

mtias commented Mar 30, 2021

In my mind we would have a more abstracted setting such as "Space between blocks" that would be configured globally and apply to vertical and horizontal inner blocks. This setting would be opinionated in that it might reset for the first and last element of a nested group by default. Individual blocks could overwrite this with margin.

The place in global styles for this would be {context}.spacing.betweenBlocks.

@youknowriad
Copy link
Contributor

@mtias that makes sense and aligns with the third option here #27315 (comment)

@jordesign
Copy link
Contributor

To my understanding this is resolved by #30375

I'll close it - but we can re-open it if it is still something we need to work on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json
Projects
None yet
Development

No branches or pull requests

8 participants