Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove more colors #23648

Merged
merged 11 commits into from
Jul 3, 2020
Merged

Remove more colors #23648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e52fc74
Retire $radius-round-rectangle
jasmussen Jul 1, 2020
b623f14
Retire dark-gray-900
jasmussen Jul 2, 2020
66a39f2
Retire dark-gray-800
jasmussen Jul 2, 2020
e44cf24
Rename new colors to be on a scale.
jasmussen Jul 2, 2020
03014ba
Rename a variable, retire blue medium 100.
jasmussen Jul 2, 2020
f2a92b0
Retire light-gray-100
jasmussen Jul 2, 2020
6a8bf00
Make all grays neutral.
jasmussen Jul 2, 2020
ca89529
Retire light-gray-200
jasmussen Jul 2, 2020
5da21e9
Retire light-gray-300.
jasmussen Jul 2, 2020
b1f5680
Replace light-gray-400
jasmussen Jul 2, 2020
da1b926
Retire light gray 500, update borders.
jasmussen Jul 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 12 additions & 20 deletions packages/base-styles/_colors.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@
* Colors
*/

// WordPress colors.
$black: #000; // Use only when you truly need pure black. For UI, use $dark-gray-primary.
$dark-gray-primary: #1e1e1e;
$medium-gray-text: #757575; // Meets 4.6:1 text contrast against white.
$light-gray-ui: #949494; // Meets 3:1 UI or large text contrast against white.
$light-gray-secondary: #ccc;
$light-gray-tertiary: #e7e8e9;
// WordPress grays.
$black: #000; // Use only when you truly need pure black. For UI, use $gray-900.
$gray-900: #1e1e1e;
$gray-700: #757575; // Meets 4.6:1 text contrast against white.
$gray-600: #949494; // Meets 3:1 UI or large text contrast against white.
$gray-400: #ccc;
$gray-200: #ddd; // Used for most borders.
$gray-100: #f0f0f0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great too see less colors. Maybe later we could try to name these variables differently to clarify their purpose.

for example instead of $gray-200 it could $border-color etc...?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would vote for keeping the names generic. A certain color may be recommended for borders, but will likely be used in other contexts as well. Reading css can get a little confusing with explicit var names.

I like having the descriptions / recommendations in this file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This for me reduces the value of the variables. Why $black instead of black?

I understand that variables can be used for different things and in that case for me we need two variables that may use the same color. That way when we change a color, we know the impact while right now if you change say the value of $gray-100, you have no idea what you're doing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in that case for me we need two variables that may use the same color

I didn't think about that - good point.

Copy link
Contributor Author

@jasmussen jasmussen Jul 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why $black instead of black?

Because you might want a $black that's actually #000009 to have just a smidgeon of blue in it. This is less of a case now that we're actually moving towards pure neutral grays, but the old grays had a slight cold tinge. I'm happy to remove black and white variables in favor of the CSS colors instead.

More importantly, the fact that it's registered in the color variables sheet, hopefully, trains a developer to look there for guidance before using any colors at all. In this case, you should not use pure black unless you know what you're doing. The primary use case is to create drop shadows, in which case it's appropriate. But given the gray-900 is so very close to black, people might look at the block UI and think "oh it has a black border, I'll just use black". They might still, but it's harder to search the codebase for black than it is to search for $black, and correct any errors.

That way when we change a color, we know the impact while right now if you change say the value of $gray-100, you have no idea what you're doing.

I hear this, and I think this topic is actually the most important to work out in this PR. I intentionally moved from $dark-gray-primary to $gray-900, because I felt the former was fairly confusingly named. In fact I created this gray scale to figure out where to map the grays, on a scale from 0 to 10, 0 being white, 10 being black:

Colors

The swatches that are pushed downwards do not exist yet as variables. And maybe we don't want them to? The intent more than anything is fewer colors, and I definitely hear your point, because you really should have some guidance on which colors to use for which UI elements — this is for block UI after all.

