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

Usage of !important rule by version/update #34782

Closed
Sandstromer opened this issue Sep 13, 2021 · 9 comments
Closed

Usage of !important rule by version/update #34782

Sandstromer opened this issue Sep 13, 2021 · 9 comments
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed.

Comments

@Sandstromer
Copy link

Sandstromer commented Sep 13, 2021

Just tracking instances of the CSS !important rule in the various Gutenberg versions.

References to !important in .txt files (ie changelog) are excluded.

Version - Instances of !important
7.2.0 - 130
7.3.0 - 130
7.4.0 - 130
7.5.0 - 132
7.6.0 - 128
7.7.0 - 160
7.8.0 - 156
7.9.0 - 128
8.0.0 - 116
8.1.0 - 132
8.2.0 - 144
8.3.0 - 144
8.4.0 - 136
8.5.0 - 138
8.6.0 - 138
8.7.0 - 138
8.8.0 - 138
8.9.0 - 139
9.0.0 - 157
9.1.0 - 158
9.2.0 - 158
9.3.0 - 158
9.4.0 - 158
9.5.0 - 168
9.6.0 - 168
9.7.0 - 210
9.8.0 - 212
9.9.0 - 238
10.0.0 - 241
10.1.0 - 244
10.2.0 - 246
10.3.0 - 262
10.4.0 - 270
10.5.0 - 273
10.6.0 - 256
10.7.0 - 276
10.8.0 - 275
10.9.0 - 298
11.0.0 - 297
11.1.0 - 301
11.2.0 - 297
11.3.0 - 297
11.4.0 - 295
11.5.0 - 309
11.6.0 - 386
11.7.0 - 393
11.8.0 - 399
11.9.0 - 413
12.0.0 - 423
12.1.0 - 439

@skorasaurus skorasaurus added the CSS Styling Related to editor and front end styles, CSS-specific issues. label Sep 13, 2021
@talldan
Copy link
Contributor

talldan commented Sep 14, 2021

@Sandstromer This issue needs some more information, the main one being the point you're trying to make?

Is it that !important is an indicator of code smell, or that it's hard for themes to override styles, or something else entirely?

If this is related to theming, it wouldn't make sense to analyse the entire codebase, as there's a lot of CSS completely unrelated to theming.

@talldan talldan added the [Status] Needs More Info Follow-up required in order to be actionable. label Sep 14, 2021
@Sandstromer
Copy link
Author

Not making a point or trying to. This is for reference. If there is a better place or format for reference information, please any help would be appreciated.

Sorry I don't know the meaning of "code smell" so unable to answer that question.

Now you mention it, yes it can sometimes be difficult for themes to override styles.

If you propose this be narrowed in focus to theming, do you have any suggestions how to do this?

@mkkeck
Copy link

mkkeck commented Sep 17, 2021

Is it that !important is an indicator of code smell, or that it's hard for themes to override styles, or something else entirely?

If this is related to theming, it wouldn't make sense to analyse the entire codebase, as there's a lot of CSS completely unrelated to theming.

That' not really true.

See #33436, #34575 or #34658 for example.

@landwire
Copy link

The use of !important in Gutenberg interferes more and more with my theme styling, where I am trying to keep the specificity as low as possible.

@Sandstromer This issue needs some more information, the main one being the point you're trying to make?

Is it that !important is an indicator of code smell, or that it's hard for themes to override styles, or something else entirely?

If this is related to theming, it wouldn't make sense to analyse the entire codebase, as there's a lot of CSS completely unrelated to theming.

I think the main issue here is your point: "or that it's hard for themes to override styles".
In my view the problem is however not that much, that it is hard to override styles, but mainly that Gutenberg is overriding carefully crafted theme styles for block based themes with the new rules that are added. And especially the use of !important obviously breaks a lot of things. That should be handled very careful in my opinion.

So I think it is totally valid to make this point about the usage of !important. As you know the code base better, maybe you could analyse where it actually does have an impact on themes and if the usage is really necessary. My main problem at the moment ist the comment here: #34575 (comment)

@talldan
Copy link
Contributor

talldan commented Sep 20, 2021

Sorry I don't know the meaning of "code smell" so unable to answer that question.

Yes, sorry, this phrase probably isn't widely known. It means low code quality—generally something that adds overhead for those contributing to the project, something that is likely to cause a bug, or something that results in another unwanted implication. Use of !important is usually considered an anti-pattern, so I was wondering if this was a code quality metric for the code base.

That' not really true.

@mkkeck What isn't true? I think we have some miscommunication. I was just pointing out that there's a lot of CSS in this codebase for things that a theme generally can't modify, like the editor user interface. Because of that, some of those !importants won't have any effect on themes. There might even be more CSS for that than theme related styles.

I do know that specificity is an issue for theme authors. It's also a really difficult thing to get right for contributors to Gutenberg.

As you know the code base better, maybe you could analyse where it actually does have an impact on themes and if the usage is really necessary.

Looking at particular subfolders would be an option. The packages/block-library folder has all of the block styles, so that seems a good place to start. As you say, it might then be productive to look at individual instances. I would hope that any time !important is used a comment is added next to the line to say why, so hopefully the reasoning is clear.

@mkkeck
Copy link

mkkeck commented Sep 20, 2021

@talldan

That' not really true.

@mkkeck What isn't true? I think we have some miscommunication. I was just pointing out that there's a lot of CSS in this codebase for things that a theme generally can't modify, like the editor user interface. Because of that, some of those !importants won't have any effect on themes. There might even be more CSS for that than theme related styles.

Perhaps of course we had miscommunication. Sorry for that.

Perhaps you've seen my referenced issues.
There some CSS wich have really effect on themes.

I know, that there's lot of CSS wich may not effect a theme, and that these CSS is for the interface.

But CSS from interface should not have any effect on themes, see my referenced issues.

@github-actions
Copy link

Help us move this issue forward. This issue is being marked stale since it has no activity after 15 days of requesting more information. Please add info requested so we can help move the issue forward. Note: The triage policy is to close stale issues that need more info and no response after 2 weeks.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Oct 29, 2021
@devinmaeztri devinmaeztri removed the [Status] Needs More Info Follow-up required in order to be actionable. label Jan 12, 2022
@Thelmachido
Copy link

The other issues @mkkeck referenced have been closed but this issue #37590 to allow developers to opt-out of !important CSS rules generated from theme.json is still ongoing.

Adding for future reference.

@jordesign
Copy link
Contributor

Going to close this in favour of #37590 (as mentioned in the above comment)

@jordesign jordesign closed this as not planned Won't fix, can't repro, duplicate, stale Jul 10, 2023
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. [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed.
Projects
None yet
Development

No branches or pull requests

8 participants