Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Part of the CSS Cleanup: http://drupal.org/node/1089868
Overview of Goals
- Make it easy to remove unwanted design assumptions in the theme layer, while maintaining critical functionality (such as functional JavaScript widgets).
- Prevent uneeded administrative styles from loading on the front end.
- Give modules the ability to include a generic design implementation with their module, without burdening themers.
- Make CSS and related markup more efficient and less intrusive to improve the themer experience.
The CSS Clean-up Process
Use the following guidelines when writing patches for the core issues listed below.
- Put CSS is in the appropriate file: CSS should be moved to separate files, using the following
name spacing conventions based on their purpose:- module.base.css
- Should hold structural and behavior related styling. CSS should be coded against the Stark theme. The absolute bare minimum CSS required for the module to function should go here. If there is no CSS required, this file should be omitted.
- module.theme.css
- Should hold generic design-related styles that could be used with Stark and other themes. It's where all design assumptions like backgrounds, borders, colors, fonts, margins, padding, etc, would go.
- module.admin.css
- Should hold styles that are only applicable to administrative pages.
To see an example of this in practice, look at Drupal's system module.
- Remove Assumptions: Styles that make too many assumptions, introduce superflous margins, padding and add things like font settings are not necessary and don't belong in core module CSS files. In cases where core themes depend on these properties, they should be moved to the CSS stylesheet of the respective theme.
- Reduce Selector Specificity: CSS code that resides in modules should be written in a way that's easily overridable in the theme layer. To improve the Themer Experience and make core CSS more efficient, CSS selectors should be made as general and short as possible. For example:
- Use
.style {}
overdiv.style {}
where possible. - Use
.module .style {}
overdiv.module div.somenestedelement .style
where possible.
- Use
- Don't use IDs in selectors: Use of ID's in core CSS selectors requires more specificity in the theme layer, making it harder and more annoying to deal with. It makes achieveing consistency in complex design implementations much harder than it needs to be. We need to stop making life hard for theme developers.
- Don't be afraid to change markup: There's lots of overlap between using proper and semantic markup and doing CSS right. If you come across a case where CSS is being applied where using a more semantic elements would solve the problem, then change the markup in your patch to make it right. For more information, see the Drupal 8 Markup Gate rules.
- Start with Stark and cross-browser test.
- "Design" markup and CSS for the Stark theme.
- If applicable, adapt the styles to match the core themes afterward.
- Finally, test the changes in all supported browsers and ensure no regressions are introduced.
Comment | File | Size | Author |
---|---|---|---|
#13 | search-1217036-13.patch | 2.57 KB | jyve |
#9 | search-1217036-9.patch | 2.49 KB | jyve |
#8 | search-1217036-8.patch | 2.5 KB | jyve |
#6 | search-1217036-6.patch | 2.27 KB | jyve |
#5 | search.seven_.after_.png | 177.34 KB | Jacine |
Comments
Comment #1
franzHow should -rtl.css files be handled? I think they should be either equally split into categories proposed or be all included into base.css.
In this patch I left them in base.css
Comment #2
franzComment #3
droplet CreditAttribution: droplet commentedI always want to help to do some cleanup but [#1089868] is not clear. no good example and further explanation.
I only see this issue needs review now. I try to share some of my thought and get some idea so I can work on other clean up issues.
I feel that no a require for search module. without it, still worked well with Stark theme.
I think font-size should be in theme.css
If I look at system.module example, it may move to base.css. It provides base behavior style ?
Comment #4
jyve CreditAttribution: jyve commentedI agree with most of the feedback from droplet, but have taken it a little further. Most of the css can be removed imo, since it makes useless assumptions about margins, paddings and font-size. Removing that css did not change anything in Stark, and almost nothing in Bartik (only font-size on the search result header).
Attached is a new patch.
Comment #5
JacineThis is an awesome cleanup @jyve.
I'd actually expect to see a numbered list here when I remove .theme.css files. It's not exactly a structural or behavioral requirement, so I think this should go in the search.theme.css.
This should have a LTR comment.
After that stuff is fixed, this is RTBC in my book. I've also attached before and after screenshots. It's pretty amazing how much garbage code we have. The differences are teeny tiny. Nice work! :D
Comment #6
jyve CreditAttribution: jyve commentedThanks for testing @Jacine :) It does feel good to get out a pile of useless css.
New patch attached with your feedback!
Comment #7
Jacine@jyve Sure no problem. Sorry I didn't end up having time for more. Per the other issue, we're gonna have to add the appropriate comments. Basically we need a @file block at the top, and I think the rest is self-explanatory. Hopefully @sun will agree.
Comment #8
jyve CreditAttribution: jyve commentedA @file block was added at the top, together with block group headers, just to be on the safe side :)
Comment #9
jyve CreditAttribution: jyve commentedupdated the wording just a tiny bit.
Comment #10
xjmThanks for your work on this patch! Note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."
Tagging as novice for the task of rerolling the Drupal 8.x patch.
If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.
Comment #11
BBommarito CreditAttribution: BBommarito commentedComment #12
BBommarito CreditAttribution: BBommarito commentedComment #13
jyve CreditAttribution: jyve commentedRerolled the patch to match the new folder structure.
Comment #14
aspilicious CreditAttribution: aspilicious commentedTested, and reviewed, jacine already approved this.
NCE!
Comment #15
catchLovely, lovely minuses.
Committed/pushed to 8.x, thanks!
Comment #16.0
(not verified) CreditAttribution: commentedUpdated issue summary.