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

Heading: Update the order of style nodes for correct css specificity #40819

Conversation

madhusudhand
Copy link
Member

What?

This PR fixes following two issues.

  1. When values are provided in both styles.blocks.core/heading and styles.elements.h1 the values provided in the "blocks.core/heading" portion are leveraged rather than the "elements.h1" section.

  2. Also styles.blocks.core/heading.elements.h1 produces h1 h1 selector instead of h1, because of this overrides are not working.

Fixes: #39698

Why?

Use case would be user can define generic styles such as fontFamily at core/heading which would apply to all headings and can define elements.h6 with a different fontFamily to override.

Currently this is not working because of CSS specificity issues (order of the style placement is not right)

How?

Changed the order of blocks and elements so that result styles are in right overriding order.

Given theme.json definition

"blocks": {
  "core/heading": {
    "typography": {
      "fontWeight": "400",
      "fontSize": "20px"
    },
    "elements": {
      "h1": {
        "typography": {
          "fontSize": "30px"
        }
      }
    }
  }
},
"elements": {
  "h1": {
    "typography": {
      "fontSize": "min(max(2.625rem, 5vw), 4.5rem)"
    }
  }
}

Following is the result.

image

Testing Instructions

Case 1:

Define styles under styles.blocks.core/heading and override them in styles.elements.h1.
Later styles should apply in frontend.

Case 2:

Define styles under styles.blocks.core/heading and override them in styles.blocks.core/heading.element.h1
Later styles should apply in frontend.

Screenshots or screencast

@github-actions
Copy link

github-actions bot commented May 4, 2022

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @madhusudhand! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label May 4, 2022
@madhusudhand madhusudhand changed the title Update the order of style nodes for correct css specificity Heading: Update the order of style nodes for correct css specificity May 5, 2022
@madhusudhand madhusudhand added the [Block] Heading Affects the Headings Block label May 5, 2022
@madhusudhand madhusudhand force-pushed the fix/theme-json-style-nodes-order branch from c1835a6 to d318eba Compare May 6, 2022 11:18
@madhusudhand madhusudhand force-pushed the fix/theme-json-style-nodes-order branch from d318eba to 2340cab Compare May 6, 2022 11:44
@scruffian
Copy link
Contributor

Thanks so much for working on this. I have some concerns with this approach:

  1. I think conceptually elements should come before blocks. Elements are more generic and can be used in multiple blocks, so they should be use as a base which blocks then modify. Imagine we wanted to define all buttons in all blocks to have one color but the button inside one particular block to look different - then we would want the block definitions to come after element definitions.

  2. I don't think we should be tying the implementation of theme.json to specific blocks unless necessary.

I do agree that we need to fix the issue of defining elements inside headings. I have proposed a fix for that here:
#40889

@madhusudhand
Copy link
Member Author

Closing this in favor of #40889

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 First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

theme.json styles.blocks.core/heading values take precedence over styles.elements.h1
2 participants