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

Inline container css overriding css generated by theme.json #40813

Closed
simonwammerfors opened this issue May 4, 2022 · 17 comments
Closed

Inline container css overriding css generated by theme.json #40813

simonwammerfors opened this issue May 4, 2022 · 17 comments
Labels
[Block] Paragraph Affects the Paragraph Block [Feature] Layout Layout block support, its UI controls, and style output. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended

Comments

@simonwammerfors
Copy link

simonwammerfors commented May 4, 2022

I've started to try to get into developing block themes from scratch (using @carolinan excellent generator at fullsiteediting.com to generate an empty starter theme), and I've run into a problem I'm unsure how to solve. It would be great if someone with more insight could point me in the right direction.

It turns out that some inline css outputted by WP for the different block containers is overriding CSS generated by theme.json. This kind of CSS:

.wp-container-9 > * { margin-block-start: 0; margin-block-end: 0; }

overrides the spacing css outputted by theme.json:

p { margin-top: 1em; margin-bottom: 1em; }

I had a suspicion that global styles CSS interferes here, but I have reset all GS and this persists...

I'm using a fresh local install, no plugins except for Gutenberg.

@amustaque97
Copy link
Member

Hi @simonwammerfors, thank you for reporting this. I would appreciate if you can provide some more info in terms of reproduction steps so that we try to investigate and proceed further. Thank you.

@amustaque97 amustaque97 added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Status] Needs More Info Follow-up required in order to be actionable. labels May 4, 2022
@simonwammerfors
Copy link
Author

Sure.

  1. Spin up a fresh install.
  2. Create a blank block theme. I used the generator at fullsiteediting.com to create an empty theme.
  3. Edit theme.json and add values to settings.layout.contentSize and settings.layout.wideSize
  4. WP now output inline CSS like .wp-container-7 > * {margin-block-start: 0;margin-block-end: 0;}
  5. Add some margin for paragraphs in theme.json
  6. Go to the Hello world post (or any other post)
  7. The margins for the p elements now gets overridden by the container css

@simonwammerfors
Copy link
Author

Do you have any thoughts on this @carolinan?

@github-actions
Copy link

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label May 21, 2022
@adamwoodnz
Copy link

Could you please show me some code for how you're styling paragraphs in theme.json (5)?

@adamwoodnz adamwoodnz removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label May 31, 2022
@simonwammerfors
Copy link
Author

Hi

"blocks": {
	 "core/paragraph": {
		"spacing": {
			"margin": {
				"top": "1em",
				"bottom": "1em"
			}					
		}
	}
}

This is what I use. The inline styles for the p elements also shows up front-end but is overridden by the margin-block-start and margin-block-end rules.

Skärmavbild 2022-05-31 kl  07 54 23

@adamwoodnz
Copy link

adamwoodnz commented May 31, 2022

Thanks, I have reproduced this, with or without setting settings.layout.contentSize and settings.layout.wideSize.

This is a CSS specificity issue; .wp-container-7 > * is more specific than p so it takes precedence.

https://github.com/WordPress/wordpress-develop/tree/trunk/src/wp-content/themes/twentytwentytwo is a good resource for theme configuration I find. If you look at the way it manages spacing for this type of content it doesn't style the paragraph block directly, rather they set styles.spacing.blockGap (https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-content/themes/twentytwentytwo/theme.json#L343) and this changes the container margin:

Screen Shot 2022-06-01 at 10 26 28 AM

If you do want to target the p tag directly, probably the best way to fix this is to add more specific rules for your p tags using your stylesheet rather than theme.json, eg. p tags in block post content:

Screen Shot 2022-06-01 at 9 46 26 AM

To do this you'll need to enqueue styles in your theme (this doesn't seem to be done in the generated theme):

{theme}/functions.php

