[Meta] Architect our CSS

Draft Proposal here: http://drupal.org/node/1887918
Grouping CSS rulesets: http://drupal.org/node/1887922

The filenames for modules' CSS will match the pattern: module_name.module.css, module_name.skin.css, module_name.admin.css, module_name.admin.skin.css

I don't get this name change. Where was this discussed?

Add new issue link

Why the rename from .theme to .skin?

.theme seems to be perfectly in line with http://smacss.com/book/type-theme

It's even more clear in Drupal, because we have the "theme" terminology already.

+1 for keeping .theme for the reason @sun put forward, it makes more sense to keep the consistency.

+1 to using .skin because the Drupal definition of theme is not the same as the SMACSS definition. The are completely inconsistent.

In Drupal, a theme can cover:

The mark up output.
CSS for the entire layout and structure of a site, which could include multiple variants of a 'theme' from the SMACSS definition.
Javascript that affects the functionality of the site.

I don't think I've ever seen a Drupal theme that covers only colours and type. A good example of the SMACSS definition of theme would be the theme options for jQuery UI or jQuery Mobile.

Add new issue link

Adding tags

Adding tags again...

Add new issue link

Added issue #1941118: Allow underscores in class names by John Ferris

Added the #1969124 issue

remove dead issue link

Where does SMACSS specify "skin" over "theme"?
That rename seems absurd.

Also, the first three patches I looked at aren't rolled correctly.
Please read http://drupal.org/documentation/git/configure and use renames = copies

@tim.plunkett SMACSS uses the term 'theme' to refer to non-structural, non-layout styles (the surface colors, borders, padding, backgrounds, etc.). Since the existing Drupal term 'theme' often refers to a much broader set of things, we wanted to avoid confusion. Are you saying that’s not necessary?

I'm saying that I agree with #5, and I think the name change is a distraction and is even more foreign to Drupal.

I do think the .base.css to .module.css switch makes sense, but let's not blindly switch 100% to a naming convention just because we like 80% of it.

EDIT:

By my grep, the word "skin" appears zero times in core. In fact, "skin" appears zero times on http://smacss.com/book/categorizing too.
We have modules, we have themes, let's call it according to what we have.

In the SMACSS book, the word "skin" appears in two places:

The need for theming within a project is necessary when you want the user to receive an alternate skin that provides some cosmetic alterations.

There are certainly multiple ways to skin a cat.

The entire section on "Theme Rules" makes me think that .theme.css is EXACTLY what we want to be calling this.

Thanks to @dead_arm for sharing her copy of the SMACSS book with me.

I want us to use the SMACSS concepts; the labels we use are less important, as long as folks understand them. The BAT convention already uses “theme” in a way that’s more in line with SMACSS that the normal Drupal meaning, so I guess there’s precedent. I’m fine either way.

What I think we should avoid is the same kind of confusion we had with tpls and template.php. Two completely different things with the same name.

The template.php issue was because there were named exactly the same across themes.

If you can't tell the difference between bartik.theme, bartik.theme.css, garland.theme, and garland.theme.css, that's your problem.

All of the currently RTBC issues need to be knocked to NW.

Hm. I don't know that I see enough compelling arguments here to introduce Yet Another Weird Term Into Drupal's Vernacular, personally. #14 is a pretty compelling argument against it, in fact.

Additionally, in a quick search at GitHub, I can't find any other smacss code out there that's using "skin" but several that are using "theme." So it seems like we'd be inventing a Drupalism here, in addition to introducing another term?

I think that Lewis's concerns in #7 and #16 are valid, but only for people with a deep understanding of Drupal's nuances. For most normal people, "theme" == "how it looks," which is probably why SMACSS chose that term.

As I understand, we refer to "theme" as the whole bunch of files to style our site. But inside our modules we want to have some files as:

- foo.module.css: the minimun styles our module should have to work.
- foo.skin.css: other styles (color, fonts...) that can be overrided by other themes.

The same for admin pages:

- foo.admin.css: base styles (layout, state, component) for the administative pages of our module.
- foor.admin.skin.css: other styles that can be overrided by other themes.

