Feature/customizer color controll for featured image color overlay

[fabiankaegy]

[fabiankaegy]

as discussed in #85

[kjellr]

This is looking good to me, thanks @fabiankaegy. Looping @allancole in for one last look.

[LittleBigThing]

This custom color only works for single posts/pages.

In my honest opinion, this should also affect featured images in the post list (index or archives), otherwise, those will still have the default blue color filter. Those featured images are affected by the selectors .image-filters-enabled .hentry .post-thumbnail:before and .image-filters-enabled .hentry .post-thumbnail:after.

We should also consider adding a description to the Customizer setting and probably changing the name if it would apply for instances of featured images.

[fabiankaegy]

I agree that this should be added.
I’m also working in anothe pr that uses the new color for all elements using the blue on the page. I.e. buttons, navigation, block quotes

[spencerfinnell]

Since I think the idea is to eventually spread this color around the theme (#154) I think it would make sense to have a filterable function that returns a string of CSS with placeholder values that can be used directly in the PHP and then passed to the JS.

$css = sprintf( twentynineteen_get_primary_color_css(), $site_header_overlay_color );

---

wp_localize_script( 'twentynineteen-customizer', 'twentyNineteenCustomizer', array(
   'primaryColorCss' => twentynineteen_get_primary_color_css()
) );

var css = twentyNineteenCustomizer.primaryColorCss.replace( '%s', newval );

---

There is already plenty of talk about duplicate CSS with regards to using PostCSS or Sass, so introducing another place to maintain two sets of rules will only add further future development burden.

[fabiankaegy]

@spencerfinnell I’m totally with you on this. Didn’t think of doing it this way but I will try to do it that way. Totally makes sense.

[joyously]

Does this Customizer control allow the choice of no color overlay? I'm probably not the only one that chooses my featured images for the image, not to have them covered by a color.

[LittleBigThing]

No color should be black or some kind of gray, otherwise the text above the image won’t be readable.

[joyously]

"No color" to me means transparent or don't apply the effect at all.
Text over images can have text shadow to increase legibility.

[celloexpressions]

We should avoid duplicating the CSS in the JavaScript file. My recommended approach is to do something like this:

`value.bind( function( to ) {

// Update custom color CSS
var style = $( '#twentyseventeen--color' ), // Selector of output style element.
    color = style.data( 'color' ),
    css = style.html();
css = css.split( color ).join( to ); // equivalent to css.replaceAll.
style.html( css )
       .data( 'color', to );

} );

See https://celloexpressions.com/blog/a-strategy-for-custom-colors-in-the-customizer/ for the full explanation of this approach.

[fabiankaegy]

I've done a bit of research on how Gutenberg implemented their Contrast Checker. They are using a small library called tinycolor2. Their implementation is done here.

I don't believe we should or even can import their module because of the react nature of it. But we can use their implementation as a starting point for ours.

The big question though is that we would need a external JS dependency in a core theme. I believe that it would help the Usability greatly but I'm not sure what's the best practice here.

[kjellr]

Thanks for looking into the contrast checker, @fabiankaegy. Initially I was excited about the idea of including one, but I have a couple concerns.

• Adding something this functionally heavy seems a little too much for a theme (at least, for a default theme). This should be something in core that can be hooked into.
• This is minor (and definitely less likely, but) in the case of a theme, users could technically be adding custom background themselves via CSS. So this may actually not be accurate. :/

Hopefully with phase 2 of Gutenberg expanding into more of the customizer, it'll be able to expand its contrast checking functionality to handle this sort of thing.

In the meantime, I think a good option would be to provide a few recommended colors via the Default Color Schemes option that we've used in previous themes:

If others agree on this direction, I can work on defining colors for those alternate options this week.

[fabiankaegy]

Adding something this functionally heavy seems a little too much for a theme (at least, for a default theme). This should be something in core that can be hooked into.

I totally agree. Thats why I wanted to explore it but I thought it would be too much for a default theme.

In the meantime, I think a good option would be to provide a few recommended colors via the Default Color Schemes option that we've used in previous themes:

Would you then only allow for the predefined color options?

[LittleBigThing]

I am not sure what kind of custom color schemes would be interesting to have for the theme. It is a structurally simple theme, different from Twenty Fifteen, for example, that had a clear header and sidebar. We have one main color that works fine with the cool effect for the featured images. Also, the theme is meant to showcase/enable users to create pages/layouts using Gutenberg and do it in a reliable way, with consistency in styling of the editor and the frontend. So, a color scheme should rather be part of the editor?

I like the idea of having a light, and perhaps a dark theme, together with a custom hue picker to pick a main color. Just like Twenty Seventeen has, as proposed by @celloexpressions. It is simple and straightforward.

I think it would be also important to make the main (custom) color available in the editor. This is achievable using Gutenberg's color palette feature. A cool example is published by Rich Tabor. The custom color is added as (part of) a color palette, defaulting to the default color:

add_theme_support(
	'editor-color-palette', array(
		array(
			'name'  => esc_html__( 'Main color', 'twentynineteen' ),
			'slug'  => 'main',
			'color' => esc_attr( get_theme_mod( 'main_color', '#0073aa' ) ),
		)
	)
);

We might do it differently by adding the custom color to the default colors available in Gutenberg. Or by generating some additional, matching colors based on the custom color (lighter, darker, contrasting, ...) to enable the user to apply them to the blocks. This way, the color scheme would be part of the editor, rather than (only) the Customizer.

[joyously]

I have written comments on issues in Gutenberg about how the theme interfaces with the editor in terms of names and styles. I am opposed to promoting theme-specific naming of classes or any inline styles, because the user doesn't know any better. Today he makes several posts with red highlights. Next month, he wants to change his theme, and those red things don't look right with the new theme.
So, it's about theme lock-in, and global versus local styling.
If the theme provides a class name and it's used in 5 posts, what happens to those things when a different theme is chosen?

I prefer a solution where there are standard class names that all themes style (or choose not to). The theme should not be tied in with the editor. The theme should style HTML elements, which get generated by the editor. The theme should not affect what is generated.

[kjellr]

I like the idea of having a light, and perhaps a dark theme, together with a custom hue picker to pick a main color.

By color scheme, I'm really just referring to the single primary color. 🙂 I'm not 100% sure we'll be able to fit a dark color scheme in by RC1 next week, so I think it'd be good to have something like:

[] Blue
[] (some other curated color — maybe a dark theme)
[] Custom

I think it would be also important to make the main (custom) color available in the editor. This is achievable using Gutenberg's color palette feature. A cool example is published by Rich Tabor. The custom color is added as (part of) a color palette, defaulting to the default color:

Whatever we do, we will definitely make these colors available in the editor. That's definitely a requirement.

[fabiankaegy]

Updated the code to have a very crude working implementation. I wasn't able to include the recommendations by @spencerfinnell or @LittleBigThing.

For the moment I just wanted to get the stuff that I have like up here so others can work on refining it so we don't have all the duplicate selectors to maintain.

One thing that is also missing from this is enqueuing the styles in the editor.

[kjellr]

This is looking like a really good base to build on.

This seems to work fairly well, but there are a couple things on the front end that don't seem to inherit the styles:

• Previous/next post pagination links (don't have the correct hover color)
• cover image backgrounds (don't have the new overlay cover)

Also — we'll need another set of functions that applies these colors into our editor styles as well. This should be sorted out before this is ready to merge.

[kjellr]

One thing that is also missing from this is enqueuing the styles in the editor.

Ah, just read that last line in your comment. I figured you already knew about that. 😄

Thanks again, @fabiankaegy — this is looking great so far.