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

Do not add to the block-library CSS bundle the colors that come from theme.json #33924

Merged
merged 5 commits into from
Aug 11, 2021
Merged

Do not add to the block-library CSS bundle the colors that come from theme.json #33924

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f161b02
Do not import the CSS that comes from theme.json
oandregal Aug 6, 2021
b5134f4
Use gradient-colors-deprecated
oandregal Aug 6, 2021
90dd246
Remove no longer user mixins
oandregal Aug 11, 2021
465777f
Add deprecation note
oandregal Aug 11, 2021
0d1a0ec
Add changelog for block-library
oandregal Aug 11, 2021
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
4 changes: 4 additions & 0 deletions packages/base-styles/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Breaking Changes

- Remove the background-colors, foreground-colors, and gradient-colors mixins.

## 2.0.0 (2020-07-07)

### Breaking Changes
Expand Down
158 changes: 6 additions & 152 deletions packages/base-styles/_mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,186 +479,44 @@
}
}

@mixin background-colors() {
.has-pale-pink-background-color {
background-color: #f78da7;
}

.has-vivid-red-background-color {
background-color: #cf2e2e;
}

.has-luminous-vivid-orange-background-color {
background-color: #ff6900;
}

.has-luminous-vivid-amber-background-color {
background-color: #fcb900;
}

.has-light-green-cyan-background-color {
background-color: #7bdcb5;
}

.has-vivid-green-cyan-background-color {
background-color: #00d084;
}

.has-pale-cyan-blue-background-color {
background-color: #8ed1fc;
}

.has-vivid-cyan-blue-background-color {
background-color: #0693e3;
}

.has-vivid-purple-background-color {
background-color: #9b51e0;
}

.has-white-background-color {
background-color: #fff;
}

// Deprecated from UI, kept for back-compat.
// Deprecated from UI, kept for back-compat.
@mixin background-colors-deprecated() {
.has-very-light-gray-background-color {
background-color: #eee;
}

.has-cyan-bluish-gray-background-color {
background-color: #abb8c3;
}

// Deprecated from UI, kept for back-compat.
.has-very-dark-gray-background-color {
background-color: #313131;
}

.has-black-background-color {
background-color: #000;
}
}

@mixin foreground-colors() {
.has-pale-pink-color {
color: #f78da7;
}

.has-vivid-red-color {
color: #cf2e2e;
}

.has-luminous-vivid-orange-color {
color: #ff6900;
}

.has-luminous-vivid-amber-color {
color: #fcb900;
}

.has-light-green-cyan-color {
color: #7bdcb5;
}

.has-vivid-green-cyan-color {
color: #00d084;
}

.has-pale-cyan-blue-color {
color: #8ed1fc;
}

.has-vivid-cyan-blue-color {
color: #0693e3;
}

.has-vivid-purple-color {
color: #9b51e0;
}

.has-white-color {
color: #fff;
}

// Deprecated from UI, kept for back-compat.
// Deprecated from UI, kept for back-compat.
@mixin foreground-colors-deprecated() {
.has-very-light-gray-color {
color: #eee;
}

.has-cyan-bluish-gray-color {
color: #abb8c3;
}

// Deprecated from UI, kept for back-compat.
.has-very-dark-gray-color {
color: #313131;
}

.has-black-color {
color: #000;
}
}

@mixin gradient-colors() {
// Our classes uses the same values we set for gradient value attributes, and we can not use spacing because of WP multi site kses rule.

// Deprecated from UI, kept for back-compat.
@mixin gradient-colors-deprecated() {
/* stylelint-disable function-comma-space-after */
.has-vivid-cyan-blue-to-vivid-purple-gradient-background {
background: linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%);
}

.has-vivid-green-cyan-to-vivid-cyan-blue-gradient-background {
background: linear-gradient(135deg,rgba(0,208,132,1) 0%,rgba(6,147,227,1) 100%);
}

.has-light-green-cyan-to-vivid-green-cyan-gradient-background {
background: linear-gradient(135deg,rgb(122,220,180) 0%,rgb(0,208,130) 100%);
}

.has-luminous-vivid-amber-to-luminous-vivid-orange-gradient-background {
background: linear-gradient(135deg,rgba(252,185,0,1) 0%,rgba(255,105,0,1) 100%);
}

.has-luminous-vivid-orange-to-vivid-red-gradient-background {
background: linear-gradient(135deg,rgba(255,105,0,1) 0%,rgb(207,46,46) 100%);
}

.has-very-light-gray-to-cyan-bluish-gray-gradient-background {
background: linear-gradient(135deg,rgb(238,238,238) 0%,rgb(169,184,195) 100%);
}

.has-cool-to-warm-spectrum-gradient-background {
background: 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%);
}

.has-blush-light-purple-gradient-background {
background: linear-gradient(135deg,rgb(255,206,236) 0%,rgb(152,150,240) 100%);
}

.has-blush-bordeaux-gradient-background {
background: linear-gradient(135deg,rgb(254,205,165) 0%,rgb(254,45,45) 50%,rgb(107,0,62) 100%);
}

.has-purple-crush-gradient-background {
background: linear-gradient(135deg,rgb(52,226,228) 0%,rgb(71,33,251) 50%,rgb(171,29,254) 100%);
}

.has-luminous-dusk-gradient-background {
background: linear-gradient(135deg,rgb(255,203,112) 0%,rgb(199,81,192) 50%,rgb(65,88,208) 100%);
}

.has-hazy-dawn-gradient-background {
background: linear-gradient(135deg,rgb(250,172,168) 0%,rgb(218,208,236) 100%);
}

.has-pale-ocean-gradient-background {
background: linear-gradient(135deg,rgb(255,245,203) 0%,rgb(182,227,212) 50%,rgb(51,167,181) 100%);
}

.has-electric-grass-gradient-background {
background: linear-gradient(135deg,rgb(202,248,128) 0%,rgb(113,206,126) 100%);
}

.has-subdued-olive-gradient-background {
background: linear-gradient(135deg,rgb(250,250,225) 0%,rgb(103,166,113) 100%);
}
Expand All @@ -670,9 +528,5 @@
.has-nightshade-gradient-background {
background: linear-gradient(135deg,rgb(51,9,104) 0%,rgb(49,205,207) 100%);
}

.has-midnight-gradient-background {
background: linear-gradient(135deg,rgb(2,3,129) 0%,rgb(40,116,252) 100%);
}
/* stylelint-enable function-comma-space-after */
}
6 changes: 3 additions & 3 deletions packages/block-library/src/common.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@

:root {
// Background colors.
@include background-colors();
@include background-colors-deprecated();

// Foreground colors.
@include foreground-colors();
@include foreground-colors-deprecated();

// Gradients
@include gradient-colors();
@include gradient-colors-deprecated();

}

Expand Down
6 changes: 3 additions & 3 deletions packages/block-library/src/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@

:root .editor-styles-wrapper {
// Background colors.
@include background-colors();
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these mixins still necessary/used somewhere? Should we remove them as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't found any place where they're used. The reason I didn't remove them was that they're part of the base-styles package API so I'm not sure how do we want to go about that: do we want to keep them for third parties?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a development package, so I think you can just remove them and add a breaking change note to the changelog file

Copy link
Contributor

Choose a reason for hiding this comment

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

The question I have is what happens when you instantiate a default block editor instance (just JS no backend), will you get decent defaults for theme.json configs/palettes/styles... How does that work?

Copy link
Member Author

Choose a reason for hiding this comment

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

My understanding is that the block-editor was never concerned with providing the CSS for default colors, the CSS was there by other means.

Historically, that CSS was provided by the block-library and in current trunk is provided by the block-library and the server code for global styles. This PR removes the block-library CSS. It doesn't change anything for the block-editor. Is this your reading as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the unused mixins at ce60eae Added a changelog note at dff2e85

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR removes the block-library CSS. It doesn't change anything for the block-editor. Is this your reading as well?

It seems like you're right, though it does bother me a bit because block-library is something third-party editors can easily load but the theme.json CSS is not.

Take a look at the playground for instance https://wordpress.github.io/gutenberg/?path=/story/playground-block-editor--default

You can see that the block-library stylesheets are loaded there but with this PR they won't be. This won't break styles of the editor immediately because we add the "inline styles" for colors for backward compatibility but the classNames won't have any impact.

So it seems for me, this PR should be fine to land because it doesn't have a conceptual impact but it highlights one that we should tackle at some point. What should we do with the styles generated from theme.json: are these styles that the block editor should render based on its settings? Does the block editor work as it should without them? Should this style generation be baked or at least have a dedicated API/function in the frontend.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I'd need a 👍 to land this then.

Do we want to add a breaking change to the block-library changelog as well? It's not directly an "API" but it seems consumers would be expecting those styles anyway.

I can look at what you mention as a follow-up. I'm around that area with #32702 so can probably dig deeper.

Copy link
Member Author

Choose a reason for hiding this comment

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

Created an issue for it at #34005

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, let's add the changelog mention and ship.

@include background-colors-deprecated();

// Foreground colors.
@include foreground-colors();
@include foreground-colors-deprecated();

// Gradients
@include gradient-colors();
@include gradient-colors-deprecated();
}

// Font sizes.
Expand Down