calypsoify: use css vars from calypso-color-schemes

[flootr]

related to #11478

Changes proposed in this Pull Request:

• Add a PostCSS build step to calypsoify's gulp task for processing styles
• Move SASS color variables over to use CSS custom properties (using @automattic/calypso-color-schemes

Is this a new feature or does it add/remove features to an existing part of Jetpack?

p9oQ9f-9o-p2

Testing instructions:

• one part is to make sure each SASS color variable gets replaced with their correct CSS custom property replacement

test locally:

• Start your local Jetpack dev environment and make sure Calypsoify is enabled (see my diff below)

diff --git a/modules/module-extras.php b/modules/module-extras.php
index 989512dcb..a3c976fc6 100644
--- a/modules/module-extras.php
+++ b/modules/module-extras.php
@@ -14,6 +14,7 @@
  */
 $tools = array(
        // Always loaded, but only registered if theme supports it.
+       'calypsoify/class.jetpack-calypsoify.php',
        'custom-post-types/comics.php',
        'custom-post-types/testimonial.php',
        'custom-post-types/nova.php',
@@ -40,7 +41,7 @@ $tools = array(

 // Some features are only available when connected to WordPress.com.
 $connected_tools = array(
-       'calypsoify/class.jetpack-calypsoify.php',
+       /* 'calypsoify/class.jetpack-calypsoify.php', */
        'plugin-search.php',
        'simple-payments/simple-payments.php',
        'woocommerce-analytics/wp-woocommerce-analytics.php',

• Open http://localhost/wp-admin/plugin-install.php?calypsoify=1 and verify that all colors match what's currently used in Calypsoify on master
• Open Gutenberg and make sure all colors are in place
• Make sure the CSS custom props fallback is there so colors are available in IE11

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello flootr! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D28164-code before merging this PR. Thank you!

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: June 4, 2019.
Scheduled code freeze: May 28, 2019

Generated by 🚫 dangerJS against abaa2ca

[simison]

All these double minor versions make me think if we should relax calypso-build semver just a little bit with ~ (instead of strict or ^). cc @ockham

Edit; tho I guess this specific one isn't coming from calypso-build since it has ^, but there were others like autoprefixer.

[sirreal]

The way I read this line is that the ^ versions are working as expected, allowing compatible versions of the same package to dedupe to a single version. In this case ^2.0.0 and ^2.0.1 are both satisfied by 2.0.1. There's no negative impact to longer lines like this.

[matticbot]

flootr, Your synced wpcom patch D28164-code has been updated.

[matticbot]

flootr, Your synced wpcom patch D28164-code has been updated.

[sirreal]

#12277 has landed so you'll likely want to rebase.

[matticbot]

flootr, Your synced wpcom patch D28164-code has been updated.

[matticbot]

flootr, Your synced wpcom patch D28164-code has been updated.

[matticbot]

flootr, Your synced wpcom patch D28164-code has been updated.

[flootr]

Rebased and updated testing instructions.

[sirreal]

Thanks for the work here!

I've left some comments and questions more on the build side of this.

I don't really know anything about the calypsoify module, so I'd like to delegate that part of the review to someone better equipped.

[sirreal]

Clarification/confirmation:

It looks like autoprefixer is replaced by postcss, but postcss uses the config added by this PR which is the config from calypso-build:

https://github.com/Automattic/wp-calypso/blob/fecfa1d72f1e244bbd9844488fb900581f1e274c/packages/calypso-build/postcss.config.js#L1-L9

And that config includes autoprefixer, so we still get it. Is that correct?

Seems there are many other gulp tasks using autoprefixer directly, so we still need to depend on the package directly.

[sirreal]

Do we need this dependency directly, or should we rely on calypso-build to pull it in?

[kraftbj]

If we directly call on it, keep it. If it is only through calypso-build's provided functionality, I would say remove it.

Otherwise, we'd get in situations where we'd update postcss-custom-properties out of sync with calypso-build, just adding more bulk to the build process without reason.

[sixhours]

👋 Hey all! What is the status on this? Fixing the Flows is looking at making some tweaks to the Calypsoify styles to bring them into better alignment with the current Calypso sidebar. The lack of color schemes support has come up as a potential flow to be fixed; folks who have a custom color scheme will see the default colors when viewing Calypsoify, making for a rough transition.

I'm posting a data request to find out what kind of impact we're looking at, but also curious if this is close to being finished. It's possible we could help bring it to the finish line if need be. :)

[flootr]

@sixhours I'm not currently working on this and there seem to be some conflicts now. From what I remember though this was in a working state back then. Feel free to jump in if this is something which could be valuable to you (and ping me in case anything is unclear).

[eeeeevon13]

@sixhours is this still an open issue? clearing out some "needs design review" items for Hack week. If you still need design review can you re-apply the "needs design" tag? Thanks!

[jeherve]

I'll close this PR for now because of the lack of activity on this. We can always reopen in the future if needed, but it will need a rebase, so it may be easier to start a new PR at this point.