The attached patch adds two simple and related bits of automation.
1) If a module is active, and there is a file in the module's directory named modulename.css, it will automatically be included in the page head.
2) However if modulename.css exists in the currently in-use theme, then that file will be included instead, and the one in the module's directory will be ignored.
Both of the above happen automatically with no code needed on the part of a module or theme.
Here's why:
Currently, if a module wants to include a CSS file, it has to do so manually by calling theme_add_style(). I keep seeing contrib modules put that call inside hook_menu(), which makes not the slightest bit of sense. :-) The exception is core modules, which have all of their CSS rules dumped into the oft-malaigned drupal.css file. Those rules are then included always, even if the module to which they correspond is not active.
Moreover, if a theme wants to style something differently from the way drupal.css or the module's own CSS file define it, then the theme author has to specifically override every part of the rule in question. We've had the discussion about how annoying that is many times, so I will not repeat it here. :-)
With this patch, Drupal will pick up module CSS files automatically if they exist. The module author need not pollute the already-abused hook_menu() with it. Moreover, theme authors can trivially override a given module's CSS file by simply copying and pasting the file and modifying as needed, or even making it an empty file to have no rules at all.
That also paves the way for the death of drupal.css, now that core modules are in their own directories (yay!). With this patch, drupal.css could be split up into modulename.css files for the appropriate core module and placed appropriately. Then, with no code changes, all of the extra stuff in drupal.css that you never use if a given module is not active goes away, and overriding drupal.css in themes becomes trivially easy; copy/paste/modify the file in question and you're done.
Known issues:
- I'm not certain that I have this code in the best function. :-) I've put the logic into theme_get_styles(), but I can see an argument for it in drupal_get_html_head() as well. I defer to the big people on this one.
- Currently it does not handle different media types in the slightest. Not all contrib modules do either, to be frank, but I am open to suggestions on how to handle those.
- It may be faster to cache this information rather than rebuilding it every time. Again, I defer to those with more experience with caching.
Feedback requested!
| Comment | File | Size | Author |
|---|---|---|---|
| module_auto_css.patch.txt | 1.01 KB | Crell |
Comments
Comment #1
Tobias Maier commentedi like your concept of adding and overwriting css files automatically.
the easiest solution to add handling of different media types is to add the
@mediaat-rule:but whats about this naming:
module_examplemodule#print.css"module_" is there that the devs see easiy that this is a style which overwrites a modules' stylesheet
"#" makes the break after the modulename
and then comes "print", which is the media type
if this file fits for more media types we could seperate them with a comma
if it is just named
module_examplemodule.cssthen it is media type "all"
Comment #2
Crell commentedHm. I like the name idea, although I'd simplify it somewhat. modulename.css is "all", modulename_print.css is for "print", modulename_screen.css is for "screen", etc. That wouldn't be hard to do.
Unfortunately it looks like the big names are behind this patch instead. Frankly I don't like a programmatic solution to this problem as it means themers have to write PHP code that calls path-futzing functions, but it looks like I'm going to lose this one. :-(
Comment #3
dries commentedDoes no longer apply.
I don't think this is necessary. Modules want to insert CSS files in an intelligent way, and can do this as of today (see CSS patch that got committed). Tempted to mark this "won't fix".
Comment #4
merlinofchaos commentedI agree with Dries.
I actually want to push Drupal to do more including of CSS files on demand, rather than have more and more CSS always there.
Comment #5
Crell commentedI respectfully disagree, as I think the new method from the patch that went in is too heavy and requires too much PHP work on the part of themers. But, since that went in I guess this is now redundant, so I'll mark it won't fix.
Can't win 'em all, I guess. :-)
Comment #6
merlinofchaos commentedI agree with the portion that a theme css file should be able to easily override a module or core's CSS file. It would be fairly trivial to add that part to the existing mechanism.
I think theme_stylesheet_import could do it relatively easily.
With that in mind I'll re-open this.
Comment #7
Crell commentedMerlin, I'm not sure I follow. You mean you want to see this patch (as laid out in comment #2), and the one that just went in? How is that not redundant?
Comment #8
Crell commentedMerlin, can you clarify? If you can't show how this is not redundant with the patch that already got in, I'm going to close this. :-)
Comment #9
Crell commentedYeah, redundant now.