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

group block padding can't be set by theme.json #33456

Closed
andrewstaffell opened this issue Jul 15, 2021 · 8 comments · Fixed by #34854
Closed

group block padding can't be set by theme.json #33456

andrewstaffell opened this issue Jul 15, 2021 · 8 comments · Fixed by #34854
Assignees
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@andrewstaffell
Copy link

andrewstaffell commented Jul 15, 2021

Description

Group block padding (with background color) can't be configured through theme.json.

Step-by-step reproduction instructions

  1. Create theme.json:
{
  "version": 1,
  "styles": {
    "blocks": {
      "core/group": {
        "spacing": {
          "padding": "1rem"
        }
      }
    }
  }
}
  1. Insert Group block and assign any background colour
  2. Check block padding – should be 1rem following theme.json but is actually 1.25em 2.375em. The 1rem rule is output in the stylesheet (for .wp-block-group) but is overridden.

Expected behaviour

Padding should be 1rem – and it should only apply to group blocks with a background. Maybe this requires a different syntax in theme.json but this seems to be undocumented.

Actual behaviour

Group block padding is set indiscriminately 1.25em 2.375em, overriding any theme.json setting, and not specific to group blocks with background colour set.

WordPress information

  • WordPress version: 5.8 RC3
  • Gutenberg version: 11.0.0
  • Are all plugins except Gutenberg deactivated? Yes
  • Are you using a default theme (e.g. Twenty Twenty-One)? Yes, have tried with Twenty-Twenty One but it adds loads of its own CSS which obscures the problem. It's better exposed by a more minimal theme.

Device information

  • Device: MacBook Pro
  • Operating system: Mac OS 11.4
  • Browser: Chrome 92
@skorasaurus skorasaurus added the [Block] Group Affects the Group Block label Jul 15, 2021
@carolinan
Copy link
Contributor

carolinan commented Jul 16, 2021

Hi
I am using a block theme, with WordPress 5.8-RC4-51447 and Gutenberg 11.0.0

This is what I am seeing: The 1rem padding works correctly as the default in the editors and front when it is set in theme.json.
I did not try with a classic theme with a theme.json.

I disagree that the padding should only be present when a background color is used. The group block is often used as a container, independent of the background.

I can see the benefit of being able to update the default padding specifically for when a background color is used, to have two different paddings, but that is not possible with theme.json right now.

Without background color:

Group block padding added in theme.json match the theme.json file.

With background color (both custom and preset)

Group block with background, the padding added in theme.json match the theme.json file

@andrewstaffell
Copy link
Author

Thanks @carolinan for the reply and sorry for the slow acknowledgement, I got buried in another project and it took me a while before I could test this again. The problem for me seems to be that the .wp-block-group coming from theme.json doesn't get auto-wrapped in .editor-styles-wrapper as in your example, and therefore it gets overridden by the default block style. Here's a screenshot of the equivalent definitions in dev tools, to compare with yours.

Screenshot 2021-08-31 at 14 24 18

The theme.json config is as above. Why might this be different?

I'm on WP 5.8 and Gutenberg 11.3.0.

@carolinan
Copy link
Contributor

I believe you need to set top, right, bottom and left values, instead of one single value.
Example:

				"spacing": {
					"padding": {
						"top": "0.3rem",
						"right": "1rem",
						"bottom": "0.3rem",
						"left": "1rem"
					}
				}

(it would be nice if both worked but I don't think it does)

@andrewstaffell
Copy link
Author

Those are still getting overridden for me:

Screenshot 2021-08-31 at 18 05 55

Why would that declaration be getting wrapped in .editor-styles-wrapper in your installation but not mine?

@andrewstaffell
Copy link
Author

andrewstaffell commented Aug 31, 2021

Sorry, just realised the answer to that. My dev tools screengrab is from the front-end whereas yours was from the editor. Doh!

In essence, it's fine in the editor but not on the front end. The .has-background declaration overrides the values set in theme.json and there's no way to get them back [this goes back to the original reason for posting the issue].

Any suggestions?

This might indicate a generalised issue with theme.json specificity being inferior to core block styles?

@carolinan
Copy link
Contributor

I retested, and I am also seeing different results in the editor vs the front now.
You are absolutely correct that the CSS for the background color padding is more specific on the front, while the theme.json padding is more specific in the editor 🤔

@carolinan carolinan added Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended and removed [Block] Group Affects the Group Block labels Sep 15, 2021
@carolinan
Copy link
Contributor

So first a decision needs to be made about what the expected result is, if the theme.json default is intended to override the background color padding or not.

Thoughts @oandregal ?

@oandregal
Copy link
Member

#34854 a try to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants