WordPress.org
Get WordPress

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#54550 closed defect (bug) (fixed)

The margin added at the top level styles (for body) in theme.json is override by a margin added from Gutenberg

Reported by: get_dave's profile get_dave Owned by: hellofromtonya's profile hellofromTonya
Milestone: 5.9 Priority: normal
Severity: minor Version: 5.9
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

Companion Issue for ​https://github.com/WordPress/gutenberg/issues/36147.

Essentially any margin added to the root element via theme.json is overwritten by a CSS reset margin value provided by the Gutenberg Plugin. The theme.json setting should take precedence.

I have a PR ready in ​https://github.com/WordPress/gutenberg/pull/36960 which requires a similar fix in WP Core.

Change History (12)

This ticket was mentioned in ​PR #1989 on ​WordPress/wordpress-develop by ​getdave.
4 years ago #1

  • Keywords has-patch added

Companion fix to ​https://github.com/WordPress/gutenberg/pull/36960.

Note there is now also an alternative approach to this in ​https://github.com/WordPress/gutenberg/pull/36960.

In ​https://github.com/WordPress/gutenberg/issues/36147 we learn that the Theme JSON class automatically applies a margin: 0 to the body element. Unfortunately it does so in a way that means if you use theme.json to define a margin on the body element, the default margin: 0 will still take precedence.

For example applying this to theme.json will still leave you with the _default_ margin: 0 on the body, rather than the _expected_ value of margin: 0 5rem:

{
	"styles": {
		"spacing": {
			"margin": "0 5rem",
			"padding": "0"
		},
	}
}

This PR fixes this by only applying the default margin: 0 if the root spacing does not have a margin declaration.

Trac ticket: https://core.trac.wordpress.org/ticket/54550

#2 @noisysocks
4 years ago

  • Milestone changed from Awaiting Review to 5.9

This ticket was mentioned in ​Slack in #core by audrasjb. ​View the logs.
4 years ago

This ticket was mentioned in ​Slack in #core-test by hellofromtonya. ​View the logs.
4 years ago

#5 @hellofromTonya
4 years ago

  • Keywords needs-testing added

​PR 1989 is ready for testing and a test report.

This ticket was mentioned in ​Slack in #core-test by hellofromtonya. ​View the logs.
4 years ago

#7 @hellofromTonya
4 years ago

  • Keywords commit added; needs-testing removed

Test Report

Env:

  • OS: macOS
  • Localhost: wp-env
  • Theme: TT2
  • Plugins: none
  • WordPress: trunk

Steps

  1. Add the following to the TT2's src/wp-content/themes/twentytwentytwo/theme.json file in the "styles" section on about line 333: 
		"spacing": {
			"blockGap": "1.5rem",
		  	"margin": "0 5rem",
		  	"padding": "0"
		},
  1. Run in the following commands in the console (terminal): 
npm run build
npm run env:start
npm run env:install
  1. Open the site in your favorite browser ​http://localhost:8889/
  2. In your browser, open the dev tools to the Elements tab and click on the <body> element. Look at the Styles.

Before the patch, the <body> margin will be 0 with the margin: 0 5rem; from the theme crossed out.

After applying the patch, the <body> margin: 0; is crossed out and the theme's margin: 0 5rem; is active.

Results

I can reproduce the issue.

Applying the patch resolves it.

Marking for commit.

#8 @hellofromTonya
4 years ago

  • Owner set to hellofromTonya
  • Status changed from new to reviewing

Self-assigning for commit.

#9 @hellofromTonya
4 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 52329:

Editor: Allow theme.json to take precedence in setting the root level element's reset margin value.

Any margin added to the root element via theme.json is overwritten by a CSS reset margin value provided by the Gutenberg Plugin.

This commit makes theme.json setting take precedence by generating the global styles body reset prior to processing the theme.json.

Follow-up to [52049].

Props get_dave, hellofromTonya.
Fixes #54550.

#11 @hellofromTonya
4 years ago

In 52330:

Themes: Better names for WP_Theme::is_block_theme() and wp_is_block_theme() and make wp_is_block_theme() a wrapper.

This commit renames the following method and function to better represent block theme terminology:

  • WP_Theme::is_block_based() to WP_Theme::is_block_theme()
  • wp_is_block_template_theme() to wp_is_block_theme()

It also changes wp_is_block_theme() to be a helper wrapper (sugar syntax) for wp_get_theme()->is_block_theme();. Why? To ensure both the method and function behave the same, to help Gutenberg maintain WordPress cross-version compatibility, and to make it less cumbersome to port changes from Gutenberg to Core.

Follow-up to [52069], [52247], [52279].

Props antonvlasenko, costdev, hellofromTonya, noisysocks.
Fixes #54550.

#12 @hellofromTonya
4 years ago

[52330] is supposed to go to ticket #54552. :facepalm:

Note: See TracTickets for help on using tickets.