feat: colors' css variables and action hook for mobile toggle

[miguelpeixe]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

This PR introduces root CSS variables for the theme's colors and 2 action hooks placed before and after the header mobile menu toggle.

How to test the changes in this Pull Request:

Further details and instructions at Automattic/newspack-plugin#1754

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

[laurelfulford]

Testing this PR with Automattic/newspack-plugin#1754, it's functionally correct -- the button does appear and displays on both desktop and mobile.

I did notice some visual issues (with alignment, spacing, and contrast), but I don't think finalizing that kind of stuff is the purpose of this PR 🙂 Based on that assumption, I've marked this PR as approved.

Please let me know if I have that wrong, and I can do a more thorough visual review!

I'm also happy to spin up an issue for addressing the styles down the road -- just let me know, thanks @miguelpeixe!

[miguelpeixe]

Thank you, @laurelfulford!

I've pushed another change on 31034ad to introduce global CSS variables for the theme's colors. This will help reader activation and other projects to more easily integrate with the theme's UI.

If you can give it a test, it should display the correct applied colors as variables on DevTools:

[laurelfulford]

Thanks @miguelpeixe! This works as described, but I have some feedback about the approach:

I'm probably overthinking this one, but I think it might be easier in the long run to handle any theme-related styles for the reader activation stuff in the theme, rather than the plugin. This is because when we release a new theme, things like the primary and secondary colour settings or header colour settings won't exist as they do now -- they'll either be provided by Global Styles, or come from a block setting, rather than a universal spot we can pass like this. If all of this colour code is in the theme, at least there's nothing to override/untangle when this functionality is used with a different theme. (There's no reason not to use variables though!)

In a similar vein, we might be able to make the theme styles more general -- like make sure anything in the header is picking up the correct colours without having to know the classes -- to help reduce the amount of activation-specific CSS we need!

If we do go ahead with this approach, it might be helpful to pass the contrasting colours as well. These colours are either black or white, depending on what would contrast against one of the custom colours -- so one example would be if the header colour is very light, its contrasting colour would be black, and would be used for the header text; if the header colour is very dark, that colour would be white.

Also, it's an extreme edge case, but the colour styles are only added to a site when something has been customized -- so if you're using the default colours, default ad colours and default mobile CTA colours, these variables won't be available. This won't affect any "real" sites, but it could affect demos/our own sites -- it might be worth adding some defaults in one of the theme's Sass files.

Just let me know if you have questions any of this!

[miguelpeixe]

Thank you for your feedback on the approach!

when we release a new theme, things like the primary and secondary colour settings or header colour settings won't exist as they do now

If the new theme is going to provide variables such as --wp--preset--color--primary from theme.json, I believe this would be the most straightforward standard to implement across our entire codebase. If we adopt this approach, updating the codebase to the new variable name would be much quicker than refactoring to stop using PHP variables, javascript, or any other approach that we are currently using for the same goal. WDYT?

If we do go ahead with this approach, it might be helpful to pass the contrasting colours as well.

Good call! I'll update to include these variants.

Also, it's an extreme edge case, but the colour styles are only added to a site when something has been customized

I didn't realize that! I'll also change the approach to have a separate action hook rendering the variables.

[laurelfulford]

If the new theme is going to provide variables such as --wp--preset--color--primary from theme.json, I believe this would be the most straightforward standard to implement across our entire codebase.

Ah right, I think that should work! 🙂 The only "gotcha" I think just might be the weird few things the theme does with colours (eg. half use the primary colour as the default header colour; others use shades of grey), but approaching this in more of a future looking way would be best in the long term -- we can always tackle those weird things as they come up!

For this, it may not make sense to add the theme contrast colours after all? I'm not exactly sure how those would be replaced dynamically in a future FSE theme... though that can also be something we can tackle when it actually becomes a problem. It's a ways in the future at the moment.

[miguelpeixe]

For this, it may not make sense to add the theme contrast colours after all?

It may be the case to adjust/include the existing color variables as they are required. AFAIK, FSE does all that automatically, so we'll soon not have to worry about exposing theme colors at all. It'll be about choosing which and where to use them.

[miguelpeixe]

For reference, this is how Twenty Twenty-Two rendered body variables looks:

body {
    --wp--preset--color--black: #000000;
    --wp--preset--color--cyan-bluish-gray: #abb8c3;
    --wp--preset--color--white: #ffffff;
    --wp--preset--color--pale-pink: #f78da7;
    --wp--preset--color--vivid-red: #cf2e2e;
    --wp--preset--color--luminous-vivid-orange: #ff6900;
    --wp--preset--color--luminous-vivid-amber: #fcb900;
    --wp--preset--color--light-green-cyan: #7bdcb5;
    --wp--preset--color--vivid-green-cyan: #00d084;
    --wp--preset--color--pale-cyan-blue: #8ed1fc;
    --wp--preset--color--vivid-cyan-blue: #0693e3;
    --wp--preset--color--vivid-purple: #9b51e0;
    --wp--preset--color--foreground: #CA2315;
    --wp--preset--color--background: #FFF6F6;
    --wp--preset--color--primary: #2031b8;
    --wp--preset--color--secondary: #FFFFFF;
    --wp--preset--color--tertiary: #F5F5F5;
    --wp--preset--gradient--vivid-cyan-blue-to-vivid-purple: linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%);
    --wp--preset--gradient--light-green-cyan-to-vivid-green-cyan: linear-gradient(135deg,rgb(122,220,180) 0%,rgb(0,208,130) 100%);
    --wp--preset--gradient--luminous-vivid-amber-to-luminous-vivid-orange: linear-gradient(135deg,rgba(252,185,0,1) 0%,rgba(255,105,0,1) 100%);
    --wp--preset--gradient--luminous-vivid-orange-to-vivid-red: linear-gradient(135deg,rgba(255,105,0,1) 0%,rgb(207,46,46) 100%);
    --wp--preset--gradient--very-light-gray-to-cyan-bluish-gray: linear-gradient(135deg,rgb(238,238,238) 0%,rgb(169,184,195) 100%);
    --wp--preset--gradient--cool-to-warm-spectrum: linear-gradient(135deg,rgb(74,234,220) 0%,rgb(151,120,209) 20%,rgb(207,42,186) 40%,rgb(238,44,130) 60%,rgb(251,105,98) 80%,rgb(254,248,76) 100%);
    --wp--preset--gradient--blush-light-purple: linear-gradient(135deg,rgb(255,206,236) 0%,rgb(152,150,240) 100%);
    --wp--preset--gradient--blush-bordeaux: linear-gradient(135deg,rgb(254,205,165) 0%,rgb(254,45,45) 50%,rgb(107,0,62) 100%);
    --wp--preset--gradient--luminous-dusk: linear-gradient(135deg,rgb(255,203,112) 0%,rgb(199,81,192) 50%,rgb(65,88,208) 100%);
    --wp--preset--gradient--pale-ocean: linear-gradient(135deg,rgb(255,245,203) 0%,rgb(182,227,212) 50%,rgb(51,167,181) 100%);
    --wp--preset--gradient--electric-grass: linear-gradient(135deg,rgb(202,248,128) 0%,rgb(113,206,126) 100%);
    --wp--preset--gradient--midnight: linear-gradient(135deg,rgb(2,3,129) 0%,rgb(40,116,252) 100%);
    --wp--preset--gradient--vertical-secondary-to-tertiary: linear-gradient(to bottom,var(--wp--preset--color--secondary) 0%,var(--wp--preset--color--tertiary) 100%);
    --wp--preset--gradient--vertical-secondary-to-background: linear-gradient(to bottom,var(--wp--preset--color--secondary) 0%,var(--wp--preset--color--background) 100%);
    --wp--preset--gradient--vertical-tertiary-to-background: linear-gradient(to bottom,var(--wp--preset--color--tertiary) 0%,var(--wp--preset--color--background) 100%);
    --wp--preset--gradient--diagonal-primary-to-foreground: linear-gradient(to bottom right,var(--wp--preset--color--primary) 0%,var(--wp--preset--color--foreground) 100%);
    --wp--preset--gradient--diagonal-secondary-to-background: linear-gradient(to bottom right,var(--wp--preset--color--secondary) 50%,var(--wp--preset--color--background) 50%);
    --wp--preset--gradient--diagonal-background-to-secondary: linear-gradient(to bottom right,var(--wp--preset--color--background) 50%,var(--wp--preset--color--secondary) 50%);
    --wp--preset--gradient--diagonal-tertiary-to-background: linear-gradient(to bottom right,var(--wp--preset--color--tertiary) 50%,var(--wp--preset--color--background) 50%);
    --wp--preset--gradient--diagonal-background-to-tertiary: linear-gradient(to bottom right,var(--wp--preset--color--background) 50%,var(--wp--preset--color--tertiary) 50%);
    --wp--preset--duotone--dark-grayscale: url(#wp-duotone-dark-grayscale);
    --wp--preset--duotone--grayscale: url(#wp-duotone-grayscale);
    --wp--preset--duotone--purple-yellow: url(#wp-duotone-purple-yellow);
    --wp--preset--duotone--blue-red: url(#wp-duotone-blue-red);
    --wp--preset--duotone--midnight: url(#wp-duotone-midnight);
    --wp--preset--duotone--magenta-yellow: url(#wp-duotone-magenta-yellow);
    --wp--preset--duotone--purple-green: url(#wp-duotone-purple-green);
    --wp--preset--duotone--blue-orange: url(#wp-duotone-blue-orange);
    --wp--preset--duotone--foreground-and-background: url(#wp-duotone-foreground-and-background);
    --wp--preset--duotone--foreground-and-secondary: url(#wp-duotone-foreground-and-secondary);
    --wp--preset--duotone--foreground-and-tertiary: url(#wp-duotone-foreground-and-tertiary);
    --wp--preset--duotone--primary-and-background: url(#wp-duotone-primary-and-background);
    --wp--preset--duotone--primary-and-secondary: url(#wp-duotone-primary-and-secondary);
    --wp--preset--duotone--primary-and-tertiary: url(#wp-duotone-primary-and-tertiary);
    --wp--preset--font-size--small: 0.875rem;
    --wp--preset--font-size--medium: 1rem;
    --wp--preset--font-size--large: 1.25rem;
    --wp--preset--font-size--x-large: clamp(1.5rem, 3vw, 2rem);
    --wp--preset--font-family--ibm-plex-sans: "IBM Plex Sans", sans-serif;
    --wp--preset--font-family--ibm-plex-mono: "IBM Plex Mono", monospace;
    --wp--custom--spacing--small: max(0.75rem, 4vw);
    --wp--custom--spacing--medium: clamp(1.75rem, 6vw, calc(1.5 * var(--wp--style--block-gap)));
    --wp--custom--spacing--large: clamp(3rem, 8vw, 5rem);
    --wp--custom--spacing--outer: var(--wp--custom--spacing--small, 0.75rem);
    --wp--custom--typography--font-size--huge: clamp(2rem, 4vw, 2.25rem);
    --wp--custom--typography--font-size--gigantic: clamp(2.25rem, 6vw, 2.75rem);
    --wp--custom--typography--font-size--colossal: clamp(2.75rem, 8vw, 3.25rem);
    --wp--custom--typography--line-height--tiny: 1.15;
    --wp--custom--typography--line-height--small: 1.2;
    --wp--custom--typography--line-height--medium: 1.4;
    --wp--custom--typography--line-height--normal: 1.6;
    --wp--custom--line-height--normal: 1.7;
}

[miguelpeixe]

2ea6497 moves the color variables to its own hook to not be conditioned to existing customizations.

[laurelfulford]

Thanks for updating this, @miguelpeixe!

I'm not sure it will fully work as expected -- when I change the theme colours, I still end up with mostly the defaults (in my settings, primary, secondary, mobile CTA and header have been changed, but only the mobile CTA variable is different than the default in the CSS variables):

Also, the header colour isn't always the primary colour -- it's the default for some of the themes when you turn on the solid background, but then it can be changed, but it doesn't look like that will work with the current set up.

Would it work to set a default for these variables -- even in one of the theme's SCSS files -- and then override them using the original custom colours code as needed?

[miguelpeixe]

Thanks, @laurelfulford!

I didn't properly set up the variables used to generate the CSS variables, I was copying a subset of the definition of the variables. For better maintenance, 24e2a3c isolates the definition of colors to be used by the color-patterns.php and by newspack_colors_css_variables().

The CSS variables should now reflect the patterns exactly.

[laurelfulford]

Thanks @miguelpeixe! What's here looks good, and is a nice refactor of all that colour code!

I did run into one issue: each of the child themes also has their own colour patterns PHP under /inc/child-color-patterns.php, and the formatting should be updated to match. Overall I don't think this is an issue that has to be resolved in this PR -- we can refactor all of those in a separate PR -- but it is going to cause one small bug:

Three of the child themes don't use the primary colour as their header background default, they use different shades of grey -- specifically:

• Newspack Joseph uses #111
• Newspack Katharine uses #333
• Newspack Sacha uses #333

(Originally there wasn't an option to control the header colour at all -- it just either used the primary colour for half the themes, and grey for the other half. This is a weird artifact of that).

If the header with these themes is set to have a solid background but not a custom colour, the CSS variables will be incorrect -- they will be for the primary colour and its contrasting text colour, rather than the actual header colour and its contrasting text colour.

In the unlikely chance a site has one of these themes, a solid header and no custom header colour set, the issue could be worked around by setting the header background colour to the same grey as the default.

Based on the above, I still think the child theme code can be addressed in a separate PR to help keep things moving. So I'm marking this one as 'approved' -- if you have any hesitation about moving forward with it as is, though, I'm happy to re-review!

[miguelpeixe]

Thank you, @laurelfulford!

we can refactor all of those in a separate PR -- but it is going to cause one small bug:

Where will this bug be visible? I noticed that it'll generate an inconsistency between the created CSS variable and the actual applied color for the child themes.

Although the CSS variables are created for all colors, they are currently only implemented for reader activation buttons/links using the primary color.

[laurelfulford]

Where will this bug be visible? I noticed that it'll generate an inconsistency between the created CSS variable and the actual applied color for the child themes.

It would only be visible if:

1. You're using Newspack Joseph, Katharine or Sacha
2. You have a solid header background set, but no colour picked so it's using the default dark grey
3. You have a really light primary colour
4. You use the CSS variable generated for the header text colour in the header area, and it needs to contrast against the header background colour.

The issue would be the CSS variable for the header text colour would be dark (to contrast against the light primary colour), but it should actually be light, to contrast against the default header colour. It's a VERY specific set of circumstances, and pretty unlikely to actually crop up in the wild 🙂 If it did, setting the header background to be a dark grey would fix it!

[matticbot]

🎉 This PR is included in version 1.63.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

[matticbot]

🎉 This PR is included in version 1.63.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