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.

Sign up for GitHub

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

Jump to bottom

Fix default duotone preset SVG and style generation #38681

Merged
merged 22 commits into from
Mar 9, 2022
+221 −173
Merged

Fix default duotone preset SVG and style generation #38681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
01d593e
Fix spacing
Feb 9, 2022
51d2bc5
Add defaultDuotone to disable default SVGs
Feb 9, 2022
9072348
Disable duotone css vars too
Feb 9, 2022
ccb65ac
Add defaultDuotone to JSON schema
Feb 9, 2022
93f23af
Fix phpcs
Feb 9, 2022
7b4e11c
Update docs
Feb 9, 2022
795de03
Add defaultDuotone to prevent override
Feb 10, 2022
3cda90b
Try refactoring override path and bool to not be inverse
Feb 10, 2022
68cdf4e
Use multi-origin duotone presets
Feb 10, 2022
fe1c37a
Try refactor should_override_preset
Feb 10, 2022
6b65616
Prevent generating other default presets
Feb 10, 2022
ece841d
Fix conditional hook
Feb 16, 2022
d47041e
Add example for get_metadata_boolean
Feb 16, 2022
bb55054
Fix phpcs
Feb 16, 2022
e6ceb4b
Override core duotone rendering in plugin
Feb 21, 2022
67dbd4b
Use protected/static instead of private/self
Feb 21, 2022
10e3e26
Fix toolbar items for duotone
Feb 21, 2022
159afc3
Disable duotone generation in classic themes while keeping the defaul…
Feb 21, 2022
9de2494
Update schema description for defaultDuotone
Feb 21, 2022
1410ae1
Add comment about overriding
Feb 21, 2022
6bdf5d0
Fix comment style
Feb 21, 2022
77b04c7
Revert extra check in theme.json resolver
Feb 24, 2022
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
1 change: 1 addition & 0 deletions docs/reference-guides/theme-json-reference/theme-json-living.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Settings related to colors.
| custom | boolean | true | |
| customDuotone | boolean | true | |
| customGradient | boolean | true | |
| defaultDuotone | boolean | true | |
| defaultGradients | boolean | true | |
| defaultPalette | boolean | true | |
| duotone | array | | colors, name, slug |
Expand Down
208 changes: 117 additions & 91 deletions lib/compat/wordpress-5.9/class-wp-theme-json-5-9.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,79 +69,93 @@ class WP_Theme_JSON_5_9 {
*
* This contains the necessary metadata to process them:
*
* - path => where to find the preset within the settings section
* - override => whether a theme preset with the same slug as a default preset
* can override it
* - value_key => the key that represents the value
* - value_func => optionally, instead of value_key, a function to generate
* the value that takes a preset as an argument
* (either value_key or value_func should be present)
* - css_vars => template string to use in generating the CSS Custom Property.
* Example output: "--wp--preset--duotone--blue: <value>" will generate as many CSS Custom Properties as presets defined
* substituting the $slug for the slug's value for each preset value.
* - classes => array containing a structure with the classes to
* generate for the presets, where for each array item
* the key is the class name and the value the property name.
* The "$slug" substring will be replaced by the slug of each preset.
* For example:
* 'classes' => array(
* '.has-$slug-color' => 'color',
* '.has-$slug-background-color' => 'background-color',
* '.has-$slug-border-color' => 'border-color',
* )
* - properties => array of CSS properties to be used by kses to
* validate the content of each preset
* by means of the remove_insecure_properties method.
* - path => Where to find the preset within the settings section.
* - prevent_override => Whether a theme preset with the same slug as a default preset
* should not override it or the path to a setting for the same
ajlende marked this conversation as resolved.
Show resolved Hide resolved
* When defaults.
* The relationship between whether to override the defaults
* and whether the defaults are enabled is inverse:
* - If defaults are enabled => theme presets should not be overriden
* - If defaults are disabled => theme presets should be overriden
* For example, a theme sets defaultPalette to false,
* making the default palette hidden from the user.
* In that case, we want all the theme presets to be present,
* so they should override the defaults by setting this false.
Copy link
Member

@dmsnell dmsnell Feb 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

are we introducing this concept, prevent_override here?
is there a way to reframe it in the assertive positive sense? I always have trouble understanding contradictions or negative constructions. for example, instead of prevent_override = true maybe hide_defaults = false?

not that hide_defaults is certainly what we want, but it's actively what we're doing (or maybe "hide" is wrong, but we're dong something), instead of what we're not doing.

I guess this is an existing term, but I still find it confusing, especially how we bring that negation into the JS code with doNot = ! do; value = doNot ? EMPTY : value

Not a big deal, and it's in a rather complicated settings sourcing function so I get is. That's my only real feedback I think I'm able to provide here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we introducing this concept, prevent_override here? Is there a way to reframe it in the assertive positive sense? I always have trouble understanding contradictions or negative constructions.

It was kind of like that before, but I changed it hoping that it would make things less confusing despite being a negative construction. I'll explain my thought process for replacing override with prevent_override.

Arrays in the PRESETS_METADATA reference values in theme.json, so let's take this theme.json as an example. I'm using defaultPalette here, but defaultGradients and defaultDuotone should behave the same way.

{
  "settings": {
    "color": {
      "defaultPalette": false
    }
  }
}

Previously, there was a key called override that could be a path array or a boolean.

const PRESETS_METADATA = array(
  array(
    'override' => array( 'color', 'defaultPalette' ), // Path to a value in theme.json settings
  ),
  array(
    'override' => true, // A hard-coded value that doesn't have a corresponding theme.json setting
  ),
);

I expected the values to resolve like this when I was reading it.

const PRESETS_METADATA = array(
  array(
    'override' => false // Substitute in the value from theme.json
  ),
  array(
    'override' => true // Pass through a boolean
  ),
);

But actually, there was a function, should_override_preset, that would look up the value and invert it or just pass through a boolean without inverting it. So the actual substitution would look like this.

const PRESETS_METADATA = array(
  array(
    'override' => true // Substitute in the value from theme.json _and_ invert it
  ),
  array(
    'override' => true // Booleans don't get inverted
  ),
)

As part of this PR I added use_default_presets which also looks up a value from the theme.json, but does not need to invert the lookup.

const PRESETS_METADATA = array(
  array(
    'use_default_presets' => array( 'color', 'defaultPalette' ),
  ),
  array(
    'use_default_presets' => true,
  ),
);

It needs to resolve to this.

const PRESETS_METADATA = array(
  array(
    'use_default_presets' => false // Substitute in the value from theme.json
  ),
  array(
    'use_default_presets' => true // Pass through a boolean
  ),
);

Between the weird behavior of should_override_preset and wanting consistency for how theme.json settings are looked up, it made more sense to me to replace override with prevent_override and use a more generic get_metadata_boolean for both of them.

Does that make sense to you? Would you rather have inconsistent path array meanings or a negative construction? Or is there another simple option that I'm overlooking?

Copy link
Member

Choose a reason for hiding this comment

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

it's too bad the JSON file has the inversion, rather than something like, showDefaultPallette or useDefaultPallette or includeDefaultPalette

probably the confusion stems from having multiple speakers saying the same words but from different perspectives.

should prevent_override protect the default values, the hard-coded values, or the JSON file values?

in the PRESETS_METADATA examples are those priority lists? or are those two separate settings, such as palette and gradient etc…?

I guess the idea is that if we have true or false as the value of use_default_presets or for prevent_override then that value will be used int he end. if we set a path as an array for the field though it will read from the JSON and use that?

what is the relationship between prevent_override and use_default_presets? pardon my confusion; I'm trying to learn something about this system but still feel ignorant. is there a logic table with the inputs and desired outcomes for this?

My confusion peaks in lines like these. It's hard for me to read through the code and explain what impact $skip(_default_presets) is supposed to have on the end result.

$skip = ! static::get_metadata_boolean( $this->theme_json['settings'], false, true );
$skip = ! static::get_metadata_boolean( $this->theme_json['settings'], true, true );
$skip = ! static::get_metadata_boolean( $this->theme_json['settings'], [ 'color', $skip = 'defaultPalette' ], true );

Are "defaults" and "presets" different?

Copy link
Contributor Author

@ajlende ajlende Feb 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Lots of good questions! I'll answer them a bit out of order in a way that they can build upon one another.

Are "defaults" and "presets" different?

A preset is the metadata needed to generate the relevant global styles parts: CSS variables for everything, CSS classes for things like text/background color, and SVGs for things like duotone filters.

They come from three different origins listed here in the order that they are processed:

  • "default" presets are hard-coded in lib/theme.json. They were previously called "core" presets.
  • "theme" presets come from a block theme's theme.json file.
  • "custom" presets come from the styles sidebar of the site editor. They were previously called "user" presets.

In the theme.json, the presets would be the objects inside the duotone array, for example.

{
  "settings": {
    "color": {
      "duotone": [
        {
          // This object is a preset.
          "name": "Black and White",
          "slug": "black-and-white",
          "colors": [ "#000", "#FFF" ]
        }
      ]
    }
  }
}

Should prevent_override protect the default values, the hard-coded values, or the JSON file values?

prevent_override protects the slugs used by default presets.

So maybe we could name it protect_default_slugs or reserve_default_slugs or something? Would that make more sense?

The override option was added in #36811 and updated to use a path in #37008. tl;dr It was added to fix the UI by skipping theme slugs that already existed in the default palette. Theme developers that had "defaultPalette": false in their theme expected to be able to use these reserved default slugs, so it was then tied to defaultPalette.

These screenshots from #36811 illustrate it well—black and white are duplicate keys in TT1-blocks and were colored hot pink #FF69B4 for emphasis.

Before After
Captura de ecrã de 2021-11-24 10-35-44 Captura de ecrã de 2021-11-24 10-38-16

Let's run through an example with a slightly modified theme.json from the #36811 example to see what is being generated from the presets.

{
  "settings": {
    "color": {
      "defaultPalette": true,
      "palette": [
        {
          "slug": "black",
          "color": "#FF69B4"
        },
        {
          "slug": "dark-gray",
          "color": "#2A303C"
        },
        // ...other theme preset colors...
        {
          "slug": "white",
          "color": "#FF69B4"
        }
      ]
    }
  }
}

Here are the relevant CSS variables that are generated.

body {
  --wp--preset--color--black: #000000;
  --wp--preset--color--cyan-bluish-gray: #abb8c3;
  --wp--preset--color--white: #ffffff;
  /* ...other default preset colors... */
  --wp--preset--color--dark-gray: #2A303C;
  /* ...other theme preset colors... */
}

Notice that the white and black values we defined in theme.json do not get generated. The value for --wp--preset--color--white is #ffffff instead of #FF69B4. When defaultPalette is true, the default colors take precedence.

My expectation as a block theme creator would be that values I pass replace the default ones, but the expected outcome according to #36811 and #37008 is the other way around.

With "defaultPalette": false, this is what should be generated now.

body {
  --wp--preset--color--black: #2A303C;
  --wp--preset--color--dark-gray: #2A303C;
  /* ...other theme preset colors... */
  --wp--preset--color--white: #2A303C;
}

What is the relationship between prevent_override and use_default_presets?

prevent_override controls how theme presets with the same slug as default presets should be handled as explained above.

use_default_presets was added by this PR to wholesale prevent generating anything from default presets (including the SVGs in question) when its value is false. My take was that the color/gradient/duotone palettes should all behave the same, so I fixed the default generation for them as well.

In the case of the color/gradient/duotone palettes. I suppose you could say that use_default_presets obsoletes prevent_override.

However, in the case of fontSizes and fontFamilies, the values differ. The default values should always generate and the theme preset slugs should override the default preset slugs like they did before.

Is there a logic table with the inputs and desired outcomes for this?

use_default_presets prevent_override outcome
false N/A Nothing is generated from default presets
true false Generate default presets, but replace the value when the theme preset slug matches the default preset slug (theme wins)
true true Generate default presets, but skip the theme preset when the slug matches the default preset slug (default wins)

In the PRESETS_METADATA examples are those priority lists? Or are those two separate settings, such as palette and gradient etc…?

They are separate settings. You could add duplicates to the list, and I expect it would generate duplicates in the output.

However, ALLOWED_ORIGINS is kind of a priority list with the exceptions noted above for the default{Palette,Gradients,Duotone} option in theme.json.

I guess the idea is that if we have true or false as the value of use_default_presets or for prevent_override then that value will be used in the end. if we set a path as an array for the field though it will read from the JSON and use that?

Exactly.

Copy link
Member

Choose a reason for hiding this comment

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

that's extremely helpful and clarifying @ajlende, thank you for taking the effort to add that.

They were previously called "core" presets.…They were previously called "user" presets.

Oh no! those names make so much more sense to me 😆. I trust there was a good reason to diverge from them.

please don't let this derail the PR or the fixes, it's me doodling on the idea.

it's almost like we're looking at a natural hierarchy and talking about what behavior happens when finding duplicate values. the order will always be core (default) applies first, theme second, and user third. at any point in the hierarchy we can choose to ignore a new duplicated value and let the previous assignment remain or replace the previous assignment with our new value.

the kicker is that sometimes we don't want core values enter into the equation at all. we want a blank slate.

❓ is there any reason a custom config couldn't start clean as well and wipe out all of the previously-assigned values?

{
	// sets completely new duotone values regardless
	// of what was previously set
	"color": {
		"resetConfig": true,
		"duotone": { … }
	}
}

{
	// retains existing values but if we find a new
	// value for the same key, use the new one
	"color": {
		"duplicateKeys": "replace",
		"duotone": { … }
	}
}

{
	// retains existing values but if we find a new
	// value for the same key, use the old one
	"color": {
		"duplicateKeys": "retain",
		"duotone": { … }
	}
}

I wonder if this description could account not only for use_default but also prevent_override and make it work at both the theme and the custom levels. a theme could decide to resetConfig and the custom level could extend that.

on the other hand, it couldn't bring back the default levels with this description since the theme might have already reset them.

my other idea was wondering if we are standardized on default, theme, and custom, if we could go ahead and set the priority.

{
	// removes default config values entirely,
	// custom config values take priority over theme
	"color": {
		"configs": [ "theme", "custom" ],
		"duplicateKeys": "replace",
		"duotone": { … }
	}
}

while a bit more verbose this would give each level in the hierarchy the control to decide which values should appear and which should take precedence in a conflict. if a theme turned off the default values then a custom config could turn them back on. a custom config to let the defaults stay but wipe out the theme's colors.

any thoughts? way too much discussion about something that's already embedded in the system? my noodling is mostly around thinking through what exactly the behaviors are we're doing and avoiding special-casing in the interface, such as when calling out default in use_default.

Copy link
Contributor Author

@ajlende ajlende Feb 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Any thoughts? Way too much discussion about something that's already embedded in the system?

I'm happy to explain the existing system and how it drove my decisions for this bug fix, but, yeah, we're starting to get into big picture changes with some of these. I'm just trying to make sure SVG filters don't get left behind when changes are made to colors/gradients/etc. and, in this case, fixing a bug that is more obvious because of SVG filters.

Still, these questions might be interesting for @oandregal and/or @jorgefilipecosta to see since they are more involved in global styles than I am.

* - value_key => the key that represents the value
* - value_func => optionally, instead of value_key, a function to generate
* the value that takes a preset as an argument
* (either value_key or value_func should be present)
* - css_vars => template string to use in generating the CSS Custom Property.
* Example output: "--wp--preset--duotone--blue: <value>" will generate as many CSS Custom Properties as presets defined
* substituting the $slug for the slug's value for each preset value.
* - classes => array containing a structure with the classes to
* generate for the presets, where for each array item
* the key is the class name and the value the property name.
* The "$slug" substring will be replaced by the slug of each preset.
* For example:
* 'classes' => array(
* '.has-$slug-color' => 'color',
* '.has-$slug-background-color' => 'background-color',
* '.has-$slug-border-color' => 'border-color',
* )
* - properties => array of CSS properties to be used by kses to
* validate the content of each preset
* by means of the remove_insecure_properties method.
*/
const PRESETS_METADATA = array(
array(
'path' => array( 'color', 'palette' ),
'override' => array( 'color', 'defaultPalette' ),
'use_default_names' => false,
'value_key' => 'color',
'css_vars' => '--wp--preset--color--$slug',
'classes' => array(
'path' => array( 'color', 'palette' ),
'prevent_override' => array( 'color', 'defaultPalette' ),
'use_default_presets' => array( 'color', 'defaultPalette' ),
'use_default_names' => false,
'value_key' => 'color',
'css_vars' => '--wp--preset--color--$slug',
'classes' => array(
'.has-$slug-color' => 'color',
'.has-$slug-background-color' => 'background-color',
'.has-$slug-border-color' => 'border-color',
),
'properties' => array( 'color', 'background-color', 'border-color' ),
'properties' => array( 'color', 'background-color', 'border-color' ),
),
array(
'path' => array( 'color', 'gradients' ),
'override' => array( 'color', 'defaultGradients' ),
'use_default_names' => false,
'value_key' => 'gradient',
'css_vars' => '--wp--preset--gradient--$slug',
'classes' => array( '.has-$slug-gradient-background' => 'background' ),
'properties' => array( 'background' ),
'path' => array( 'color', 'gradients' ),
'prevent_override' => array( 'color', 'defaultGradients' ),
'use_default_presets' => array( 'color', 'defaultGradients' ),
'use_default_names' => false,
'value_key' => 'gradient',
'css_vars' => '--wp--preset--gradient--$slug',
'classes' => array( '.has-$slug-gradient-background' => 'background' ),
'properties' => array( 'background' ),
),
array(
'path' => array( 'color', 'duotone' ),
'override' => true,
'use_default_names' => false,
'value_func' => 'gutenberg_get_duotone_filter_property',
'css_vars' => '--wp--preset--duotone--$slug',
'classes' => array(),
'properties' => array( 'filter' ),
'path' => array( 'color', 'duotone' ),
'prevent_override' => array( 'color', 'defaultDuotone' ),
'use_default_presets' => array( 'color', 'defaultDuotone' ),
'use_default_names' => false,
'value_func' => 'gutenberg_get_duotone_filter_property',
'css_vars' => '--wp--preset--duotone--$slug',
'classes' => array(),
'properties' => array( 'filter' ),
),
array(
'path' => array( 'typography', 'fontSizes' ),
'override' => true,
'use_default_names' => true,
'value_key' => 'size',
'css_vars' => '--wp--preset--font-size--$slug',
'classes' => array( '.has-$slug-font-size' => 'font-size' ),
'properties' => array( 'font-size' ),
'path' => array( 'typography', 'fontSizes' ),
'prevent_override' => false,
'use_default_presets' => true,
'use_default_names' => true,
'value_key' => 'size',
'css_vars' => '--wp--preset--font-size--$slug',
'classes' => array( '.has-$slug-font-size' => 'font-size' ),
'properties' => array( 'font-size' ),
),
array(
'path' => array( 'typography', 'fontFamilies' ),
'override' => true,
'use_default_names' => false,
'value_key' => 'fontFamily',
'css_vars' => '--wp--preset--font-family--$slug',
'classes' => array( '.has-$slug-font-family' => 'font-family' ),
'properties' => array( 'font-family' ),
'path' => array( 'typography', 'fontFamilies' ),
'prevent_override' => false,
'use_default_presets' => true,
'use_default_names' => false,
'value_key' => 'fontFamily',
'css_vars' => '--wp--preset--font-family--$slug',
'classes' => array( '.has-$slug-font-family' => 'font-family' ),
'properties' => array( 'font-family' ),
),
);

Expand Down Expand Up @@ -229,6 +243,7 @@ class WP_Theme_JSON_5_9 {
'custom' => null,
'customDuotone' => null,
'customGradient' => null,
'defaultDuotone' => null,
ajlende marked this conversation as resolved.
Show resolved Hide resolved
'defaultGradients' => null,
'defaultPalette' => null,
'duotone' => null,
Expand Down Expand Up @@ -1021,9 +1036,14 @@ protected static function scope_selector( $scope, $selector ) {
protected static function get_settings_values_by_slug( $settings, $preset_metadata, $origins ) {
$preset_per_origin = _wp_array_get( $settings, $preset_metadata['path'], array() );

$skip_default_presets = ! static::get_metadata_boolean( $settings, $preset_metadata['use_default_presets'], true );

$result = array();
foreach ( $origins as $origin ) {
if ( ! isset( $preset_per_origin[ $origin ] ) ) {
if (
! isset( $preset_per_origin[ $origin ] ) ||
( 'default' === $origin && $skip_default_presets )
) {
continue;
}
foreach ( $preset_per_origin[ $origin ] as $preset ) {
Expand Down Expand Up @@ -1065,9 +1085,14 @@ protected static function get_settings_slugs( $settings, $preset_metadata, $orig

$preset_per_origin = _wp_array_get( $settings, $preset_metadata['path'], array() );

$skip_default_presets = ! static::get_metadata_boolean( $settings, $preset_metadata['use_default_presets'], true );

$result = array();
foreach ( $origins as $origin ) {
if ( ! isset( $preset_per_origin[ $origin ] ) ) {
if (
! isset( $preset_per_origin[ $origin ] ) ||
( 'default' === $origin && $skip_default_presets )
) {
continue;
}
foreach ( $preset_per_origin[ $origin ] as $preset ) {
Expand Down Expand Up @@ -1470,7 +1495,7 @@ public function merge( $incoming ) {

// Replace the presets.
foreach ( static::PRESETS_METADATA as $preset ) {
$override_preset = static::should_override_preset( $this->theme_json, $node['path'], $preset['override'] );
$override_preset = ! static::get_metadata_boolean( $this->theme_json['settings'], $preset['prevent_override'], true );
Copy link
Member

Choose a reason for hiding this comment

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

here's an example of the confusion: $override_preset reads to me like a directive, such as "you should override the preset" while prevent_override sounds more like an allowance, "you may or may not override the preset."

two changes that may not be ideal but which would remove the confusion from this line would be to:

// keep the name so it means the same thing when reading
$prevent_override = static::get_metadata_boolean( … );

…

		( 'theme' === $origin && ! $prevent_override )
// mirror the permissive language of the setting from the JSON file
// but in the positive sense
$allow_override = ! static::get_metatdata_boolean( … );

…

		( 'theme' === $origin && $allow_override )


foreach ( static::VALID_ORIGINS as $origin ) {
$base_path = array_merge( $node['path'], $preset['path'] );
Expand Down Expand Up @@ -1517,6 +1542,7 @@ public function get_svg_filters( $origins ) {
$blocks_metadata = static::get_blocks_metadata();
$setting_nodes = static::get_setting_nodes( $this->theme_json, $blocks_metadata );

$filters = '';
foreach ( $setting_nodes as $metadata ) {
$node = _wp_array_get( $this->theme_json, $metadata['path'], array() );
if ( empty( $node['color']['duotone'] ) ) {
Expand All @@ -1525,9 +1551,11 @@ public function get_svg_filters( $origins ) {

$duotone_presets = $node['color']['duotone'];

$filters = '';
foreach ( $origins as $origin ) {
if ( ! isset( $duotone_presets[ $origin ] ) ) {
if (
! isset( $duotone_presets[ $origin ] ) ||
( 'default' === $origin && false === $node['color']['defaultDuotone'] )
) {
continue;
}
foreach ( $duotone_presets[ $origin ] as $duotone_preset ) {
Expand All @@ -1540,42 +1568,40 @@ public function get_svg_filters( $origins ) {
}

/**
* Returns whether a presets should be overriden or not.
* For metadata values that can either be booleans or paths to booleans, gets the value.
*
* ```php
* $data = array(
* 'color' => array(
* 'defaultPalette' => true
* )
* );
*
* static::get_metadata_boolean( $data, false );
* // => false
*
* static::get_metadata_boolean( $data, array( 'color', 'defaultPalette' ) );
* // => true
* ```
*
* @param array $theme_json The theme.json like structure to inspect.
* @param array $path Path to inspect.
* @param bool|array $override Data to compute whether to override the preset.
* @param array $data The data to inspect.
* @param bool|array $path Boolean or path to a boolean.
* @param bool $default Default value if the referenced path is missing.
* @return boolean
*/
protected static function should_override_preset( $theme_json, $path, $override ) {
if ( is_bool( $override ) ) {
return $override;
protected static function get_metadata_boolean( $data, $path, $default = false ) {
if ( is_bool( $path ) ) {
return $path;
}

// The relationship between whether to override the defaults
// and whether the defaults are enabled is inverse:
//
// - If defaults are enabled => theme presets should not be overriden
// - If defaults are disabled => theme presets should be overriden
//
// For example, a theme sets defaultPalette to false,
// making the default palette hidden from the user.
// In that case, we want all the theme presets to be present,
// so they should override the defaults.
if ( is_array( $override ) ) {
$value = _wp_array_get( $theme_json, array_merge( $path, $override ) );
if ( is_array( $path ) ) {
$value = _wp_array_get( $data, $path );
if ( isset( $value ) ) {
return ! $value;
return $value;
Copy link
Member

Choose a reason for hiding this comment

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

this does seem like a very nice improvement over should_override_preset because the conditional inversion is surprising.

}

// Search the top-level key if none was found for this node.
$value = _wp_array_get( $theme_json, array_merge( array( 'settings' ), $override ) );
if ( isset( $value ) ) {
return ! $value;
}

return true;
}

return $default;
}

/**
Expand Down
3 changes: 3 additions & 0 deletions lib/compat/wordpress-5.9/class-wp-theme-json-resolver-5-9.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,9 @@ public static function get_theme_data( $deprecated = array() ) {
$default_gradients = true;
}
$theme_support_data['settings']['color']['defaultGradients'] = $default_gradients;

// Classic themes without a theme.json don't support global duotone.
$theme_support_data['settings']['color']['defaultDuotone'] = false;
}
$with_theme_supports = new WP_Theme_JSON_Gutenberg( $theme_support_data );
$with_theme_supports->merge( static::$theme );
Expand Down
62 changes: 26 additions & 36 deletions lib/compat/wordpress-5.9/render-svg-filters.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,44 +5,34 @@
* @package gutenberg
*/

/*
* If wp_global_styles_render_svg_filters is defined, it means the plugin
* is running on WordPress 5.9.1, so don't need to render the global styles
* SVG filters as it was already done by WordPress core.
/**
* Render the SVG filters supplied by theme.json.
*
* Note that this doesn't render the per-block user-defined
* filters which are handled by wp_render_duotone_support,
* but it should be rendered before the filtered content
* in the body to satisfy Safari's rendering quirks.
*/
if ( ! function_exists( 'wp_global_styles_render_svg_filters' ) ) {
/**
* Render the SVG filters supplied by theme.json.
*
* Note that this doesn't render the per-block user-defined
* filters which are handled by wp_render_duotone_support,
* but it should be rendered before the filtered content
* in the body to satisfy Safari's rendering quirks.
function gutenberg_global_styles_render_svg_filters() {
/*
* When calling via the in_admin_header action, we only want to render the
* SVGs on block editor pages.
*/
function wp_global_styles_render_svg_filters() {
/*
* When calling via the in_admin_header action, we only want to render the
* SVGs on block editor pages.
*/
if (
is_admin() &&
! get_current_screen()->is_block_editor()
) {
return;
}

$filters = wp_get_global_styles_svg_filters();
if ( ! empty( $filters ) ) {
echo $filters;
}
if (
is_admin() &&
! get_current_screen()->is_block_editor()
) {
return;
}

add_action(
'wp_body_open',
'wp_global_styles_render_svg_filters'
);
add_action(
'in_admin_header',
'wp_global_styles_render_svg_filters'
);
$filters = gutenberg_get_global_styles_svg_filters();
if ( ! empty( $filters ) ) {
echo $filters;
}
}

// Override actions introduced in 5.9.1 if they exist.
remove_action( 'wp_body_open', 'wp_global_styles_render_svg_filters' );
remove_action( 'in_admin_header', 'wp_global_styles_render_svg_filters' );
add_action( 'wp_body_open', 'gutenberg_global_styles_render_svg_filters' );
add_action( 'in_admin_header', 'gutenberg_global_styles_render_svg_filters' );
Loading