Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
We should add a CSS coding standard:
Browser hacks and !important styles are only allowed in themes. Not in core/module CSS files.
That is, because they are more or less irreversible.
There are always proper ways to avoid them.
Comment | File | Size | Author |
---|---|---|---|
#5 | drupal.css_.5.patch | 3.73 KB | sun |
Comments
Comment #1
JacineYes, please.
Comment #2
jwolf CreditAttribution: jwolf commentedYes please, some more!
Comment #3
casey CreditAttribution: casey commented+1 This keeps css more maintainable.
Although we shoud make some exceptions for system module (like .clearfix)
Comment #4
xmacinfoI love standards.
Is this a meta issue? Or will a single patch resolve all CSS standard issues?
Comment #5
sunWhy not? Actually, that's *much less* than I imagined :)
Comment #6
RobLoachThere must be more CSS hacks then just this...
Comment #7
sunClarifying title. ;)
Comment #8
sunLooking at this (again...), it's quite unusual to apply totally different (wrapping) styles to elements, just because JavaScript is enabled, or not? Totally prone to errors IMO... who actually tests core with JS disabled?!
Powered by Dreditor.
Comment #9
sunAll fieldset-specific stuff now lives in a patch over in #745380: CSS contains too many needless differences between JS and no-JS versions
Comment #10
sunAforementioned issue has been committed.
Can someone explain this !important?
Let's straight roll back that patch.
Prefixing the selectors with 'table' should probably be sufficient.
132 critical left. Go review some!
Comment #11
aspilicious CreditAttribution: aspilicious commentedcrosslinking: #766444: Add !important to CSS attributes of .element-invisible in system-behavior.css
Comment #12
james.elliott CreditAttribution: james.elliott commentedsubscribe
Comment #13
casey CreditAttribution: casey commentedWhat about "!important may only be used when the CSS declaration is ment not to be overridden in any circumstance"?
Comment #14
sunWould be cool to resolve the 2-3 remaining issues before beta. Any takers?
Comment #15
sunComment #16
jessebeach CreditAttribution: jessebeach commented@sun, was drupal.css_.5.patch committed? If so, what are the 2 - 3 remaining issues you mentioned?
Comment #17
sunThe remaining issues are those outlined in #10
Comment #18
nomonstersinme CreditAttribution: nomonstersinme commentedI agree that !important should not be used in core and that styling elements based on JS being enabled is a bit annoying. I especially don't think modules should have any !important's...
This is definitely there for the reason the comment states but I've got a problem with these contextual links. I don't like the way they are handled - I think that its css should always load LAST, personally. I hate when I style list items and then my contextual links are all messed up. It creates unnecessary work that I usually just don't even bother with and my cogs are a mess. ;) Obviously that is a whole other issue.
Maybe the way to 'solve' this is to add additional selectors? a bit clunky and not really a proper solution but maybe it would help in getting rid of the !important:
Comment #19
steinmb CreditAttribution: steinmb commentedsubscribe
Comment #20
JacineComment #21
jessebeach CreditAttribution: jessebeach commentedWe can't be dogmatic about
!important
. It's a tool, not a curse. Take these declarations in system.base.css for example.The
!important
s are necessary to ensure proper functioning of these accessibility-enabling classes. When you override them, which we do in contextual.base.cssYou do it with intention. An absolute interdiction on the use of
!important
in our guidelines might cause some overzealous and green CSS purists to mess with these declarations.I like what the proposed CSS Architecture guidelines have to say about
!important
Comment #22
JohnAlbinI agree with Jesse. But I'll bet we have some
!important
s that could be removed somewhere.The patch above is way out-of-date, so let's close this issue and re-open a new issue when we start implementing the new CSS Architecture guidelines in D8.
Comment #23
sunThe point of this issue is to explicitly disallow usage of !important in CSS of core and contributed modules, unless there's a very very good reason for using it.
One of those very good reasons are the .element-hidden, .element-invisible, etc classes defined in system.base.css. Those do not need any further explanation/clarification.
I didn't have time to check whether the remaining instances in #10 still exist, but all of them are problematic. It's also possible that latest HEAD contains new instances.
Likewise, as @nomonstersinme mentioned, elements/widgets that receive special styling (like contextual links) are a "third" category, which we need to address. As a themer, I need to be able to (partially) adjust/override those styles and I certainly don't want to and should not have to use !important for everything I define. At the same time, if my other CSS targets more specific elements that also happen to be contained within those specially styled elements, then my custom styles should not negatively affect those elements.
Comment #24
LewisNyman CreditAttribution: LewisNyman commentedWe now provide guidance on !important in our CSS coding standards, hooray!