Rewinding a bit, here's before:

$black: #000;				// Use only when you truly need pure black. For UI, use $dark-gray-primary.
$dark-gray-primary: #1e1e1e;
$medium-gray-text: #757575;		// Meets 4.6:1 text contrast against white.
$light-gray-ui: #949494;		// Meets 3:1 UI or large text contrast against white.
$light-gray-secondary: #ccc;
$light-gray-tertiary: #e7e8e9;
$white: #fff;

Here's after:

$black: #000;			// Use only when you truly need pure black. For UI, use $gray-900.
$gray-900: #1e1e1e;
$gray-700: #757575;		// Meets 4.6:1 text contrast against white.
$gray-600: #949494;		// Meets 3:1 UI or large text contrast against white.
$gray-400: #ccc;
$gray-200: #ddd;		// Used for most borders.
$gray-100: #f0f0f0;
$white: #fff;

I like the systematizing of these. Could we do something like this?

$black-pure: #000;
$gray-900-block-ui: #1e1e1e;
$gray-700-aa-contrast: #757575;
$gray-600-aa-ui-contrast: #949494;
$gray-400: #ccc;
$gray-200-borders: #ddd;
$gray-100-light-borders: #f0f0f0;
$white-pure: #fff;

Or maybe that's too verbose, we could also ditch the white and black as Riad implies, and reduce to this:

$gray-900-block-ui: #1e1e1e;
$gray-700-aa: #757575;
$gray-600-aa-ui: #949494;
$gray-400: #ccc;
$gray-200-borders: #ddd;
$gray-100-borders: #f0f0f0;

Let me know your thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these latest variables are better, that said when you name a constant when you're coding, you don't name it something like: MAX_COLUMNS_8 you name it just MAX_COLUMNS (the value is not on the name).

I do see where you're coming from, defining a limited set of colors to be used in the UI, so I think the solution might be to use "two sets of variables". One that is internal to the _variables file and should never be used elsewhere, this:

$black: #000;			// Use only when you truly need pure black. For UI, use $gray-900.
$gray-900: #1e1e1e;
$gray-700: #757575;		// Meets 4.6:1 text contrast against white.
$gray-600: #949494;		// Meets 3:1 UI or large text contrast against white.
$gray-400: #ccc;
$gray-200: #ddd;		// Used for most borders.
$gray-100: #f0f0f0;
$white: #fff;

And use this set to define the actual variables used in the different modules

$block-ui-border-color: $gray-900;
$dark-border-color: $gray-200;
$light-border-color: $gray-100;

I don't like the dark/light too much because it's not really clear when to use one or the other but it's a bit better. I also intentionally removed the others you defined above to "force us" to add more variables to define where they are being used as right now it seems we don't really know, it's kind of random.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I'm happy to do revert to having just the verbosely named colors, instead of the scale-numbered ones. If need be, I can write the scale value in an HTML comment on the side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feedback is non-blocking as this PR is better than master :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, great feedback all around.

