Adds core style overrides & bumps jetpack version

[cpapazoglou]

This is a follow up of #18097. Related issue is #18092

Changes proposed in this Pull Request:

• Bumps jetpack version as a cache buster solution for styles loaded from wpcom (discussed here p1608286346170200-slack-CBG1CP4EN)
• Restructures core styles overrides to be compiled by a template
• Conditionally loads the correct styles override file
• Adds class to site-switcher and replaces #adminmenuwrap > #adminmenu > .wp-first-item.menu-top-first > .wp-first-item.menu-top-first with #adminmenuwrap > #adminmenu .site-switcher
• Fixes Site-Switcher and Notes states

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Simple Sites

1. Enable nav-unification experiment (see paYJgx-1af-p2)
2. Apply the patch D54531
3. Change color-scheme from https://wordpress.com/me/account to a core color scheme (see list below)

Atomic Sites

1. Select this branch on jetpack on an Atomic site (can't find it right now though. )
2. Install nav-unification enabler (needs to be proxied)
   nav-unification.zip
3. Change color-scheme from /wp-admin/profile.php

Check the following color-schemes:

• default
• light
• Modern
• Blue
• Coffee
• Ectoplasm
• Midnight
• Ocean
• Sunrise

for the following:
Masterbar:

• Masterbar background color
• Masterbar hover color (Hover on Reader)
• Masterbar active color (My Sites)

Sidebar:

• Site card fadeout
• Site card background color is not changing on hover
• Site card icon color

Proposed changelog entry for your changes:

• Restructures color-scheme styles for nav-unification

[jetpackbot]

	Warnings
⚠️	pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against ad61626

[matticbot]

Caution: This PR has changes that must be merged to WordPress.com
Hello cpapazoglou! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D54531-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

[frontdevde]

First of all kudos for making the effort to switch the plain css over to generating the overrides per scheme via scss. This will significantly reduce the maintenance burden in the future while also making it less error prone.

Select this branch on jetpack on an Atomic site (can’t find it right now though. )

Same here, the branch name doesn't show on the Jetpack beta plugin.

For Simple sites I applied the diff and noticed that it couldn't be cleanly applied which lead to the addition of an .orig file on the diff. Seems be an svn issue. Let's make sure the php file is as expected on the diff and then remove the .orig file.

While testing for Simple sites, the override was loaded in addition to the scheme as expected:

On Simple sites, I noticed that the active state for the Notification bell in the wpcom masterbar isn't yet accounted for. Let's add that:

Calypso	wp-admin

Edit:

Same goes for the hover state on the Notification bell.
See the relevant selectors here https://github.com/Automattic/jetpack/pull/18115/files#diff-bf5e4657a1c54509b24846ff67f30e788493be6b9d7b34cddc6575225ba9d84eR73-R81

[cpapazoglou]

First of all kudos for making the effort to switch the plain css over to generating the overrides per scheme via scss. This will significantly reduce the maintenance burden in the future while also making it less error prone.

Select this branch on jetpack on an Atomic site (can’t find it right now though. )

Same here, the branch name doesn't show on the Jetpack beta plugin.

For Simple sites I applied the diff and noticed that it couldn't be cleanly applied which lead to the addition of an .orig file on the diff. Seems be an svn issue. Let's make sure the php file is as expected on the diff and then remove the .orig file.

While testing for Simple sites, the override was loaded in addition to the scheme as expected:

On Simple sites, I noticed that the active state for the Notification bell in the wpcom masterbar isn't yet accounted for. Let's add that:

Calypso wp-admin

Edit:

Same goes for the hover state on the Notification bell.
See the relevant selectors here https://github.com/Automattic/jetpack/pull/18115/files#diff-bf5e4657a1c54509b24846ff67f30e788493be6b9d7b34cddc6575225ba9d84eR73-R81

Thanks @frontdevde for wrangling this testing! I have addressed your feedback!

[frontdevde]

I just noticed another very subtle issue when retesting with blue. The Site Switcher text has an alternative text color in nav unification that is not yet accounted for:

Calypso	wp-admin

For the color schemes originating in Calypso this was addressed here:
https://github.com/Automattic/jetpack/pull/18115/files#diff-bf5e4657a1c54509b24846ff67f30e788493be6b9d7b34cddc6575225ba9d84eR116-R130

[cpapazoglou]

There are cases when "Browse Sites" is not rendered and thus not the first item

[frontdevde]

Thanks for fixing the regressions as well! 🙌

Starting to look really solid! I've re-tested all color schemes again on Simple and the only issues I found were the following:

• On the Light color scheme the Notification bell icon is white when it's black in Calypso.

• Related, on the Light color scheme the active state of the Notification bell is still off:

Calypso	wp-admin

• In [Coffee, Blue, Ectoplasm, Midnight, Ocean, Sunrise] the Write button looks off on hover:

Calypso	wp-admin

[cpapazoglou]

Fixed both issues and refreshed the patch

[frontdevde]

Looking good in all color schemes on Simple now 👏

Now that the branch is available in Jetpack Beta, I started testing as per your Atomic instructions and I'm noticing that the box-shadow at the bottom of the Masterbar differs:

Calypso & Simple	Atomic

For the Calypso schemes this was addressed here https://github.com/Automattic/jetpack/pull/18115/files#diff-bf5e4657a1c54509b24846ff67f30e788493be6b9d7b34cddc6575225ba9d84eR62-R65

[frontdevde]

Thanks for addressing all the feedback, looking slick now!

Tested as per testing instructions and can confirm it's now looking as expected in both Simple and WoA.

I'd appreciate if you could update the PR description with the latest changes so it provides some context for the next person reviewing. That said, I think we can have Crew take a look at this.

[leogermani]

@zinigor and I reviewed this PR and are approving it.

We found one issue in the WP-admin for atomic sites: when switching between color schemes, the master bar is not changing until a full page refresh.

This needs more testing and we aim to make it easier for testing approving this PR with a few usability issues still present.

slack discussion: p1608616675330200-slack-CBG1CP4EN

[cpapazoglou]

Deployed r218681-wpcom