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

Global Styles: Expose body level settings as CSS variables #39432

Closed
wants to merge 1 commit into from

Conversation

scruffian
Copy link
Contributor

@scruffian scruffian commented Mar 14, 2022

What?

This exposes some of the styles from theme.json as CSS variables so that blocks and themes can build on them.

Why?

The source of truth for these styles should be the Global Styles, and in many cases these rules are able to cascade to other blocks using CSS inheritance, but this doesn't work if we want the style to apply to a different property of a block, e.g. we might want to apply the text color to the border of a table.

This takes the ideas from #29714 and spins out a new PR to deal with some of the issues raised in #38998.

How?

For all properties inside "styles", except those in the "blocks" and "elements" array, we create CSS variables.

Testing Instructions

  1. Use a theme that has body level styles in the theme.json file e.g. twentytwentytwo
  2. Check that these styles are output as CSS variables with the body selector:
body {
--wp--styles--spacing--block-gap: 1.5rem;
--wp--styles--color--background: var(--wp--preset--color--background);
--wp--styles--color--text: var(--wp--preset--color--foreground);
--wp--styles--typography--font-family: var(--wp--preset--font-family--system-font);
--wp--styles--typography--line-height: var(--wp--custom--typography--line-height--normal);
--wp--styles--typography--font-size: var(--wp--preset--font-size--medium);
}

@scruffian scruffian added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Mar 14, 2022
@scruffian scruffian self-assigned this Mar 14, 2022
Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I see here is that these vars become a public CSS API which makes the form of theme.json a fixed contract. Using these vars liberally makes it so that potential theme.json updates become problematic for themes.

cc @youknowriad

return;
}

$theme_json_styles = _wp_array_get( $settings, array( 'styles' ) );
Copy link
Contributor

@draganescu draganescu Mar 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment here explaining what "root" means to understand the unsets would be good.

@oandregal
Copy link
Member

The source of truth for these styles should be the Global Styles, and in many cases these rules are able to cascade to other blocks using CSS inheritance, but this doesn't work if we want the style to apply to a different property of a block, e.g. we might want to apply the text color to the border of a table.

I'd like to unwrap a bit the use case to make sure we explore the solution space.

This is how a theme can create the above design (both the root text and the table border have the same color):

{
  "styles": {
    "color": {
      "text": "var(--wp--preset--color--YELLOW)"
    },
    "blocks": {
      "core/table": {
        "border": {
          "color": "var(--wp--preset--color--YELLOW)"
        }
      }    
    }
  }
}

So, a theme can already do this. However, what happens when the user changes the value of the top-level text color to any other thing? This is the core issue, in my view, and we discussed this in a few other places, including this thread.

If the user modifies the text color the values are no longer "connected":

{
  "styles": {
    "color": {
      "text": "var(--wp--preset--color--RED)"
    },
    "blocks": {
      "core/table": {
        "border": {
          "color": "var(--wp--preset--color--YELLOW)"
        }
      }    
    }
  }
}

If this is the problem we want to solve, the root cause is that we don't have a way to maintain the "design links" between the different components. Note that there's other issues but we should discuss them separately (see).

This has been "theme territory" so far and I've shared here a way for themes to address this: it involves the theme creating more CSS Custom Properties and wire them up itself. If we want the framework to absorb this issue, we can do it differently.

What if a theme could declare "design links"?

{
  "styles": {
    "color": {
      "text": "var(--wp--preset--color--yellow)"
    },
    "blocks": {
      "core/table": {
        "border": {
          "color": "styles.color.text"
        }
      }    
    }
  }
}

The CSS output:

body {
  color: var(--wp--preset--color--yellow);
}
.wp-block-table > table {
  /* NOTE how this value has been resolved to the value of styles.color.text */
  border-color: var(--wp--preset--color--yellow);
}

When we generate the styles, the framework will resolve styles.color.text to the specific color used there. If the user changes the text color, the table border color would be updated accordingly. We won't enqueue more CSS Custom Properties, but we're telling the framework the "design links" provided by the theme.

Thoughts on this approach:

  • While this can provide value at the framework level, the user can still "break the design links". In the example above, by setting the table border color to a specific color.
  • I think "breaking the design links" is a valid use case that we should allow.
  • Though, if we wanted to explore this route, I think it'd be benefitial to surface the "design links" to the user in the UI as well, so they can make their own decisions: intentionally break the links, maintain them, add new ones, etc.
  • Adding to the UI components the ability to see the "design links" is technically doable. In my view, the complexity of surfacing the "design links" in the UI is not technical but humane: I'm not sure how easy is going to be for the user to understand "the design links" of a particular theme that they have just installed.

@scruffian
Copy link
Contributor Author

@oandregal this is a great idea for themes, but I'm not sure how it would work for default block styles - do you have any thoughts about that? We could add these kind of rules to the default core theme.json, would that be appropriate?

@oandregal
Copy link
Member

but I'm not sure how it would work for default block styles

Do you mean how blocks can also take into account "user choices"? Don't they already have via the block supports? Do you have any use case in mind? I know there's still cases that are weird but, in general, my thinking is that we need to keep improving on that (moving the styles generation for all block supports to the server -aka "styles engine"-should help address these issues).

There's a another angle that we left unexplored: the block's CSS should be another source of styles that is merged with the rest. We'd have something like "defaults (core theme.json) < blocks < theme (the theme's theme.json) < user (styles provided via GS sidebar)". I've got a PR for this at #34180

@scruffian
Copy link
Contributor Author

Don't they already have via the block supports?

I'm not sure how. We have a case like this for the file block - we would like the file block to have a background color that uses the currentColor as defined by the theme. It looks like we could combine #34180 with your idea above to achieve that...

@aristath aristath changed the title Global Styles: Expose body level settings as CSS varaibles Global Styles: Expose body level settings as CSS variables Mar 18, 2022
@oandregal
Copy link
Member

I'm not sure how. We have a case like this for the #39118 (comment) - we would like the file block to have a background color that uses the currentColor as defined by the theme. It looks like we could combine #34180 with your idea above to achieve that...

Ah, I see what you mean. That would be feasible just with "design links", right? No need for absorving the block's CSS. We could add those to core's theme.json as in:

{
  "core/file": {
    "color": {
      "background": "style.color.background"
    }
  }
}

@scruffian
Copy link
Contributor Author

Yes that's correct, we just need to build that affordance!

@scruffian
Copy link
Contributor Author

Closing in favour of #41696

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants