Skip to content

Fix default duotone preset SVG and style generation #38681

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

Merged
ajlende merged 22 commits into from
Mar 9, 2022
Merged

Fix default duotone preset SVG and style generation #38681

Changes from 1 commit
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
Prev Previous commit
Next Next commit
Try refactor should_override_preset
  • Loading branch information
Alex Lende committed Mar 8, 2022
commit fe1c37a97b5b15312db6ea6fcca9ece1025a0c3c
31 changes: 12 additions & 19 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 @@ -1487,7 +1487,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['prevent_override'] );
$override_preset = ! static::get_metadata_boolean( $this->theme_json['settings'], $preset['prevent_override'], true );
Copy link
Member

@dmsnell dmsnell Feb 22, 2022

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 @@ -1560,32 +1560,25 @@ public function get_svg_filters( $origins ) {
}

/**
* Returns whether a presets should be overridden or not.
* For metadata values that can either be booleans or paths to booleans, gets the value.
*
* @param array $theme_json The theme.json like structure to inspect.
* @param array $path Path to inspect.
* @param bool|array $prevent_override Data to compute whether to prevent override of the preset.
* @param array $data The data to inspect.
* @param bool|array $path Boolean or path to a boolean.
* @return boolean
*/
protected static function should_override_preset( $theme_json, $path, $prevent_override ) {
if ( is_bool( $prevent_override ) ) {
return ! $prevent_override;
protected static function get_metadata_boolean( $data, $path, $default = false ) {
if ( is_bool( $path ) ) {
return $path;
}

if ( is_array( $prevent_override ) ) {
$value = _wp_array_get( $theme_json, array_merge( $path, $prevent_override ) );
if ( is_array( $path ) ) {
$value = _wp_array_get( $data, $path );
if ( isset( $value ) ) {
return ! $value;
return $value;
Copy link
Member

@dmsnell dmsnell Feb 22, 2022

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' ), $prevent_override ) );
if ( isset( $value ) ) {
return ! $value;
}

return false;
}

return $default;
}

/**
Expand Down