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 gutenberg_get_block_editor_settings overriding other hooks #50760

Merged
merged 4 commits into from
May 22, 2023
Merged

Fix gutenberg_get_block_editor_settings overriding other hooks #50760

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
36662d8
Update the hook priority to run first
May 18, 2023
afbb4b6
Add __experimentalFeatures migration code from core
May 18, 2023
43a4bbb
Update comments
May 18, 2023
17c4485
Fix PHPCS
May 19, 2023
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
77 changes: 73 additions & 4 deletions lib/block-editor-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@
*/

/**
* Adds styles and __experimentalFeatures to the block editor settings.
* Replaces core 'styles' and '__experimentalFeatures' block editor settings from
* wordpress-develop/block-editor.php with the Gutenberg versions. Much of the
* code is copied from get_block_editor_settings() in that file.
*
* This hook should run first as it completely replaces the core settings that
* other hooks may need to update.
*
* Note: The settings that are WP version specific should be handled inside the `compat` directory.
*
Expand All @@ -15,7 +20,6 @@
* @return array New block editor settings.
*/
function gutenberg_get_block_editor_settings( $settings ) {
// Recreate global styles.
$global_styles = array();
$presets = array(
array(
Expand Down Expand Up @@ -74,9 +78,74 @@ function gutenberg_get_block_editor_settings( $settings ) {

$settings['styles'] = array_merge( $global_styles, get_block_editor_theme_styles() );

// Copied from get_block_editor_settings() at wordpress-develop/block-editor.php.
$settings['__experimentalFeatures'] = gutenberg_get_global_settings();
// These settings may need to be updated based on data coming from theme.json sources.
Copy link
Member

Choose a reason for hiding this comment

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

There's no similar handler in the core; let's double-check if this is really needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is copied directly from https://github.com/WordPress/wordpress-develop/blob/23b007b126fa73211b7db56a38e78e601e4abbc6/src/wp-includes/block-editor.php#L443.

if ( isset( $settings['__experimentalFeatures']['color']['palette'] ) ) {
$colors_by_origin = $settings['__experimentalFeatures']['color']['palette'];
$settings['colors'] = isset( $colors_by_origin['custom'] ) ?
$colors_by_origin['custom'] : (
isset( $colors_by_origin['theme'] ) ?
$colors_by_origin['theme'] :
$colors_by_origin['default']
);
}
if ( isset( $settings['__experimentalFeatures']['color']['gradients'] ) ) {
$gradients_by_origin = $settings['__experimentalFeatures']['color']['gradients'];
$settings['gradients'] = isset( $gradients_by_origin['custom'] ) ?
$gradients_by_origin['custom'] : (
isset( $gradients_by_origin['theme'] ) ?
$gradients_by_origin['theme'] :
$gradients_by_origin['default']
);
}
if ( isset( $settings['__experimentalFeatures']['typography']['fontSizes'] ) ) {
$font_sizes_by_origin = $settings['__experimentalFeatures']['typography']['fontSizes'];
$settings['fontSizes'] = isset( $font_sizes_by_origin['custom'] ) ?
$font_sizes_by_origin['custom'] : (
isset( $font_sizes_by_origin['theme'] ) ?
$font_sizes_by_origin['theme'] :
$font_sizes_by_origin['default']
);
}
if ( isset( $settings['__experimentalFeatures']['color']['custom'] ) ) {
$settings['disableCustomColors'] = ! $settings['__experimentalFeatures']['color']['custom'];
unset( $settings['__experimentalFeatures']['color']['custom'] );
}
if ( isset( $settings['__experimentalFeatures']['color']['customGradient'] ) ) {
$settings['disableCustomGradients'] = ! $settings['__experimentalFeatures']['color']['customGradient'];
unset( $settings['__experimentalFeatures']['color']['customGradient'] );
}
if ( isset( $settings['__experimentalFeatures']['typography']['customFontSize'] ) ) {
$settings['disableCustomFontSizes'] = ! $settings['__experimentalFeatures']['typography']['customFontSize'];
unset( $settings['__experimentalFeatures']['typography']['customFontSize'] );
}
if ( isset( $settings['__experimentalFeatures']['typography']['lineHeight'] ) ) {
$settings['enableCustomLineHeight'] = $settings['__experimentalFeatures']['typography']['lineHeight'];
unset( $settings['__experimentalFeatures']['typography']['lineHeight'] );
}
if ( isset( $settings['__experimentalFeatures']['spacing']['units'] ) ) {
$settings['enableCustomUnits'] = $settings['__experimentalFeatures']['spacing']['units'];
unset( $settings['__experimentalFeatures']['spacing']['units'] );
}
if ( isset( $settings['__experimentalFeatures']['spacing']['padding'] ) ) {
$settings['enableCustomSpacing'] = $settings['__experimentalFeatures']['spacing']['padding'];
unset( $settings['__experimentalFeatures']['spacing']['padding'] );
}
if ( isset( $settings['__experimentalFeatures']['spacing']['customSpacingSize'] ) ) {
$settings['disableCustomSpacingSizes'] = ! $settings['__experimentalFeatures']['spacing']['customSpacingSize'];
unset( $settings['__experimentalFeatures']['spacing']['customSpacingSize'] );
}

if ( isset( $settings['__experimentalFeatures']['spacing']['spacingSizes'] ) ) {
$spacing_sizes_by_origin = $settings['__experimentalFeatures']['spacing']['spacingSizes'];
$settings['spacingSizes'] = isset( $spacing_sizes_by_origin['custom'] ) ?
$spacing_sizes_by_origin['custom'] : (
isset( $spacing_sizes_by_origin['theme'] ) ?
$spacing_sizes_by_origin['theme'] :
$spacing_sizes_by_origin['default']
);
}

return $settings;
}
add_filter( 'block_editor_settings_all', 'gutenberg_get_block_editor_settings', PHP_INT_MAX );
add_filter( 'block_editor_settings_all', 'gutenberg_get_block_editor_settings', 0 );
Copy link
Contributor Author

@ajlende ajlende May 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe 0 is too soon, but it has to be less than 10 because that's the priority that wp_add_editor_classic_theme_styles is called at (unless the intention is to remove that stylesheet with the plugin active).

Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit worried about this change. It means that every custom filter can override the settings the plugin depends on to be available. This could make debugging issues harder.

Maybe we should use the previous solution of filtering out global styles and recreating them.

I checked, and wp_add_editor_classic_theme_styles adds styles with isGlobalStyles set to false. You also left an inline note for Duotone styles.

// Remove existing global styles provided by core.
$existing_non_global_styles = array();
foreach ( $settings['styles'] as $style ) {
    if (
        ! isset( $style['isGlobalStyles'] ) && ! $style['isGlobalStyles']
    ) {
        $existing_non_global_styles[] = $style;
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It means that every custom filter can override the settings the plugin depends on to be available. This could make debugging issues harder.

This is a fair concern but I'm not sure that it will really be a problem. Maybe we can try it like this for now and if that does become an issue we can go back to the old way of doing it?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not against this change. However, considering that PHP_INT_MAX has been used as a priority for similar filters for years now, it would be hard to guess the side effects of this change.