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

Expose presets declared via add_theme_support in global styles #22076

Merged
merged 4 commits into from
May 6, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions lib/global-styles.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,56 @@ function gutenberg_experimental_global_styles_get_core() {
}

/**
* Return theme's Global Styles.
* Return theme's Global Styles. It also fetches the editor palettes
* declared via add_theme_support.
*
* @return array Global Styles tree.
*/
function gutenberg_experimental_global_styles_get_theme() {
return gutenberg_experimental_global_styles_get_from_file(
locate_template( 'experimental-theme.json' )
$theme_supports = array();
// Take colors from declared theme support.
$theme_colors = get_theme_support( 'editor-color-palette' )[0];
if ( is_array( $theme_colors ) ) {
foreach ( $theme_colors as $color ) {
$theme_supports['preset']['color'][ $color['slug'] ] = $color['color'];
}
}

// Take gradients from declared theme support.
$theme_gradients = get_theme_support( 'editor-gradient-presets' )[0];
if ( is_array( $theme_gradients ) ) {
foreach ( $theme_gradients as $gradient ) {
$theme_supports['preset']['gradient'][ $gradient['slug'] ] = $gradient['gradient'];
}
}

// Take font-sizes from declared theme support.
$theme_font_sizes = get_theme_support( 'editor-font-sizes' )[0];
if ( is_array( $theme_font_sizes ) ) {
foreach ( $theme_font_sizes as $font_size ) {
$theme_supports['preset']['font-size'][ $font_size['slug'] ] = $font_size['size'];
}
}

/*
* We want the presets declared in theme.json
* to take precedence over the ones declared via add_theme_support.
*
* However, at the moment, it's not clear how we're going to declare them
* in theme.json until we resolve issues related to i18n and
* unfold the proper theme.json hierarchy. See:
*
* https://github.com/wp-cli/i18n-command/pull/210
* https://github.com/WordPress/gutenberg/issues/20588
*
* Hence, for simplicity, we take here the existing presets
* from the add_theme_support, if any.
*/
return array_merge(
gutenberg_experimental_global_styles_get_from_file(
locate_template( 'experimental-theme.json' )
),
$theme_supports
Copy link
Member

Choose a reason for hiding this comment

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

I guess $theme_supports should be before gutenberg_experimental_global_styles_get_from_file to give more priority to theme.json in case it uses theme supports and theme.json at the same time.

Copy link
Member Author

@oandregal oandregal May 6, 2020

Choose a reason for hiding this comment

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

Oh, this was intentional, I tried to capture why in the comment before the return statement: essentially, there's yet a few options to store the presets in the theme.json and I didn't want people to start using something like this in theme.json:

{
  // global styles stuff
  "presets": { /* this data would be processed */ }
}

Which brings me to a different question: at the moment, we don't validate that the keys in theme.json are valid or follow the schema we expect (global, blocks, etc), so anything that theme authors put there will be converted to variables as well. I think this is fine for this stage and I even add some validation in some of the PRs I have somewhere. However, I'm on the fence if this is a good or a bad thing: in a way, having that freedom could enable theme authors to use it creatively (custom CSS). Do you have any thoughts about that?

);
}

Expand Down