if ( ! function_exists( 'my_theme_styles' ) ) :

	/**
	 * Enqueue styles.
	 *
	 * @since My Theme 1.0
	 *
	 * @return void
	 */
	function my_theme_styles() {
		// Register theme stylesheet.
		$theme_version = wp_get_theme()->get( 'Version' );

		$version_string = is_string( $theme_version ) ? $theme_version : false;
		wp_register_style(
			'my-theme-style',
			get_template_directory_uri() . '/style.css',
			array(),
			$version_string
		);

		// Enqueue theme stylesheet.
		wp_enqueue_style( 'my-theme-style' );

	}

endif;

add_action( 'wp_enqueue_scripts', 'my_theme_styles' );

{theme}/style.css

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

These styles won't be applied in the editor (only the frontend), but there is a good post covering editor styles here: https://css-tricks.com/getting-the-wordpress-block-editor-to-look-like-the-front-end-design/

Hope this helps.

@adamwoodnz adamwoodnz added the [Type] Help Request Help with setup, implementation, or "How do I?" questions. label May 31, 2022
@simonwammerfors
Copy link
Author

Hi

Thanks for this. Using blockGap certainly mitigates this a lot!

If I understand things correctly, giving the p tag margin via the theme.json will always be overridden by other styles generated by WordPress core. Surely that can't have been the intention? Or are there use cases where this theme.json generated css actually will make any difference?

@ooksanen
Copy link

ooksanen commented Jun 9, 2022

@adamwoodnz I think the actual problem here is the fact that blockGap is used for all blocks, even for paragraphs and headings, and your suggestion for setting blockGap doesn't exactly fix that, although it might've helped @simonwammerfors somewhat.

