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

Missing ability to remove/override "Core" color palette #36407

Closed
fabiankaegy opened this issue Nov 11, 2021 · 21 comments · Fixed by #36492
Closed

Missing ability to remove/override "Core" color palette #36407

fabiankaegy opened this issue Nov 11, 2021 · 21 comments · Fixed by #36492
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended

Comments

@fabiankaegy
Copy link
Member

Description

In the original tracking issue from Matias (#29568) it was outlined that

Of course, it still needs to be possible to fully disable the default palette since sites that have stricter branding guidelines might prefer to remove it entirely, the same way the custom colors interface can be disabled, but it should not be the default behaviour that a theme palette overwrites the color palette.

Looking at the implementation now that does not seem to be the case.

This is a big problem for custom builds with strict design systems where the litigation to certain brand colors is a very important need that is no longer met without the ability to remove / override that "Core" palette.

I do understand and agree that the default behavior should be having both paletes. But like @mtias mentioned initially then this was proposed themes should be able to override / remove that.

Step-by-step reproduction instructions

There are no steps to reproduce as there is no API to remove the Core palette.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@fabiankaegy fabiankaegy added [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended labels Nov 11, 2021
@fabiankaegy
Copy link
Member Author

I would consider this to be a Bug that should be included in the WordPress 5.9 must-haves as it impacts Agencies and Custom builds very heavily. But of course I am biased that way because I am advocating for the Agency side. :)

@fabiankaegy
Copy link
Member Author

fabiankaegy commented Nov 11, 2021

My proposed solution for it would be introducing a new key to the settings.color object in theme.json for corePalette and coreGradients that could then either be overwritten or set to an empty array to remove it completely.

{
    "version": 1,
    "settings": {
        "color": {
            "corePalette": [],
            "coreGradients": []
        }
    }
}

@fabiankaegy
Copy link
Member Author

To add some more context here. When you set the settings.color.palette to an empty array some of the blocks hide the Color options altogether. Which I think is the right behavior. But not all blocks adhere to that pattern. The Sepperator and the Navigation block for example do still show the color panel in the settings sidebar and just only show the Core palette.

As soon as you add any colors to the settings.color.palette array all the blocks now consistently show the Color panel with both the Core and the Theme palette.

And there does not seem to be a way in which you can remove or override the Core palette.

@ndiego
Copy link
Member

ndiego commented Nov 11, 2021

100% agreed, there needs to be a way to remove the core palette and only display the palette provided by the theme. Perhaps something like the following in theme json.

{
    "settings": {
        "color": {
            "palette": {
                "core": [],
                "theme": [
                    {
                        "slug": "foreground",
                        "color": "#000000",
                        "name": "Foreground"
                    },
                    ...
                ]
            }
        }
    }
}

Or maybe:

{
    "settings": {
        "color": {
            "palette": {
                "coreColors": "false",
                "theme": [
                    {
                        "slug": "foreground",
                        "color": "#000000",
                        "name": "Foreground"
                    },
                    ...
                ]
            }
        }
    }
}

@fabiankaegy
Copy link
Member Author

I tried some other options that may not be as nice to use but would still function but somehow even filtering the block_editor_settings_all hook doesn't remove the Core palette. I'm not sure why it still shows but it must be coming in somewhere after the block_editor_settings_all hook is called.

add_filter(
	'block_editor_settings_all',
	'remove_core_color_palette'
);

/**
 * Remove the "Core" Color Palette in the editor settings
 *
 * @param array $settings block editor settings
 */
function remove_core_color_palette( $settings ) {
	$settings['__experimentalFeatures']['color']['palette']['core'] = [];

	return $settings;
}

@fabiankaegy fabiankaegy changed the title Ability to remove / override "Core" color palette Missing ability to remove/override "Core" color palette Nov 11, 2021
@annezazu
Copy link
Contributor

@oandregal tagging you into this :) Thank you all for kicking this off and flagging this so clearly.

@fabiankaegy
Copy link
Member Author

fabiankaegy commented Nov 11, 2021

I'm curious why the block_editor_settings_all filter no longer gets applied here:

$settings['__experimentalFeatures'] = gutenberg_get_global_settings();

The WP_Theme_JSON_Resolver_Gutenberg::get_merged_data function combines the raw data from the theme.json so there is no way of filtering it anymore.

public static function get_merged_data( $origin = 'user' ) {
$result = new WP_Theme_JSON_Gutenberg();
$result->merge( self::get_core_data() );
$result->merge( self::get_theme_data() );
if ( 'user' === $origin ) {
$result->merge( self::get_user_data() );
}
return $result;
}

From my perspective of course an API in theme.json like mentioned here: #36407 (comment)
#36407 (comment)
would be very cool.

But the ability to filter the final merged theme.json settings before they get passed to the JS side would be a massive win because it would allow us to filter out the core palete. :)

@oandregal
Copy link
Member

👋 oh, thanks for bringing this up, we need to fix it for 5.9. Following Fabian's suggestion, I think we could do:

{
    "version": 2,
    "settings": {
        "color": {
            "corePalette": "true/false",
            "coreGradients": "true/false"
        }
    }
}

This is, we allow themes to opt-out from the core solids & gradients but themes can't override the core colors. As I see it, the value of core colors is to provide a cross-theme palette that doesn't change.

@fabiankaegy (or anyone else) would you be interested in taking this one for yourself?

