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

Gap on columns block is hard-coded with a fixed value #41431

Closed
markhowellsmead opened this issue May 30, 2022 · 20 comments
Closed

Gap on columns block is hard-coded with a fixed value #41431

markhowellsmead opened this issue May 30, 2022 · 20 comments
Labels
[Block] Columns Affects the Columns Block CSS Styling Related to editor and front end styles, CSS-specific issues.

Comments

@markhowellsmead
Copy link

Description

The inline CSS uses gap: 2em instead of a CSS custom property.

See also #41015 (comment) and #41423

Step-by-step reproduction instructions

Add columns block to the page and inspect the flex gap property.

Screenshots, screen recording, code snippet

No response

Environment info

  • WordPress 6.0 without plugin

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@markhowellsmead markhowellsmead changed the title Gp on columns block is hard-coded with a fixed value Gap on columns block is hard-coded with a fixed value May 30, 2022
@dennisheiden
Copy link

Side note: Column is weird overall, is set to flexbox but has no flex-direction. We are currently investigating, just a hint for others if their sites look broken from the last update.

@markhowellsmead
Copy link
Author

If no flex-direction is set on an element, then the default value is row.

@dennisheiden
Copy link

That makes no sense for column blocks imo.

@markhowellsmead
Copy link
Author

Column or columns?

@dennisheiden
Copy link

single column

@markhowellsmead
Copy link
Author

Using WordPress 6.0, the display value of the column block isn't overridden, so it's set to display: block. The columns block is set to display: flex, which is correct.

@dennisheiden
Copy link

Ok, thanks for the clarification. I get classes like "wp-container-44" on my blocks. I don't know if those come from the Gutenberg plugin or from another plugin. At least with your info I can check further. Display block, good to know. ty

@markhowellsmead
Copy link
Author

markhowellsmead commented May 30, 2022

They come from WordPress core and/or the Gutenberg plugin, but they don't always apply flexbox rules.

@glendaviesnz
Copy link
Contributor

The addition of a css var to set the default could maybe be considered as part of this work which adds a css var for the gap in a similar way to the gallery block.

@ramonjd
Copy link
Member

ramonjd commented May 31, 2022

The addition of a css var to set the default could maybe be considered as part of #41123 which adds a css var for the gap in a similar way to the gallery block.

Thanks for linking to that PR. I'm not sure it addresses this enhancement request because, while it does create a new CSS var to house the columns block gap spacing value --wp--style--unstable-columns-gap, I'm using it explicitly as an offset value in order to stack columns in a tablet view.

Not to say we can't extend it's usage, but it's not the subject of that PR right now.

Having said that, #41123 is a bit iffy in that it's pending discussion on suitability, so one could extract the code to achieve the effect I think this PR asks for 🤷

@justintadlock justintadlock added the [Block] Columns Affects the Columns Block label Jun 7, 2022
@skorasaurus skorasaurus added the CSS Styling Related to editor and front end styles, CSS-specific issues. label Jul 5, 2022
@andrewserong
Copy link
Contributor

Just adding a bit of background / context surrounding using fixed values for block spacing / gap instead of CSS variables. The shift away from using CSS variables for gap was started in #37360 to resolve subtle issues of conflicts between blocks (and cascade) when using CSS variables for the gap value. More recently, a refactor of the Layout support has been merged (in #40875) which now allows the Columns gap to be set at the block level in theme.json for blocks-based themes. Either in the theme.json file, or in global styles, a gap can be set, and the Layout support will output the correct direct gap value for the block.

From reading the discussion on this issue and #42333, it sounds like the issue here is wanting a way to opt-out of the fallback gap style altogether. Is that correct?

I've linked to this issue in the layout tracking issue (#39336) — one of the tasks is to come up with a way for themes to explicitly opt-out of each part of the Layout support, one of which could be opting out of fallback gap styles altogether. If themes were to do that, though, they'd then be responsible for providing their own styling for any blocks that rely on the Layout support (so not just Columns, but also Social Icons, Buttons, etc). I imagine many classic themes would probably expect to be doing that, though.

If anyone feels strongly about how themes should be able to opt-out of gap styles, please share your ideas! I'm planning to try out a couple of options, but would love to hear if folks already have ideas about how they think it should work. Thanks!

@markhowellsmead
Copy link
Author

Thanks for the explanation @andrewserong. The rabbit hole on this is too deep to follow; there are so many open, partially-merged and as yet unaddressed issues with core CSS, inheritance and specificity that it's all but impossible to debug until the next resolved version lands in public core. It's currently impossible to know what the next release will contain, and which of the many CSS problems it will resolve.