For example on a theme I'm currently developing, I've set the block gap to be clamp(2rem, 5vw, 4rem) so that if I use, say columns block, the margin between columns is that value, or if I insert two image blocks after each other, the vertical gap between them is that value. However, now if I insert text (paragraphs and/or headings) inside a column or a Media & Text block, the paragraph and heading text blocks are getting their margins (I've defined in my theme) overridden by rules like .wp-container-2 > * and .wp-container-2 > * + * which imho leads to a CSS specificity race between the theme developer and Gutenberg team.

Instead of just having to just set a margins for my paragraphs and headings, I now (to fix the problem of ridiculously large margins between paragraphs and headings, caused by this change) have to give them rather specific CSS rules overriding those inline styles in case they happen to be inside columns, group, media & text etc.. And honestly I'm a bit worried that at some stage those (nested) blocks might have even more specific rules which forces to me to yet again make some more specific rules for my basic text tags and soon I'll have to define my typography on a single block level to override core styles...

EDIT: To add to this problem, I would obviously like to have my blockGap as I've set it between layout-type blocks like columns, image, cover, etc. so that I can keep my layout as designed, and not having to jump through hoops to keep my typography styles, like margins, too.

EDIT2: Now that I started to fix the spacing problems caused by these inline styles I realize how ridiculously much they break all my spacing rules for all the blocks inside columns. And boy are these not fun to override. Basically I need to add very specific rules for paragraphs, headings, hr, etc etc that are inside a column block... [this-really-grinds-my-gears.gif]

@github-actions
Copy link

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Jun 25, 2022
@ooksanen
Copy link

Did you have any further thoughts / input on this subject @adamwoodnz? Is this something the devs will just have to live with?

@adamwoodnz adamwoodnz removed the [Status] Needs More Info Follow-up required in order to be actionable. label Jun 28, 2022
@adamwoodnz
Copy link

Did you have any further thoughts / input on this subject @adamwoodnz? Is this something the devs will just have to live with?

Hi, sorry but I don't have a huge amount of experience working with block styles yet. My expectation would be that the block gap setting would be used for controlling your overall layout spacing, and you'd use CSS for styling of your heading and text blocks. My example above doesn't use particularly specific rules and manages to override the container children rules.

Could you perhaps share screenshots of your page and devtools showing the overrides? That might help me to understand the problem better and do some research/testing.

@ooksanen
Copy link

ooksanen commented Jun 28, 2022

Sure! In my opinion the biggest issue is the use of * wildcard selector, which targets all elements inside all .wp-container-n (at least columns, likely some other blocks as well). I believe that the wildcard selector is too broad for a very common pattern of layout; placing headings, paragraphs, etc. inside columns.

For example to control the spacing between paragraphs and/or headings inside columns I need to add rules like following (I'm targeting class beginning with wp-container to avoid having to add rules for every possible container number) for every inline/text element placed inside columns:

[class^="wp-container"] > p {
	-webkit-margin-before: 0;
	margin-block-start: 0;
	-webkit-margin-after: 1.5rem;
	margin-block-end: 1.5rem;
}
[class^="wp-container"] > h1, 
[class^="wp-container"] > h2, 
[class^="wp-container"] > h3, 
[class^="wp-container"] > h4, 
[class^="wp-container"] > h5, 
[class^="wp-container"] > h6 {
	-webkit-margin-after: 0.65rem;
	margin-block-end: 0.65rem;
	-webkit-margin-before: 0;
	margin-block-start: 0;
}

And these need to be added to all elements/blocks inside columns, unless the blockGap value is suitable for every element, which it likely isn't. So for example if I were to add a <hr> between paragraphs, I'd again need to add hr specific rule to override the wildcard inline style to avoid the spacing from blockGap value:

[class^="wp-container"] > hr {
	-webkit-margin-before: 1.5rem;
	margin-block-start: 1.5rem;
	-webkit-margin-after: 1.5rem;
	margin-block-end: 1.5rem;
}

Want to add a blockquote in there? Add rules for overriding the blockGap. How about we add a list (ul/ol)? Yep, add overrides.

Below is a real world example with the default styling that has the blockGap or --wp-style-block-gap applied to all children of column/s so that all the headings and paragraphs inherit it:

image

And here's how the section looks like with my own styling like the examples above:

image

EDIT: I just had a better look at your examples and indeed targeting a paragraph inside higher level ancestor (in my case that could be the main content wrapper .hentry or similar) than the .wp-container-n actually has a higher specificity than ´.wp-container-n > *´ and thus is a cleaner/more efficient way to override those styles, but I still think it adds unnecessary complexity and I don't think we should need to add that level of specificity for basic content elements that are used throughout the site. And also this doesn't change the fact that I need to add more specificity to all CSS rules for elements inside columns.

Even though this can be easily fixed especially if using Sass or similar, a simple, clean rule for paragraph, headings, lists, etc. should be enough.

And now that I think of it, it also adds a layer of complexity for non-technical users, who might be adding simple CSS overrides in their theme's custom CSS settings. They most likely won't know about CSS specificity and just Google something like "how do I change the spacing between paragraphs", then try to add a simple rule, like:

p {
	margin-bottom: 30px;
}

and then wonder why their changes won't work.

@simonwammerfors
Copy link
Author

simonwammerfors commented Jun 28, 2022

If I understand things correctly, specifying the margin for a p element in theme.json always get overridden by block gap css, even if there is no value for block gap in theme.json set. This seem strange, after all we are given the possibility to specify margin on elements through theme.json. Shouldn't the general block gap css be overridden if I choose to specify margin for a specific element/block in theme.json?

@skorasaurus skorasaurus removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Jul 5, 2022
@mrfoxtalbot
Copy link

mrfoxtalbot commented Oct 3, 2022

Hey @simonwammerfors, I just wanted to reference #40875 in case this affects (or fixed!) this issue. Could you please take a a quick look? Thank you!

@tellthemachines
Copy link
Contributor

This is a similar specificity issue to #33795. I'm going to label it as a bug because it's unintended behaviour. It is still reproducible in trunk.

@tellthemachines tellthemachines added [Type] Bug An existing feature does not function as intended [Block] Paragraph Affects the Paragraph Block [Feature] Layout Layout block support, its UI controls, and style output. and removed [Type] Help Request Help with setup, implementation, or "How do I?" questions. labels Oct 21, 2022
@tellthemachines
Copy link
Contributor

Closing since #47858 has now been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Paragraph Affects the Paragraph Block [Feature] Layout Layout block support, its UI controls, and style output. [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

7 participants