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

Why the !important rule to all inline global styles? #29985

Closed
Sandstromer opened this issue Mar 18, 2021 · 22 comments
Closed

Why the !important rule to all inline global styles? #29985

Sandstromer opened this issue Mar 18, 2021 · 22 comments
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Regression Related to a regression in the latest release

Comments

@Sandstromer
Copy link

Not a bug or feature request as such, but why are the defined global inline styles on the frontend being appended with the !important rule?

Not sure when this change happened as only just noticed it, and as far as I can tell it is not needed.

@skorasaurus skorasaurus added the CSS Styling Related to editor and front end styles, CSS-specific issues. label Mar 18, 2021
@carolinan carolinan added Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Question Questions about the design or development of the editor. labels Mar 23, 2021
@skorasaurus skorasaurus added [Type] Regression Related to a regression in the latest release and removed [Type] Question Questions about the design or development of the editor. labels Apr 5, 2021
@CreativeDive
Copy link
Contributor

That leads to issues.

An example:

I have the class "has-huge-font-size" with a size of 35px on the desktop.

Before, I was able to display such a large heading in CSS smaller if the screen width is less than 768px.

Now none of this works anymore because these classes are all delivered with "!important".

Please remove "!important" from this inline styles again.

@ryelle
Copy link
Contributor

ryelle commented Apr 7, 2021

It looks like this was added in #29533 to fix #29381. cc @nosolosw

I added CSS like this into my own theme, using the provided custom properties which I swap out for different colors in dark mode. That's not working after this !important change. But maybe my approach could work for both the original issue & allow for CSS-level overrides?

.has-blue-background-color {
  background-color: var(--wp--preset--color--blue);
}

@oandregal
Copy link
Member

👋 The rationale for using !important in preset classes generated by the system is this: we had found issues with some blocks whose block selector specificity was higher than the preset classes, hence the presets didn't apply. The preset classes are attached to some block upon some user action, which means the user wants that style to be applied. As per the rationale at #20331 user styles should always take precedence over theme & core styles, they can't be overridden. Using !important is one of the tools we have for this.

By reading the issues you shared, it looks like you're using theme.json + adding some theme CSS stylesheets of your own (we don't output preset classes for themes without theme.json). This is a bit of an edge case, we aim to absorb all the theme CSS via theme.json. In those situations, can you use regular CSS to override the presets as you want? For example:

― font sizes:

<p class='has-huge-font-size'>content</p>
/* This is generated by the system. */
.has-huge-font-size {
  font-size: 84px !important;
}


/*
 * This is added by the theme
 * if it wants to use "unmanaged" CSS
 * of its own and override how the presets work.
 */
@media (max-width: 768px) {
  [class*="has-huge-font-size"] {
    font-size: 23px !important;
  }
}

― colors (I presume you use a top-level class to switch colors)

<div class="is-dark-theme">
  <p class='has-dark-color'>with important</p>
</div>
/* This is generated by the system. */
.has-dark-color {
  color: black !important;
}

/*
 * This is added by the theme
 * if it wants to use "unmanaged" CSS
 * of its own and override how the presets work.
 */
.is-dark-theme .has-dark-color {
  color: white !important;
}

@CreativeDive
Copy link
Contributor

@nosolosw In my opinion to use "!important" to solve this issues is not the best practice way. At the end we at "!important" to all selectors to override the global style classes? Please do not.

There are other possible ways.

Do you have an example, why this global style selectors dosen't work in your cases without "!important"?

Maybe we can find an other solution without using "!important"?

@Sandstromer
Copy link
Author

Sandstromer commented Apr 7, 2021

It looks like !important was added to fix an issue that existed because previously the ordering of preset, theme defined, and user defined styles was incorrect.

The was leading to the wrong style taking precedence, but as far as I can see the rendering order of the 'global-styles-inline-css' has now been rectified, so the !important rule can be removed as no longer needed. is still printed in the wrong order.

@oandregal
Copy link
Member

To expand on my comment above: the issue was, is, and will be given this upcoming change that block selector specificity is higher than the preset classes. Example:

/* block classes that themes use to set styles */
.wp-block-post-title h1 { /* this has higher specificity than the preset class */
  color: green;
}

/*
 * Preset class that is only attached to blocks by the user,
 * otherwise they're not present. If they're attached,
 * they should override any other rule the theme has set.
 */
.has-red-color {
  color: red;
}

Note that we discussed alternatives and thought using !important was simpler.

@CreativeDive
Copy link
Contributor

CreativeDive commented Apr 7, 2021

@nosolosw yes I can understand your example. But this is definitively a theme issue to override a specific headline.

By default the global style classes should be printed without "!important" and the best practice way is to find a good solution by the theme side.

The theme can use something like this to change the color of a specific headline:

.wp-block-post-title h1:not([class*="-color"]) {
    color: green;
}

But your example is no reason for me, to attach the "!important" to all global style classes by default.

On the other side, the theme can do:

.wp-block-post-title h1 {
  color: green!important;
}

And now, your global style classes with "!important" is working? No.

@Sandstromer
Copy link
Author

I spoke too soon without fully checking whether the printed order of defined styles was fixed.

User defined styles such as .has-red-color are still being printed before the theme defined style, only now they both have the !important rule appended, so the user defined color with the !important rule is still being overridden by the theme defined color as this also contains the !important rule.

