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

Styling set on the core/heading block of theme.json gets applied to all h1, h2, h3, etc. #42082

Closed
polarstoat opened this issue Jul 1, 2022 · 13 comments
Labels
[Block] Heading Affects the Headings Block CSS Styling Related to editor and front end styles, CSS-specific issues. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Question Questions about the design or development of the editor.

Comments

@polarstoat
Copy link

Description

Styles set on styles.blocks.core/heading of theme.json are applied as markup that effects all h1, h2, h3, h4, etc. tags. This means unless HTML heading tags have their own explicitly set styles that would override styles.blocks.core/heading, they inherit that.

For example, setting

			"core/heading": {
				"color": {
					"text": "red"
				},
				"typography": {
					"fontSize": "99px"
				}
			},

in theme.json results in the generated inline css of

<style id='wp-block-heading-inline-css'>
h1.has-background, h2.has-background, h3.has-background, h4.has-background, h5.has-background, h6.has-background {
    padding: 1.25em 2.375em
}

h1, h2, h3, h4, h5, h6 {
    color: red;
    font-size: 99px;
}
</style>

Uses of HTML heading tags elsewhere in the theme (for example core/post-title) then have the styles for core/heading applied, unless they are specifically overridden.

This also causes inconsistency between posts/pages that do and don't have the Heading block in them, as the above inline CSS is only inserted on those posts/pages that include a Heading block somewhere (see wpengine/frost#96).

Step-by-step reproduction instructions

  1. Start with a clean Wordpress 6.0, Twenty Twenty Two install
  2. Set some outrageous styling on core/heading (as above)
  3. Create a post that somewhere features the Heading block
  4. Create an otherwise identical post that does not include the Heading block
  5. Compare the difference in results

Screenshots, screen recording, code snippet

No response

Environment info

Wordpress 6.0, Twenty Twenty Two 1.2, Gutenberg 13.5.2

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

@ramonjd ramonjd added Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json CSS Styling Related to editor and front end styles, CSS-specific issues. [Type] Question Questions about the design or development of the editor. labels Jul 1, 2022
@carolinan
Copy link
Contributor

I am able to reproduce this with Gutenberg active, but not with only WordPress 6.0.

@WordPress/block-themers Do we need to add the wp-block-heading class to the block on the front and apply the styles to the class? Adding it would also match the editor better, the class is already used there.

I don't remember the arguments against adding the class from the start, it might have been related to there being so many headings on some pages, like archives, that adding the class would be cluttering.

@carolinan carolinan added the [Block] Heading Affects the Headings Block label Jul 1, 2022
@cdils
Copy link

cdils commented Jul 1, 2022

I'm seeing this behavior with Gutenberg deactivated as well.

@pbking
Copy link
Contributor

pbking commented Jul 2, 2022

I thought NOT leveraging a wp-block-heading class was strange. I definitely support transitioning to the class in the view. It's a lot more explicit about the intent and gives us better control.

@carolinan
Copy link
Contributor

It would change heading element styles for themes that has relied on this "feature" until now...

@carolinan
Copy link
Contributor

Aside: We will probably need to add the class to the paragraph block as well: #35323

@scruffian
Copy link
Contributor

The solution to this will be to add a wp-block-heading class to heading blocks, but we should ensure that this isn't serialized.

@scruffian
Copy link
Contributor

This is blocked by #42485

adamziel added a commit that referenced this issue Nov 28, 2022
## What?
This PR adds the `wp-block-heading` CSS class to the heading block.

It happens server-side so:

* The class couldn't be accidentally removed from the serialized markup
* There's no need to migrate the existing posts

## Why?

Without the classname, global styles can't distinguish between the `h1-h6` elements that belong to the heading block vs ones that don't. As a result, [the styles set on the `core/heading` block of theme.json gets applied to all h1, h2, h3, etc](#42082)

## How?

This PR transforms the stored `heading` block markup into one with an additional class name using  [WP_HTML_Tag_Processor](https://make.wordpress.org/core/2022/08/19/a-new-system-for-simply-and-reliably-updating-html-attributes/). 

## Alternatives considered

I explored leaning on `get_block_wrapper_attributes`, but it falls short:

* It infers attributes from globals.
* It only processes the `style` and `class` properties, meaning a heading block with an anchor set would lose its `id`.
* It ignores any additional attributes inside of the block content.
* We'd still need to some parsing to remove the wrapping block from `$content`.

## Testing Instructions

1. Create a new page
2. Add a few heading blocks
3. Align text to right in one of them
4. Add a custom CSS classname in another
5. Save
6. Visit the page, confirm the rendered headings have the `wp-block-heading` css class
@skorasaurus
Copy link
Member

can someone verify in the latest build (trunk) (which has #42122) that this is fixed?

@scruffian
Copy link
Contributor

#42122 isn't enough on it's own, we also need to do this: #46284

@scruffian
Copy link
Contributor

This is fixed by #46284

@scruffian
Copy link
Contributor

This is still not possible, I'm no longer sure what the correct solution is.

@scruffian scruffian reopened this Mar 23, 2023
@tellthemachines
Copy link
Contributor

This is still not possible, I'm no longer sure what the correct solution is.

What are we missing here? I can no longer reproduce the original issue; adding the proposed styles to theme.json results in

.wp-block-heading {
    background-color: var(--wp--preset--color--primary);
    color: red;
    font-size: clamp(49.777px, 3.111rem + ((1vw - 3.2px) * 5.594), 99px);
}

@scruffian
Copy link
Contributor

I think there is some confusion (for me at least!) about the priority of element styles vs block styles. If a theme sets heading element styles and block styles, the block takes priority over the elements. This means that you should only set colors for a heading block if you want heading blocks to look different from other blocks. If you want them to all match then use elements. I agree @tellthemachines I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Heading Affects the Headings Block CSS Styling Related to editor and front end styles, CSS-specific issues. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Question Questions about the design or development of the editor.
Projects
None yet
Development

No branches or pull requests

8 participants