There are still 12 more colors that I need to retire and replace. But those won't make it for 5.5. But this PR might. So for the time being, I'm going to keep the numbered grays, and I will follow up with two more PRs (both which won't make 5.5). The first followup will retire the remaining grays, the 2nd followup will open a discussion as to how to best name these colors, for example per Riad's thoughts in #23648 (comment). Sound good?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another possibility is to have define some colours and then create aliases. So $border-color = $gray-200.

$white: #fff;
$blue-medium-focus-dark: $white; // Focus color when the theme is dark.

// Opacities. For use when transparency is needed.
$dark-gray-placeholder: rgba($dark-gray-primary, 0.62);
$medium-gray-placeholder: rgba($dark-gray-primary, 0.55);
// Opacities & additional colors.
$dark-theme-focus: $white; // Focus color when the theme is dark.
$dark-gray-placeholder: rgba($gray-900, 0.62);
$medium-gray-placeholder: rgba($gray-900, 0.55);
$light-gray-placeholder: rgba($white, 0.65);

// Alert colors.
Expand All @@ -28,8 +29,6 @@ $alert-green: #4ab866;
* Please avoid using these.
*/

$dark-gray-900: #191e23;
$dark-gray-800: #23282d;
$dark-gray-700: #32373c;
$dark-gray-600: #40464d;
$dark-gray-500: #555d66;
Expand All @@ -43,10 +42,3 @@ $light-gray-900: #a2aab2;
$light-gray-800: #b5bcc2;
$light-gray-700: #ccd0d4;
$light-gray-600: #d7dade;
$light-gray-500: #e2e4e7;
$light-gray-400: #e8eaeb;
$light-gray-300: #edeff0;
$light-gray-200: #f3f4f5;
$light-gray-100: #f8f9f9;

$blue-medium-100: #e5f5fa;
6 changes: 3 additions & 3 deletions packages/base-styles/_mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
box-shadow: 0 0 0 transparent;
transition: box-shadow 0.1s linear;
border-radius: $radius-block-ui;
border: $border-width solid $medium-gray-text;
border: $border-width solid $gray-700;
@include reduce-motion("transition");
}

Expand Down Expand Up @@ -289,7 +289,7 @@

@mixin checkbox-control {
@include input-control;
border: $border-width solid $dark-gray-primary;
border: $border-width solid $gray-900;
margin-right: $grid-unit-15;
transition: none;
border-radius: $radius-block-ui;
Expand Down Expand Up @@ -350,7 +350,7 @@

@mixin radio-control {
@include input-control;
border: $border-width solid $dark-gray-primary;
border: $border-width solid $gray-900;
margin-right: $grid-unit-15;
transition: none;
border-radius: $radius-round;
Expand Down
4 changes: 1 addition & 3 deletions packages/base-styles/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ $mobile-color-swatch: 48px;
*/

$shadow-popover: 0 2px 6px rgba($black, 0.05);
$shadow-modal: 0 3px 30px rgba($dark-gray-900, 0.2);
$shadow-modal: 0 3px 30px rgba($black, 0.2);


/**
Expand Down Expand Up @@ -95,11 +95,9 @@ $block-selected-padding: 0;
$block-selected-child-margin: 5px;
$block-selected-to-content: $block-edge-to-content - $block-selected-margin - $block-selected-border-width;


/**
* Border radii.
*/

$radius-round-rectangle: 4px;
$radius-round: 50%;
$radius-block-ui: 2px;
2 changes: 1 addition & 1 deletion packages/block-directory/src/components/downloadable-block-icon/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
width: $button-size;
height: $button-size;
font-size: $button-size;
background-color: $light-gray-300;
background-color: $gray-200;
}
}
6 changes: 3 additions & 3 deletions packages/block-directory/src/components/downloadable-block-list-item/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
justify-content: center;
background: transparent;
word-break: break-word;
border-top: $border-width solid $light-gray-500;
border-bottom: $border-width solid $light-gray-500;
border-top: $border-width solid $gray-200;
border-bottom: $border-width solid $gray-200;
transition: all 0.05s ease-in-out;
@include reduce-motion("transition");
position: relative;
Expand Down Expand Up @@ -53,7 +53,7 @@
display: flex;
flex-direction: column;
padding: $grid-unit-20;
background-color: $light-gray-200;
background-color: $gray-100;
}

