WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 3 months ago

#45928 new enhancement

Twenty Nineteen: Add a filter to the $primary_color variable

Reported by: allancole Owned by:
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: Cc:

Description

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

Attachments (5)

45928.patch (624 bytes) - added by allancole 6 months ago.
45928.1.patch (5.0 KB) - added by mattwiebe 5 months ago.
45928.2.patch (9.4 KB) - added by allancole 5 months ago.
45928.3.patch (9.0 KB) - added by laurelfulford 3 months ago.
45928.4.patch (9.1 KB) - added by laurelfulford 3 months ago.

Download all attachments as: .zip

Change History (18)

@allancole
6 months ago

#1 @laurelfulford
6 months ago

  • Component changed from General to Bundled Theme
  • Milestone changed from Awaiting Review to 5.1.1

#2 @LittleBigThing
5 months ago

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

The default value of 199 is used:

@mattwiebe
5 months ago

#3 @mattwiebe
5 months ago

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
5 months ago

#4 @allancole
5 months ago

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.

#5 @laurelfulford
4 months ago

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.

This ticket was mentioned in Slack in #core-themes by lukecarbis. View the logs.


4 months ago

#7 @SergeyBiryukov
4 months ago

  • Summary changed from Twenty Nineteen: Add a filter to the $primay_color variable to Twenty Nineteen: Add a filter to the $primary_color variable

This ticket was mentioned in Slack in #core by lukecarbis. View the logs.


4 months ago

#9 @lukecarbis
4 months ago

  • Milestone changed from 5.1.1 to 5.2

#10 @desrosj
3 months ago

@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.

#11 @laurelfulford
3 months ago

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!

#12 @laurelfulford
3 months ago

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

#13 @desrosj
3 months ago

  • Milestone changed from 5.2 to 5.3

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

Note: See TracTickets for help on using tickets.