Twenty Nineteen: Add a filter to the $primary_color variable

[allancole]

There should be a filter added to the $primary_color variable, so that child-themes and plugins can override the built-in default primary color if they choose. Currently, saturation, lightness, and selected text-color can all be filtered. Once the primary color is also filterable, it’ll allow for child themes and plugins to fully customize the color dynamics in the theme.

See: https://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentynineteen/inc/color-patterns.php#L15

[LittleBigThing]

Don't we need to filter the default primary color (hue) for a couple more instances?

The default value of 199 is used:

• to add the color palette to the editor (https://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentynineteen/functions.php#L142)
• in twentynineteen_colors_css_wrap() (https://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentynineteen/functions.php#L273)
• when adding the default customizer setting (https://core.trac.wordpress.org/browser/trunk/src/wp-content/themes/twentynineteen/inc/customizer.php#L64)
• each time as a default to get the theme mod as get_theme_mod( 'primary_color_hue' , 199 ) when there is no custom hue selected yet
• even in js/customize-preview.js

[mattwiebe]

I added 45928.1.patch that will allow for filtering the default hue by a child theme. All hardcoded instances of 199 are replaced with a function call to twentynineteen_get_default_hue().

I also truncated the conditional atop twentynineteen_colors_css_wrap() because it's hooked to wp_head and will never appear anywhere other than the frontend, including within a customizer view.

[allancole]

Just added 45928.2.patch that expands on @mattwiebe’s filters solution for Hue, and adds a few additional filters to fully support HSL including, Saturation, Lightness, Saturation (text) Selection, Lightness (text) Selection and Lightness Hover colors.

[laurelfulford]

Thanks @allancole!

I've tested 45928.2.patch​ , and it seems to work well for filtering colours in child themes.

One thing I notice is if you're moving from the default colour in the Customizer to a custom colour, the current patch will not preview -- it's throwing a JavaScript error on line 29 of customize-preview.js:

Uncaught TypeError: Cannot read property 'split' of undefined

The custom colour does work fine on the front-end, and if you're moving from one custom colour to another in the Customizer, it previews fine.

This looks like it has to do with the conditional changes inside of twentynineteen_colors_css_wrap() -- with it, the inline CSS the JavaScript is looking for doesn't exist when you start with the default colour.

[desrosj]

@laurelfulford @allancole 5.2 beta 1 is in a little under 2 days. Are either of you able to wrap this up by then? If not, let's punt.

[laurelfulford]

Thanks for the reminder on this one, @desrosj!

45928.3.patch​ fixes the issue I noticed in the previous patch, with the JavaScript error in the Customizer.

Could you please take a look and see if things look correct to you? Thanks!

[laurelfulford]

Updated in 45928.4.patch​ to fix a jshint error about _TwentyNineteenPreviewData not being defined.

[desrosj]

I did not have time to test this one. The 5.2 beta deadline is just about here. Going to punt this one.

[desrosj]

@abdullahramzan Are you still able to work on refreshing this one?

[davidbaumwald]

Refreshed Patch

[desrosj]

I have some questions on this.

If we are introducing functions to get the default hue, saturation, lightness, etc., is there a need for the filters being introduced in the functions.php file? $default_hue is being set by calling one of those *_get_default_* functions, but the saturation, lightness, and lightness hover are not.

Any new filter being introduces should also be accompanied by ​a proper docblock.

[desrosj]

This one still needs work. With 5.3 beta 1 today, this one needs to be punted.

[davidbaumwald]

@desrosj Is this still doable for 5.4 with Beta 1 in a few days?

[ianbelanger]

@davidbaumwald I'm going to punt this one out of 5.4 because it still does need more work and I don't think we can have it ready for the Beta release.

[karmatosed]

I am going to recommend we close this for the following reasons:

• Initially it was being worked on as part of the default theme's release.
• Today we would approach colors differently and this isn't causing unless I am mistaken significant user issues today.

Now, if we can find evidence this needs to be resolved for users today let's revisit this. Until then we can consider it something to close as won't fix. Thank you everyone for your collaboration on this so far.

[karmatosed]

I am going to close this as it has had time for feedback, thank you everyone.