.block-directory-downloadable-block-list-item__content {
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-breadcrumb/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

.block-editor-block-breadcrumb__button.components-button,
.block-editor-block-breadcrumb__current {
color: $dark-gray-primary;
color: $gray-900;
padding: 0 $grid-unit-10;
font-size: inherit;
}
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-compare/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
.block-editor-block-compare__html {
font-family: $editor-html-font;
font-size: 12px;
color: $dark-gray-800;
color: $gray-900;
border-bottom: 1px solid #ddd;
padding-bottom: 15px;
line-height: 1.7;
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/components/block-draggable/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
}

.block-editor-block-draggable-chip {
background-color: $dark-gray-primary;
background-color: $gray-900;
border-radius: $radius-block-ui;
border: $border-width solid $dark-gray-primary;
border: $border-width solid $gray-900;
box-shadow: 0 4px 6px rgba($black, 0.3);
color: $white;
cursor: grabbing;
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-inspector/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
}
.components-panel__body {
border: none;
border-top: $border-width solid $light-gray-500;
border-top: $border-width solid $gray-100;
}

.block-editor-block-card {
Expand Down
27 changes: 8 additions & 19 deletions packages/block-editor/src/components/block-list/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,4 @@
.block-editor-block-list__layout .block-editor-block-list__block { // Needs specificity to override inherited styles.
// While block is being dragged, dim the slot dragged from, and hide some UI.
&.is-dragging {
> * {
background: $light-gray-100;
}

> * > * {
visibility: hidden;
}
}

.reusable-block-edit-panel * {
z-index: z-index(".block-editor-block-list__block .reusable-block-edit-panel *");
}
Expand Down Expand Up @@ -80,7 +69,7 @@

// Show a light color for dark themes.
.is-dark-theme & {
box-shadow: 0 0 0 $border-width-focus $blue-medium-focus-dark;
box-shadow: 0 0 0 $border-width-focus $dark-theme-focus;
}
}
}
Expand Down Expand Up @@ -163,7 +152,7 @@

// Show a lighter color for dark themes.
.is-dark-theme & {
box-shadow: 0 0 0 $border-width-focus $blue-medium-focus-dark;
box-shadow: 0 0 0 $border-width-focus $dark-theme-focus;
}
}

Expand All @@ -176,7 +165,7 @@
}

.is-navigate-mode & .block-editor-block-list__block.is-hovered:not(.is-selected)::after {
box-shadow: 0 0 0 1px $light-gray-ui;
box-shadow: 0 0 0 1px $gray-600;
}

.is-block-moving-mode & .block-editor-block-list__block.has-child-selected {
Expand All @@ -199,7 +188,7 @@
left: 0;
top: -$default-block-margin / 2;
border-radius: $radius-block-ui;
border-top: 4px solid $light-gray-secondary;
border-top: 4px solid $gray-400;
}

