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

Fix: theme colors cannot override defaults #36811

Merged
merged 13 commits into from
Nov 25, 2021
Merged

Fix: theme colors cannot override defaults #36811

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
99d7bdf
Clarify comment
oandregal Nov 23, 2021
5a6387d
Use presets_metadata as data source
oandregal Nov 23, 2021
397c8f3
Add test with desired behavior
oandregal Nov 23, 2021
59562f2
Implement the feature so the test passes
oandregal Nov 23, 2021
242a493
Better name
oandregal Nov 24, 2021
1657ce6
Limit the removal of presets to the palette and gradients
oandregal Nov 24, 2021
8a15666
Clarify naming
oandregal Nov 24, 2021
b6dc91c
Update comment
oandregal Nov 24, 2021
0c74665
Iterate only over the nodes from incoming data
oandregal Nov 24, 2021
cd9f308
Improve implementation to be more performant
oandregal Nov 24, 2021
464cebd
Clarify comment
oandregal Nov 24, 2021
e547a35
Clarify names
oandregal Nov 24, 2021
1a5bbb6
Fix lint issues
oandregal Nov 24, 2021
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
121 changes: 112 additions & 9 deletions lib/class-wp-theme-json-gutenberg.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ class WP_Theme_JSON_Gutenberg {
*
* - 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
Expand Down Expand Up @@ -174,6 +177,7 @@ class WP_Theme_JSON_Gutenberg {
const PRESETS_METADATA = array(
array(
'path' => array( 'color', 'palette' ),
'override' => false,
'value_key' => 'color',
'css_vars' => '--wp--preset--color--$slug',
'classes' => array(
Expand All @@ -185,27 +189,31 @@ class WP_Theme_JSON_Gutenberg {
),
array(
'path' => array( 'color', 'gradients' ),
'override' => 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,
'value_func' => 'gutenberg_render_duotone_filter_preset',
'css_vars' => '--wp--preset--duotone--$slug',
'classes' => array(),
'properties' => array( 'filter' ),
),
array(
'path' => array( 'typography', 'fontSizes' ),
'override' => 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,
'value_key' => 'fontFamily',
'css_vars' => '--wp--preset--font-family--$slug',
'classes' => array( '.has-$slug-font-family' => 'font-family' ),
Expand Down Expand Up @@ -1354,32 +1362,127 @@ public function merge( $incoming ) {
$incoming_data = $incoming->get_raw_data();
$this->theme_json = array_replace_recursive( $this->theme_json, $incoming_data );

// The array_replace_recursive algorithm merges at the leaf level.
// For leaf values that are arrays it will use the numeric indexes for replacement.
// In those cases, we want to replace the existing with the incoming value, if it exists.
/*
* The array_replace_recursive algorithm merges at the leaf level,
* but we don't want leaf arrays to be merged, so we overwrite it.
*
* For leaf values that are sequential arrays it will use the numeric indexes for replacement.
* We rather replace the existing with the incoming value, if it exists.
* This is the case of spacing.units.
*
* For leaf values that are associative arrays it will merge them as expected.
* This is also not the behavior we want for the current associative arrays (presets).
* We rather replace the existing with the incoming value, if it exists.
* This happens, for example, when we merge data from theme.json upon existing
* theme supports or when we merge anything coming from the same source twice.
* This is the case of color.palette, color.gradients, color.duotone,
* typography.fontSizes, or typography.fontFamilies.
*
*/
$to_replace = array();
$to_replace[] = array( 'spacing', 'units' );
foreach ( self::VALID_ORIGINS as $origin ) {
$to_replace[] = array( 'color', 'duotone', $origin );
$to_replace[] = array( 'color', 'palette', $origin );
$to_replace[] = array( 'color', 'gradients', $origin );
$to_replace[] = array( 'typography', 'fontSizes', $origin );
$to_replace[] = array( 'typography', 'fontFamilies', $origin );
foreach ( self::PRESETS_METADATA as $metadata ) {
foreach ( self::VALID_ORIGINS as $origin ) {
$to_replace[] = array_merge( $metadata['path'], array( $origin ) );
}
}

/**
* We need to remove the incoming presets that have the same slug
* than an existing default preset. This is the list of slugs present
* in the global config we'll use to filter the incoming slugs.
*/
$slugs_global = self::get_slugs_not_to_override( $this->theme_json );

$nodes = self::get_setting_nodes( $this->theme_json );
foreach ( $nodes as $metadata ) {
foreach ( $to_replace as $property_path ) {
$path = array_merge( $metadata['path'], $property_path );
$node = _wp_array_get( $incoming_data, $path, null );
if ( isset( $node ) ) {
if ( 'theme' === $path[ count( $path ) - 1 ] ) {
oandregal marked this conversation as resolved.
Show resolved Hide resolved
$slugs_for_context = self::get_slugs_not_to_override( $this->theme_json, $metadata['path'] );
$slugs_to_filter = array_merge_recursive( $slugs_global, $slugs_for_context );
$node = self::filter_slugs( $node, $property_path, $slugs_to_filter );
}
_wp_array_set( $this->theme_json, $path, $node );
}
}
}

}

/**
* Returns the slugs for all the presets that cannot be overriden
* in the given path. It returns an associative array
* whose keys are the preset paths and the leafs is the list of slugs.
*
* For example:
*
* array(
* 'color' => array(
* 'palette' => array( 'slug-1', 'slug-2' ),
* 'gradients' => array( 'slug-3', 'slug-4' ),
* ),
* )
*
* @param array $data A theme.json like structure to inspect.
* @param array $node_path The path to inspect. It's 'settings' by default.
*
* @return array An associative array containing the slugs for the given path.
*/
private static function get_slugs_not_to_override( $data, $node_path = array( 'settings' ) ) {
$slugs = array();
foreach ( self::PRESETS_METADATA as $metadata ) {
if ( $metadata['override'] ) {
continue;
}

$slugs_for_preset = array();
$path = array_merge( $node_path, $metadata['path'], array( 'default' ) );
$preset = _wp_array_get( $data, $path, null );
if ( isset( $preset ) ) {
$slugs_for_preset = array_map(
function( $value ) {
return isset( $value['slug'] ) ? $value['slug'] : null;
},
$preset
);
}
_wp_array_set( $slugs, $metadata['path'], $slugs_for_preset );
}

return $slugs;
}

/**
* Removes the preset values whose slug is equal to any of default presets.
*
* @param array $node The node with the presets to validate.
* @param array $path The path to the preset to inspect.
* @param array $slugs The slugs to remove.
*
* @return array The new node
*/
private static function filter_slugs( $node, $path, $slugs ) {
// The path comes with the origin as the last key (default, theme, custom),
// so we remove it first.
array_pop( $path );
$slugs_for_preset = _wp_array_get( $slugs, $path, array() );
if ( empty( $slugs_for_preset ) ) {
return $node;
}

$new_node = array();
foreach ( $node as $value ) {
if ( isset( $value['slug'] ) && ! in_array( $value['slug'], $slugs_for_preset, true ) ) {
$new_node[] = $value;
}
}

return $new_node;
}

/**
* Processes a setting node and returns the same node
* without the insecure settings.
Expand Down
138 changes: 138 additions & 0 deletions phpunit/class-wp-theme-json-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1081,6 +1081,144 @@ public function test_merge_incoming_data_null_presets() {
$this->assertEqualSetsWithIndex( $expected, $actual );
}

public function test_merge_incoming_data_removes_theme_presets_with_slugs_as_default_presets() {
$defaults = new WP_Theme_JSON_Gutenberg(
array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'palette' => array(
array(
'slug' => 'red',
'color' => 'red',
'name' => 'Red',
),
array(
'slug' => 'green',
'color' => 'green',
'name' => 'Green',
),
),
),
'blocks' => array(
'core/paragraph' => array(
'color' => array(
'palette' => array(
array(
'slug' => 'blue',
'color' => 'blue',
'name' => 'Blue',
),
),
),
),
),
),
),
'default'
);
$theme = new WP_Theme_JSON_Gutenberg(
array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'palette' => array(
array(
'slug' => 'pink',
'color' => 'pink',
'name' => 'Pink',
),
array(
'slug' => 'green',
'color' => 'green',
'name' => 'Greenish',
),
),
),
'blocks' => array(
'core/paragraph' => array(
'color' => array(
'palette' => array(
array(
'slug' => 'blue',
'color' => 'blue',
'name' => 'Bluish',
),
array(
'slug' => 'yellow',
'color' => 'yellow',
'name' => 'Yellow',
),
array(
'slug' => 'green',
'color' => 'green',
'name' => 'Block Green',
),
),
),
),
),
),
)
);

$expected = array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'settings' => array(
'color' => array(
'palette' => array(
'default' => array(
array(
'slug' => 'red',
'color' => 'red',
'name' => 'Red',
),
array(
'slug' => 'green',
'color' => 'green',
'name' => 'Green',
),
),
'theme' => array(
array(
'slug' => 'pink',
'color' => 'pink',
'name' => 'Pink',
),
),
),
),
'blocks' => array(
'core/paragraph' => array(
'color' => array(
'palette' => array(
'default' => array(
array(
'slug' => 'blue',
'color' => 'blue',
'name' => 'Blue',
),
),
'theme' => array(
array(
'slug' => 'yellow',
'color' => 'yellow',
'name' => 'Yellow',
),
),
),
),
),
),
),
);

$defaults->merge( $theme );
$actual = $defaults->get_raw_data();

$this->assertEqualSetsWithIndex( $expected, $actual );
}

function test_remove_insecure_properties_removes_unsafe_styles() {
$actual = WP_Theme_JSON_Gutenberg::remove_insecure_properties(
array(
Expand Down