Using !important is not simpler IMHO, it leads to even more confusion.

@Sandstromer
Copy link
Author

This is the issue I am seeing when changing for example a color in the Site Editor > Global Styles.

Using TT1-Blocks theme, change the Color Palette "Dark Gray" color from #28303d to #28303e and Update Design > Save

View the frontend and inspect the source code to see the printed global-styles-inline-css, notice the ordering;

  1. presets from the theme.json defined styles: --wp--preset--color--dark-gray: #28303D;
  2. presets from the user defined styles (the color change we made): --wp--preset--color--dark-gray: #28303e;
  3. block styles from the user defined (the color change we made): .has-dark-gray-color{color: #28303e !important;}
  4. block styles from the theme.json defined: .has-dark-gray-color{color: #28303D !important;}

3 and 4 are printed in the wrong order, so even though the user defined color is appended with !important, so too is the theme defined color, resulting in the user defined color not taking precedence.

global-styles-wrong-order

What is needed is to print the styles in the correct order, and remove the !important rules.

@oandregal
Copy link
Member

@Sandstromer oh, but that's a bug ― it has nothing to do with !inline. Thanks for reporting. I could repro and filed it at #30563 so it's tracked and can be fixed.

@Sandstromer
Copy link
Author

Sandstromer commented Apr 7, 2021

It seemed to me that the reason for the addition of !important in the user defined styles was to override the theme defined styles, rather than address the issue of duplication or incorrect ordering, but I may have misunderstood and the !important rule was added for another reason.

In any case, I think !important should not be used here. We already have more than enough uses of this (columns block!!), surely adding more is the wrong way to go?

@CreativeDive
Copy link
Contributor

@nosolosw any response?

@ryelle
Copy link
Contributor

ryelle commented Apr 7, 2021

By reading the issues you shared, it looks like you're using theme.json + adding some theme CSS stylesheets of your own (we don't output preset classes for themes without theme.json). This is a bit of an edge case, we aim to absorb all the theme CSS via theme.json.

Yes, I'm trying to make the transition to using the theme.json to manage the custom colors, fonts, etc, but still want to use the cascade of CSS for a dark mode (and because I like CSS 🙂). The example you gave is what I ended up with (higher specificity + more !importants). What I'm actually doing in my theme is this:

theme.json [collapsed to save space in example]

"settings.defaults.color.palette": [
    {
        "name": "Blue",
        "slug": "blue",
        "color": "#20599e"
    }
]

Given that ^, GB will provide --wp--preset--color--blue:#20599e, and I'm using that property in my CSS and in my posts. For example, I have a post with a blue separator in light mode, and that blue should change to a dark pink in dark mode. So I switch out just the custom property value like this, and it updates everywhere blue is used.

@media (prefers-color-scheme:dark) {
	:root {
		--wp--preset--color--blue: #bc3451;
	}
}

Going back to my suggestion, instead of outputting

.has-blue-background-color {
	background-color: #20599e !important;
}

What about using the custom property itself? You can keep the !important here to fix the original issue, because now anyone doing edge-case-y things like me can update the custom property in a class or media query.

.has-blue-background-color {
	background-color: var(--wp--preset--color--blue) !important;
}

@CreativeDive
Copy link
Contributor

@nosolosw any new progress on this issue?

@oandregal
Copy link
Member

@ryelle I haven't seen any PR for this so I went aheand and prepared the proposal you mentioned at #32627 so people can offer feedback.

@oandregal
Copy link
Member

#32627 and other issues raised in this PR are all implemented, so I'm going to close this issue.

@grappler
Copy link
Member

An issue that I have come across is not being able to easily introduce hover styles if a background colour ist set. Hover Styles are not supported yet #34717

@tlovett1
Copy link

tlovett1 commented Feb 1, 2022

This is nuts we are using !important. This change broke one of my sites.

@webmandesign
Copy link
Contributor

This is nuts we are using !important. This change broke one of my sites.

Can be fixed in CSS by defining WP custom properties as seen above, or via PHP which brings CSS to where it was before.

@jnicol
Copy link

jnicol commented Feb 10, 2022

Can be fixed in CSS by defining WP custom properties as seen above, or via PHP which brings CSS to where it was before

Moving forward it's easy enough to workaround, as you describe. But for agencies that manage a large number of websites it's painful to go back and edit each one. Removing !important would solve the issue.

I'm a little perplexed why !important is being added even when there are no user defined font sizes/colors? The rationale for using !important is so that user defined preferences trump everything else. But on my sites there is no theme.json and no user defined preferences but WP's default CSS is still being inlined with !important.

@webmandesign
Copy link
Contributor

webmandesign commented Feb 11, 2022

Hi @jnicol,

I've compiled the code into a plugin https://github.com/webmandesign/global-styles-mods

After installation you can also copy the global-styles-mods--mu.php file into your ./wp-content/mu-plugins/ folder so it is used as a "must use" plugin - active on all sites in multi-site environment.

Plugin installation ZIP file can be obtained from https://github.com/webmandesign/global-styles-mods/releases

I've also submitted the plugin to WordPress plugins repository, so hopefully it will be available there too.

@webmandesign
Copy link
Contributor

To whom it may help: plugin Global Styles Mods is out at https://wordpress.org/plugins/global-styles-mods/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

No branches or pull requests

10 participants