&::after {
Expand Down Expand Up @@ -424,7 +413,7 @@
.block-editor-block-list__block-popover-inserter {
.block-editor-inserter__toggle.components-button.has-icon {
// Basic look
background: $dark-gray-primary;
background: $gray-900;
border-radius: $radius-block-ui;
color: $white;
padding: 0;
Expand Down Expand Up @@ -533,14 +522,14 @@

.block-editor-block-contextual-toolbar {
// Block UI appearance.
border: $border-width solid $dark-gray-primary;
border: $border-width solid $gray-900;
border-radius: $radius-block-ui;
background-color: $white;

.block-editor-block-toolbar .components-toolbar-group,
.block-editor-block-toolbar .components-toolbar,
.block-editor-block-toolbar__mover-switcher-container {
border-right-color: $dark-gray-primary;
border-right-color: $gray-900;
}

.block-editor-block-toolbar__mover-switcher-container {
Expand Down Expand Up @@ -643,7 +632,7 @@
top: -$border-width;

// Block UI appearance.
box-shadow: 0 0 0 $border-width $dark-gray-primary;
box-shadow: 0 0 0 $border-width $gray-900;
border-radius: $radius-block-ui - $border-width; // Border is outset, so so subtract the width to achieve correct radius.
background-color: $white;

Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/components/block-mobile-toolbar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
.block-editor-block-mobile-toolbar {
display: flex;
flex-direction: row;
border-right: $border-width solid $light-gray-500;
border-right: $border-width solid $gray-200;

.block-editor-block-mover-button {
width: $button-size;
height: $button-size;
border-radius: $radius-round-rectangle;
border-radius: $radius-block-ui;
padding: 3px;
margin: 0;
justify-content: center;
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/components/block-navigation/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ $tree-item-height: 36px;

.block-editor-block-navigation__label {
margin: 0 0 $grid-unit-15;
color: $medium-gray-text;
color: $gray-700;
text-transform: uppercase;
font-size: 11px;
font-weight: 500;
Expand Down Expand Up @@ -157,7 +157,7 @@ $tree-item-height: 36px;
}

.block-editor-inserter__toggle {
background: $dark-gray-primary;
background: $gray-900;
color: $white;
height: $grid-unit-30;
margin: 6px 6px 6px 1px;
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-parent-selector/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
.block-editor-block-parent-selector__button {
width: $grid-unit-60;
height: $grid-unit-60;
border: $border-width solid $dark-gray-primary;
border: $border-width solid $gray-900;
border-radius: $radius-block-ui;
}
}
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-styles/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
}

.block-editor-block-styles__item-preview {
border: 2px solid $dark-gray-primary;
border: 2px solid $gray-900;
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/block-editor/src/components/block-switcher/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
// Since it's not clickable, though, don't show a hover state.
&,
.block-editor-block-icon.has-colors {
color: $dark-gray-primary !important;
color: $gray-900 !important;
}
}

Expand Down Expand Up @@ -98,7 +98,7 @@
}

.components-panel__body + .components-panel__body {
border-top: $border-width solid $light-gray-500;
border-top: $border-width solid $gray-100;
}
}

Expand All @@ -125,7 +125,7 @@

.components-popover__content {
box-shadow: none;
border: $border-width solid $dark-gray-primary;
border: $border-width solid $gray-900;
background: $white;
border-radius: $radius-block-ui;
}
Expand All @@ -139,7 +139,7 @@

.block-editor-block-switcher__preview-title {
margin-bottom: $grid-unit-15;
color: $medium-gray-text;
color: $gray-700;
text-transform: uppercase;
font-size: 11px;
font-weight: 500;
Expand Down
2 changes: 1 addition & 1 deletion packages/block-editor/src/components/block-toolbar/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
border: 0;

// Add a border after item groups to show as separator in the block toolbar.
border-right: $border-width solid $light-gray-500;
border-right: $border-width solid $gray-200;
}

> :last-child,
Expand Down
8 changes: 4 additions & 4 deletions packages/block-editor/src/components/button-block-appender/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
padding: $grid-unit-10;
width: 100%;
height: auto;
color: $dark-gray-primary;
box-shadow: inset 0 0 0 $border-width $dark-gray-primary;
color: $gray-900;
box-shadow: inset 0 0 0 $border-width $gray-900;

&:hover {
box-shadow: inset 0 0 0 $border-width var(--wp-admin-theme-color);
Expand All @@ -25,7 +25,7 @@
&.block-list-appender__toggle {
display: flex;
flex-direction: row;
color: $dark-gray-primary;
color: $gray-900;
box-shadow: none;
height: 24px;
padding: 0;
Expand All @@ -37,7 +37,7 @@

& > svg {
width: 24px;
background-color: $dark-gray-primary;
background-color: $gray-900;
color: $white;
border-radius: $radius-block-ui;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/block-editor/src/components/inserter-list-item/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

&.is-active {
color: $white;
background: $dark-gray-primary;
background: $gray-900;

// Windows High Contrast mode will show this outline, but not the box-shadow.
outline: 2px solid transparent;
Expand All @@ -49,7 +49,7 @@
.block-editor-block-types-list__item-icon {
padding: 12px 20px;
border-radius: $radius-block-ui;
color: $dark-gray-primary;
color: $gray-900;
transition: all 0.05s ease-in-out;
@include reduce-motion("transition");

Expand Down
Loading