@fabiankaegy
Copy link
Member Author

@oandregal That sounds great (y) and yes I do also think that it should be a boolean value and that you shouldn't be able to override the values of the core palette. I only mentioned it since it was in Matiases original comment :)

As far as taking it my main goal for it is that it can make the 5.9 deadline and I am not 100% certain I have a full grasp of everything that is needed in order to get this done.

Would it be a simple addition here:

that checks for the two values in the theme provided theme.json to then override the values of the core defaults?

@fabiankaegy
Copy link
Member Author

I started a draft PR but I am not jet confident I know where exactly to place the setting. I've been looking at #36246 as an inspiration but I am having the fundamental issue that this needs to execute after the two JSON files are merged.

@ellenbauer
Copy link

I'm a bit late here but I just want to thank you @fabiankaegy to take the initiative here since having the core color palettes everywhere in the editor is just a super confusion user experience and not needed at all. So I'm very very happy if we can get this updated asap.

@steveangstrom
Copy link

steveangstrom commented Nov 15, 2021

I consider this to be a very serious show stopping bug.
I do not want my clients to see the "core" palette in any case.

Its important that the removal of core palettes is available from the PHP action too. Please don't assume that everyone is jumping into the json theming method on established sites

@paulwilde
Copy link
Contributor

Does this change happen to also remove all the inline styles that automatically get output on every page of the frontend? Might be something to consider if it doesn't disabled those as they just add to the pagesize with no way to disable them (from what I could find).

Screenshot 2021-11-15 at 1 03 43 pm

@fabiankaegy
Copy link
Member Author

@paulwilde No this would not remove these classes. The reason for this is, that the Core palette is only removed from the Editor view so editors are not able to select from the Core palette anymore.

However, the reason why the actual classes don't get removed is that if a block pattern from the core pattern library gets inserted it expects the core palette to be available. Editors could then change the color of that pattern to one of the theme colors.

I can see how this is something that one may want to remove altogether. But that should be a separate issue that I think is important but not a 5.9 blocker.

@steveangstrom
Copy link

Was this tested with PHP themes before being closed?

PHP themed sites will exist for many years and these sites also update their WP / Gutenberg. PHP themed sites are also seeing core . If I try to activate theme.json on any of my legacy or current dev sites it will break the site, so I will continue to create and support PHP themes.

Please ensure that 99% of sites will function as per the docs. Please do not make WP exist on a bleeding edge.

https://make.wordpress.org/core/2020/01/23/controlling-the-block-editor/

@fabiankaegy
Copy link
Member Author

@steveangstrom I would treat your ticket here: #36489 as a follow up to this. The fix that was supplied here does not impact php themes and so it is correct to keep tracking the issue.

@skorasaurus
Copy link
Member

@paulwilde No this would not remove these classes. The reason for this is, that the Core palette is only removed from the Editor view so editors are not able to select from the Core palette anymore.

However, the reason why the actual classes don't get removed is that if a block pattern from the core pattern library gets inserted it expects the core palette to be available. Editors could then change the color of that pattern to one of the theme colors.

I can see how this is something that one may want to remove altogether. But that should be a separate issue that I think is important but not a 5.9 blocker.

The ability to remove those classes had been requested at #24684 but it was declined. :(

@turansadri
Copy link

Things like this are extremely annoying when trying to create a site for clients with brand guidelines. No one wants the content editors to be able to use core colour palette for anything and atm we are not able to remove it at all.

In how to guides theres option for "corePalette" and "coreGradients" showing but those are not included in JSON schema and they also don't work.

https://developer.wordpress.org/block-editor/how-to-guides/themes/theme-json/

Is there any way to remove the core colors currently or are we forced to revert to Gutenberg 11.8.3?

Screenshot 2021-11-17 at 9 47 46

@fabiankaegy
Copy link
Member Author

@turansadri Hey there 👋

The fix to allow you to remove the Core Palette and Gradients has been merged into Gutenberg. That means that the next version of the Gutenberg Plugin will include that fix. It has also been marked as something that should get included in the WordPress 5.9 release.

The documentation in the Handbook is coming directly from the markdown files here in GitHub and therefore they get updated when a feature has been merged. Even when it may not jet be released.

Till the next version of the Gutenberg Plugin is released I'm afraid the only option is to downgrade to 11.8.3 for the time being.

@gingerling
Copy link

Hi, is this about how to remove Default colours using the json? I have edited the theme colours to be as the brand guidelines need for my client (working in 5.9 beta 3) but can't seem to get rid of the default colours below that I don't really want them to use.

Screenshot from 2022-01-18 16-51-14

I'm not clear if this is a separate issue or not from reading this bug

@skorasaurus
Copy link
Member

skorasaurus commented Jan 18, 2022

Hi @gingerling ;

This is the same issue; I'm not able to reproduce your issue using the 5.9 RC2 (released last week) which is newer than beta 3 (released in december?).

I recall there being a lot of changes recently with theme syntax and such; what worked for me in my theme.json to disable the default colors and only provide the theme provided colors is the following; this worked in 5.9 RC 2 both with gutenberg activated and not activated.

{
  "version": 1,
  "settings": {
    "color": {
      "custom": false,
      "customGradient": false,
      "gradients": [],
      "link": false,
      "defaultPalette": false,
      "defaultGradients": false,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended
Projects
None yet
10 participants