In my opinion, keeping the names foo.theme.css and foo.admin.theme.css will cause confusion with the "theme" term. In other hand, now I am writing this, it seems that the css file names are related to the "theme" term, giving us a the impression that they are part of the theme files.

Late night deliriums...

I don't think anyone would think that /modules/pathauto/css/pathauto.admin.theme.css would be part of a theme, or if they did, that /modules/pathauto/css/pathauto.admin.skin.css would be an improvement.

Right. Or if it were that confusing to lots of people, we should have heard about it by now; D7 shipped with at least system.theme.css for a very similar purpose.

Well, I guess I’m off the fence. I’m no longer afraid the term 'theme' used this way is really going to cause confusion, and that was really the only reason to introduce it. I’d like to hear from @JohnAlbin though.

"skin" on the chopping block again, eh?
I would be happy to not use "skin" as I've voiced before. @tim.plunkett you bring up a fresh perspective in #20.

Unlike "theme", using modular "components" because we have another thing called drupal "modules", makes sense since these can be referred to as nouns, so an imagined mixup in conversation is obvious.

I think we need to do these renames as a single patch, not in separate sub-issues. Because:

#1: It's clobbering up the RTBC queue with individual issues (each carrying their own roll/review/re-roll/re-review/rtbc/re-re-re-review/needs work/commit/etc. overhead), with patches that could easily be scripted and reviewed as a whole.
#2: If they're not all done at once, we introduce inconsistencies in how files are named, which will be confusing to themers. We can't introduce more stuff to "clean up later" at this point in the release cycle. :\
#3: I still am not clear on whether or not this naming convention actually has community buy-in around it, and one place to discuss it would be much easier to determine this than several.

Can we please close these sub-issues as dupes and work at either #1921610: [Meta] Architect our CSS or some other "all in one rename" issue?

And just to be clear, I meant this only for renames which are more or less "the exact same changes made for 30K files." I think the CSS clean-up patches make total sense split into their own issues, because they probably do warrant discussing/reviewing independently, and we don't leave D8 in a "half-baked" state if we only manage to clean up half the CSS we meant to. But this is not true of file renames.

It was separated because we did those issues during a sprint at DrupalCamp SG, just to be clear. Yeah I totally agree with you.

I’m still worrying about this terminology, although I haven’t changed my mind from #22. The following is just something to keep in mind.

SMACSS theme styles within themes, not modules, is the most likely cause of confusion here (“I heard you like themes in your theme…”). Real use case: dark and light colour scheme (user-switchable via js): now you have …themename/stylesheets/theme/dark.css and …themename/stylesheets/theme/light.css. I still think that’s probably OK, because most themes won’t need it. But it is more than a little odd.

Another likely occurrence of separate theme styles within themes in starter/base themes. That might be the most common case in the wild; in fact, I would recommend all base themes carefully separate their base and structural styles the same way modules do, which means you will have theme.css within themes be quite common.

Created a single issue to rename all css files at: #1987066: Rename files to match CSS file naming convention based on request by webchick to make review easier. Refer: http://drupal.org/node/1921610#comment-7375894

Need to clarify naming convention, refer tim.plunkett query at http://drupal.org/node/1921610#comment-7362724

Updated issue summary.

@johnalbin Comment #20 needs your eyes to resolve the theme vs skin debate :)

Cross-post from comment #15on #1987066-15: Rename files to match CSS file naming convention.

Drupal uses the term module to mean something completely different than SMACSS' meaning.

SMACSS module: a single discrete component (read: chunk of html tags) on the page.

Adopting the naming convention of modulename.module.css pollutes the Drupal terminology namespace for "modules", and could create a WTF for two reasons:
1) a single Drupal module could generate code for many various disparate or loosely related SMACCS modules.
2) we need to add more clarification documentation around where the X.module.css convention for SMACSS modules can be used outside the scope of Drupal modules. Other components, including themes, install profiles, etc, could all conceivably contain a css file called X.module.css in order to follow this new SMACSS module naming convention.

With the skin/theme and now module dilema, its clear there is lots of overlap between SMACSS terminology and pre-existing Drupal nomenclature. Thinking outside the box, is there some way to implement and make use of SMACSS layering *concepts* without tying our file names to their naming scheme?

I would like to please you guys also to write comments into the CSS files. Today it's impossible to guess how class names are used and for what.

updated new issue id at #1987066: Rename files to match CSS file naming convention

Please see http://drupal.org/node/1987066#comment-7418144... Is there any point in me re-rolling this patch at the moment?

This seems to be an important task remaining but progress seems to be stalled. I am willing to do the work to get it working again if it will be committed before it breaks again.

@Kevin thanks. I assume you mean #1987066. I think a reroll is appropriate, with 'theme' instead of 'skin'. AFAIK we have consensus again on naming, e.g. system.module.css, system.theme.css.

@Kevin Morse, all renaming issues are moved to a sandbox, and what I understand from 5 in the issue summary above, that issue will be used to combine them in a larger chunk. Each of those issues in the list under "Mobile Initiative sandbox:" actually link to them where they now are located in the sandbox. So it seems the available renaming issues are in the sandbox, and you're welcome to help over there!

following up on the "skin" vs "theme" discussion, my understanding is that using .theme.css has been agreed upon.

if that's correct, some issues would need updating to match the convention, especially
- the summary of #1217054: Clean up the CSS for User module
- CSS architecture (for Drupal 8) currently uses skin instead of theme
- CSS file organization (for Drupal 8) currently uses skin instead of theme

Add link to 1995272

Added a link to the ne style guide prototype

Updated issue summary.

Updated issue summary.

Added "@" to add assignation labels for delegation purposes.

Updated the related issues by switching "skin" to "theme".

Moved #1995272: [Meta] Refactor module CSS files inline with our CSS standards to the Drupal 8 Mobile Initiative sandbox, made it a meta and changed status to active.

@dead_arm, @echoz: thanks; you rock :)

Added spin-off (smaller-scope) alternative issue for allowing '__' in classes

Wrong issue number

#2029187: [meta] Make sure CSS classes are prefixed properly

This will reduce our aggregated CSS files from 4 files to just 2.

Why not only one?

In the context of this issue, could we get some feedback on #1363112: Simplify names of "element-x" helper classes? The patch was committed, but a rollback has been requested under the premise that those classes are now unprefixed.

Added #2012020

I apologise if I'm late to the party. But I'm wondering on something related to the CSS standard in Drupal 8.
I see that Drupal 8 has a BEM-like approach with modifier classes and less nesting.

But in this issue: https://www.drupal.org/node/1968360 the region context for blocks was removed, with the reasons like:

So from a CSS/JS standpoint, you will still be able to target selectors to .region-header .block and similar.

and

contrib and custom themes can use the @extends feature of Sass to avoid similar duplication within their use cases.

Extend can complicate the css: http://csswizardry.com/2014/11/when-to-use-extend-when-to-use-a-mixin/ and make it less maintainable.

To have region available in preprocess_block means that it's easy to for example add a --smallmodifier to blocks in footer or sidebar. And this keep the CSS DRY, since the modifier is not tied to a region. Doing .region__footer .block to alter some styles for a block will create much more CSS than being easily able to add modifier classes. The mentioned issue seems counter productive to the current coding standards.

Looking through this plan, to me it seems mostly done? Issue summary contains these two issue that still are open but they seems a little out of date and not active:

• #1924528: Remove aggregator.theme.css deprecated by entity conversion #293318
• #352951: Make JS & CSS Preprocessing Pluggable

Perhaps it should be closed?

This is mostly done and I'm not sure the meta helps any more, however there are still multiple inconsistent asset libraries in core.

More recent issues:

#2880237: [meta] Refactor system/base library
#3565297: Move 'grid' and 'display link' CSS to their own views libraries
#1924522: Remove separate CSS_AGGREGATE_THEME aggregate file

Also found a couple of extra problems in #3565258: Support library-specific aggregates where the SMACSS categories appear to be wrong for a handful of libraries.

Just closed #3034767: Allow custom aggregation groups as it was clear that it was by design, not sure where that documentation should live though.