Aside from any technical discussion, the goal for this Github issue is that developers need to adjust the default block gap on columns, media text etc. using values in the theme - either through theme.json or directly in CSS - and also to be able to override them on a per-block case. (e.g. adding an editorially-selectable block style “wide gap” which then amends the gap size on that individual block.

The blockGap CSS property is often unusable for this purpose, as it immediately breaks the cascade. (Where one would st blockGap to 0 on a parent block, the nested child blocks inherit this value, instead of retaining their own gap settings.)

@andrewserong
Copy link
Contributor

The rabbit hole on this is too deep to follow

I hear you! There's a lot of parallel work going on, and it is hard to follow. Given that so much work is in progress, then, it might be worth it to review how much of these issues are addressed, and what still needs tweaking when we reach the beta phase for WordPress 6.1. It looks like the next release should include quite a few improvements that will hopefully addresses these issues, while allowing more flexibility and control.

the goal for this Github issue is that developers need to adjust the default block gap on columns...

This is now possible in theme.json for most blocks that use the Layout support as of #40875, and a direct value is used instead of CSS variables so that it doesn't break the cascade. In terms of an editorially selectable block style for preset gaps, there's work underway in #39371 to explore spacing presets in the UI, in case anyone's interested in following progress there.

In terms of additional Layout features (and exploring options for switching off / opting-out of Layout), I'll continue to post updates over in the layout tracking issue in #39336 🙂

@markhowellsmead
Copy link
Author

Thanks again @andrewserong. I'll certainly look into the Layout controls when 6.1 become publicly available. There have been issues with CSS specificity problems which are blocking theme developers from overriding inline styles - specifically #40159 - which I'm hoping are resolved by the upcoming improvements.

@andrewserong
Copy link
Contributor

There have been issues with CSS specificity problems which are blocking theme developers from overriding inline styles - specifically #40159 - which I'm hoping are resolved by the upcoming improvements.

Thanks for linking through to that issue, I've added that one to the layout tracking issue, too (#39336). I believe the current work should help progress that issue, too, in that base layout styles should wind up having lower specificity, and be easier to override (with less reliance upon inline styles/classname unless there is a "real" inline value). But again, will be good to test once we get closer to the Beta / RC stage 🙂

@markhowellsmead
Copy link
Author

markhowellsmead commented Jul 22, 2022

@andrewserong Following on from @grappler's comments on #40875 (comment): it's come to light that leaving settings.spacing.blockGap as null (the default value) leaves the value 2em hard-coded as a specific inline rule, which cannot be overridden using e.g. .wp-block-columns.

Setting settings.spacing.blockGap as false and using styles.blocks.core/columns.spacing.blockGap introduces the CSS var --wp--style--block-gap, which can be overridden within the theme CSS. But this value cascades down the tree and breaks all the gaps on all nested blocks, where the value should be set only for the columns block, not the nested blocks.

The initial issue of overridability remains for now.

@andrewserong
Copy link
Contributor

Thanks for following up @markhowellsmead:

Setting settings.spacing.blockGap as false and using styles.blocks.core/columns.spacing.blockGap introduces the CSS var --wp--style--block-gap, which can be overridden within the theme CSS. But this value cascades down the tree and breaks all the gaps on all nested blocks, where the value should be set only for the columns block, not the nested blocks.

This area gets pretty nuanced, so I just want to make sure I understand the issue here. If you're setting styles.blocks.core/columns.spacing.blockGap to a particular value (e.g. 12px) then the value will be output as .wp-block-columns.is-layout-flex { gap: 12px; } so in that situation there shouldn't be any need for the theme to provide its own CSS to override the gap. The root level --wp--style--block-gap variable is output for backwards compatibility, as there are blocks and themes that depend on the value, but it's no longer used directly for block-level gap provided in theme.json. The intention is for the blockGap feature to not depend on that CSS variable, but it's still output so that we don't break existing themes, or blocks that haven't yet been migrated to the direct value approach (e.g. Gallery block).

that leaving settings.spacing.blockGap as null (the default value) leaves the value 2em hard-coded as a specific inline rule, which cannot be overridden using e.g. .wp-block-columns

That's a great point — I wonder if it's possible for us to come up with a rule that when the fallback gap is in use, we use a lower-specificity selector (e.g. attached to .wp-block-columns instead of .wp-block-columns.is-layout-flex). I'll have a play with that and see what our options are.

@andrewserong
Copy link
Contributor

@markhowellsmead alternately, I have an open PR in #42544 that proposes adding a theme support disable-layout-styles that would allow themes to entirely opt-out of Layout styles. Is that something useful for this case, or is this one where the theme still requires Layout styles, but it's just the fallback gap that you want to get rid of?

@markhowellsmead
Copy link
Author

Current status with Core 6.1.1 is that the default gap on the columns block can be overridden by styles.blocks.core/columns.spacing.blockGap, and it is possible to override the value using a custom style (registerBlockStyle) .is-style-wide-gap with a manually-defined value in the theme/plugin CSS.

@andrewserong
Copy link
Contributor

Thanks for confirming @markhowellsmead! Since that gives folks two potential avenues to overriding the default column gap, I'll close out this issue now. Please feel free to either re-open, or create a new issue if there are follow up tasks you'd like to raise. Thanks again for all the discussion here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Columns Affects the Columns Block CSS Styling Related to editor and front end styles, CSS-specific issues.
Projects
None yet
Development

No branches or pull requests

7 participants