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.

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

Try global styles at blocks #20273

Closed
wants to merge 9 commits into from
Closed

Conversation

oandregal
Copy link
Member

This PR does two things:

  • Adds the CSS variables to most of the blocks (work in progress).
  • Explores a different approach to isolated global styles: instead of using a wp-gs class, it enqueues the styles in a separate stylesheet (if the theme has support for global styles and the experiment is enabled).

In creating an experimental theme for testing, I've found this has a number of advantages over the wp-gs:

  • We don't raise the selector specificity.
  • When authors want to override a particular mapping, they don't need to be aware of whether they need to add the wp-gs or not.
  • Global styles are only enqueued when necessary, so there are fewer opportunities for errors, code that leaks into production, or confusion in the stylesheets.

@oandregal oandregal self-assigned this Feb 17, 2020
@oandregal oandregal added the [Status] In Progress Tracking issues with work in progress label Feb 17, 2020
@youknowriad
Copy link
Contributor

Do you think it could be better to split the PRs block by block (or maybe group by similar blocks), just to review and merge quickly?

@mtias
Copy link
Member

mtias commented Feb 17, 2020

Do you think it could be better to split the PRs block by block (or maybe group by similar blocks), just to review and merge quickly?

Also would be good to focus initially on one or two blocks until we know things are sailing smoothly.

@@ -1,7 +1,7 @@
$blocks-button__height: 56px;

.wp-block-button {
color: $white;
color: var(--wp--color--white, $white);
Copy link
Member Author

Choose a reason for hiding this comment

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

This covers the case where --wp--color--white is not present, so it fallbacks to $white. However, we also need to cover cases where the browser doesn't have support for CSS Custom Properties and write something like:

.wp-block-button {
	color: $white;
	color: var(--wp--color--white, $white);
}

which can be done automatically with some plugins.

Copy link
Member Author

Choose a reason for hiding this comment

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

We also don't want to do this until we're certain this is the way to go.

@@ -1,13 +1,13 @@
.wp-block-pullquote {
border-top: 4px solid $dark-gray-500;
border-bottom: 4px solid $dark-gray-500;
border-top: 4px solid var(--wp--color--foreground-500, $dark-gray-500);
Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -1,12 +1,12 @@
.wp-block-quote {
border-left: 4px solid $black;
border-left: 4px solid var(--wp--color--border, $black);
Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -61,7 +61,7 @@
background-color: transparent;

tbody tr:nth-child(odd) {
background-color: $light-gray-200;
background-color: var(--wp--color--foreground-200, $light-gray-200);
Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -1,5 +1,5 @@
pre.wp-block-verse {
color: $dark-gray-900;
color: var(--wp--color--foreground-900, $dark-gray-900);
Copy link
Member Author

Choose a reason for hiding this comment

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

@ItsJonQ
Copy link

ItsJonQ commented Feb 18, 2020

@nosolosw Ah yes! An alternative global.css stylesheet :D. I think this improves the rendered CSS workflow by the features you highlighted.

From a core block perspective, it's easier to identify the global styles as they live in a separate folder.

As of this moment, I'm unsure how this may play out for 3rd party blocks. Perhaps they too can provide a global.css file?

(Just thinking out loud)

@oandregal
Copy link
Member Author

oandregal commented Feb 20, 2020

Do you think it could be better to split the PRs block by block (or maybe group by similar blocks), just to review and merge quickly?

This was more of an exploratory PR to see how things looked. We may want to do a single PR with a few blocks first and then expand by doing separate PRs per block.

As of this moment, I'm unsure how this may play out for 3rd party blocks. Perhaps they too can provide a global.css file?

I wish we could remove the need for a scoping mechanism altogether, but it doesn't seem like we can so far. The way I see it, we have two alternatives:

  • scoping the "global styles" with the wp-gs class, and ask 3rd parties to do the same (with the problems listed above + the fact that it's a soft requirement that it's easily missed).
  • enqueue the "global styles" in a different stylesheet and ask 3rd parties to do the same (they have to detect support the same way we do).

Both options have cons/pros. I think I slightly prefer the second one for clarity (authors don't need to know whether a style should be scoped with wp-gs, etc).

@oandregal
Copy link
Member Author

enqueue the "global styles" in a different stylesheet and ask 3rd parties to do the same (they have to detect support the same way we do).

Also worth noting that this is only for specific bits, not everything. If a property was already included in the CSS, fallbacks can be used. The issue is for those properties that weren't previously included and we want to add to expose the CSS vars for the style system. This may be a minority of things and there may be situations where it's ok to just have them in the block stylesheet. It needs to be evaluated case-by-case.

@oandregal
Copy link
Member Author

oandregal commented Apr 2, 2020

We're taking a different direction so I'm closing this.

@oandregal oandregal closed this Apr 2, 2020
@oandregal oandregal deleted the try/global-styles-at-blocks branch April 2, 2020 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants