Problem/Motivation
Our CSS coding standards have been in "DRAFT" mode for over 4 years. http://drupal.org/node/302199
I've kicked off a discussion about CSS coding standards over on groups.drupal.org and timeboxed it for 2 weeks. Jen Hodgdon suggested I add an issue here in the Drupal queue to point people to that discussion. :-)
Not only do we need to agree upon CSS formatting standards, we also need to architect our entire CSS codebase so that core and contrib has leaner, reusable styles. The architecting will require us to rewrite all the CSS selectors and re-organize our stylesheet files.
Proposed resolution
Because these standards encompass many parts, the standards have been broken into 3 parts. http://drupal.org/node/1886770 is the parent book page for all the parts.
- CSS formatting standards http://drupal.org/node/1887862
- CSS architecture (selector guidelines) http://drupal.org/node/1887918
- CSS grouping (file and aggregation guidelines) http://drupal.org/node/1887922
Remaining tasks
Come to consensus on the formatting standards.Done! See the comments on http://drupal.org/node/1887862 and http://groups.drupal.org/node/277223Move the old CSS coding standards page at http://drupal.org/node/302199 into the "archive" book and add a strong note to the top of that document pointing to the new standards.Done.Remove the word "DRAFT" from the parent page at http://drupal.org/node/1886770Done!Make it clear that the CSS architecture and grouping standards are only for Drupal 8, since they require re-writing CSS which can't be done in Drupal 7 and earlier.There are notes at the top of both of those pages that make it clear that these pages are dependent on implementation in Drupal 8 that is still on-going. They may need to be updated as we encounter implementation problems. Or completely thrown out if we fail to implement them in Drupal 8.Inform the coder.module maintainers about this change.#1922066: New CSS coding standards
Comment | File | Size | Author |
---|---|---|---|
#31 | 10.progress.png | 17.89 KB | ry5n |
Comments
Comment #1
jhodgdonThanks for filing the issue John! (That was me-Jen, not one of many other Jen people in the Drupal project.)
Comment #2
carwin CreditAttribution: carwin commentedThese will only apply to D8 since they involve rewriting the CSS:
edit: Woooooooo!! Let's get this in!
Comment #2.0
carwin CreditAttribution: carwin commentedAdd note about coder.module.
Comment #3
JohnAlbinI've updated the issue summary with what I think needs to happen next.
Comment #4
cosmicdreams CreditAttribution: cosmicdreams commentedI'm ready to have this be a reality!
Comment #4.0
cosmicdreams CreditAttribution: cosmicdreams commentedUpdate issue summary
Comment #5
mortendk CreditAttribution: mortendk commentedThis looks pretty solid all over - rtbs & stfu ;)
Comment #6
jensimmons CreditAttribution: jensimmons commentedI like this. It fits with what much of the non-Drupal front-end community does.
I didn't like the "draft" from four years ago. Which is why Bartik didn't follow it.
Many Drupal front-end developers didn't like it. Which is why it never came out of draft.
Thanks John for keeping on this, and making progress. This is good.
Comment #7
JohnAlbinUpdating title.
Comment #8
heyitspython CreditAttribution: heyitspython commentedLet's do this thing! :D
Comment #8.0
heyitspython CreditAttribution: heyitspython commentedAdded Jen Hodgdon's surname to the 'summary-problem-motivation' section
Comment #9
carwin CreditAttribution: carwin commentedI added an issue to the Coder module's queue: http://drupal.org/node/1922066, which should take care of task #5.
Comment #9.0
carwin CreditAttribution: carwin commentedExplain what to do to old page.
Comment #10
sunI'd like to have a more in-depth look at the proposed draft.
Are the changes compared to now outlined somewhere? I.e.:
1) Compared to our current CSS coding standards
2) Compared to existing proposals that went through more elaborate discussion already?
In general, I'd really prefer to move in smaller steps (like #1270218: [policy] CSS coding standards: Vertical spacing between selectors), because it is very hard to review + discuss + agree on a full-blown proposal.
Comment #11
JohnAlbinThe parts of the old draft standard that made sense were incorporated into the new draft. The rest of that old draft never had consensus and were only in the docs because they were put into the first draft and no one agreed how to change them.
I went through the old draft several times to make sure the parts that made sense made it into the new draft.
I don't think outlining the differences between the old draft and the new draft would be particularly fruitful.
I think its pretty obvious that our existing process only works for existing standards. The CSS "standards" were only ever a draft and the "small change per issue" strategy is not effective. For example:
#1270218: [policy] CSS coding standards: Vertical spacing between selectors — Opened on September 6, 2011. A slow, but useful discussion that never reached consensus until I RTBC'ed it today.
#748810: CSS Coding Standards: Comments — Opened on March 22, 2010. Still open.
#730754: CSS coding standards: No !important in core/contrib modules, ONLY IN THEMES — Opened on March 3, 2010. Still open.
CSS comments? How many blank lines between selectors? The use of !important? Those are incredibly basic things. The fact that we couldn't use the drupal issue queue to resolve those fundamental issues shows the ineffectiveness of this process. Again, I think the ineffectiveness of this process is directly related to the fact that the current "standard" has been draft for 4.5 years and never reached consensus amongst our front-end developers. There were too many parts of the old draft that people hated so they disengaged from participating.
BTW, the 3 issues listed above have all been incorporated into the new Draft standards.
I disagree about review/discuss/agree on a full-blow proposal. We opened up a lengthy groups.drupal.org discussion ( http://groups.drupal.org/node/277223 ) and all of the comments were incorporated into the docs. And the comments on the book pages were also incorporated into the docs. In addition I've held 8 different Google+ Hangouts ( https://www.youtube.com/user/MobileDrupal ), tweeted about this numerous times in Twitter and discussed this with front-end developers at BADCamp and Drupalcon Sydney. I tried very hard to cast as wide a net as possible to get as many voices and ideas to reach consensus.
Pointing people at a 3 page document and asking them to comment was the only way to get this many changes agreed to.
Comment #12
jhodgdonRE #10/11: +1000 on the process used for this update. It was well-publicized, well thought out, and carefully executed (and it also respected our old process by filing this issue so that people who follow the "coding standards" tag could take notice). The existing "process" for coding standards is broken (especially, as noted in #11, for big changes), and until we have another policy/process for coding standards, and/or a working committee to govern them, I think this worked very well. It obviously had way more participation that we ever get on coding standards issues.
Comment #13
cweagans+1 to the new draft and -1000 to splitting this out into a bunch of tiny micro issues to discuss each point ad nauseam.
Comment #14
jessebeach CreditAttribution: jessebeach commentedJohnAlbin knows about my reservations in the proposed standards. I'm not so reserved about these issues that I would prefer no standard at all to more debate. Having a standard at least means we have something we can revision over time, which of course will happen as practice evolves with the underlying technology.
Everyone's going to have a few things to nitpick. It's easier to nitpick in small chunks than to continue to hash out details without a whole to ground the discussion.
John has really thought this proposal through. He's sought input from numerous channels over a long period of time. He's done his best to incorporate feedback from many, many sources.
I may be personally cranky about a few of the changes, but I'll get over it with time and experience.
Let's set this back to RTBC, get something solid under our feet and some calluses on our fingers putting it into action. That, more than anything, will let us know if the proposal works.
Comment #15
ry5n CreditAttribution: ry5n commentedI’ve re-read the three parts of the new standards, and I’m getting excited all over again. I really think they are a step forward. It’s true that we’ll need to keep re-evaluating them, especially the Architecture and Grouping parts, as we apply them to Drupal 8 (and longer-term, as best practices evolve). That said, I am happy to support RTBC-ing this as our official CSS standards.
Comment #16
dasjothe css standards that have been worked out by john albin and others involved in the effort are really up-to-date with what i conceive to be frontend development best-practices established in the wider web communities from the conversations that i'm following.
i understand sun's #10 reasoning, that working on smaller steps is easier. but as explained in the following comments, i think the standards play out their strength in their entirety. splitting them up into separate junks could be a show-stopper for what i conceive as a very positive momentum amongst front-end developers regarding drupal core (this feeling might also be related to Proposal: A Style Guide for Seven and all the work around twig:) ).
i therefore share the excitement and am really eager to see how they are put into practice.
Comment #17
cweagansOkay, so what's the next step here? Just remove the "Draft" label and start using them?
Comment #18
JohnAlbinYep! :-)
Just made that change. FELT SO GOOD.
Yay! Thank you all so much. Without everyone's input and contribution, this change would have been impossible. You all rock!
Comment #18.0
JohnAlbinMarked task 5 complete and linked to the relevant Coder module issue.
Comment #19
sunFinally had a minute to look over the pages. Overall, this looks good. A few issues though:
All pages:
Formatting guidelines
Architecture
File organization
Comment #20
KrisBulman CreditAttribution: KrisBulman commentedI removed backticks (`) from all docs as per comment #19, and replaced them with single quotes (').
I disagree with removing -- and __ from variants and sub-components as stated in Architecture 1. & 2., they help to clearly set them apart when visually scanning a document.
The usefulness of double hyphens and underscores has been touched on thoroughly elsewhere, so I don't feel I have to re-hash their importance here:
http://nicolasgallagher.com/about-html-semantics-front-end-architecture/
http://csswizardry.com/2013/01/mindbemding-getting-your-head-round-bem-s...
Comment #21
ry5n CreditAttribution: ry5n commented@sun It would have been great to get your feedback earlier on this. John timeboxed this so we could move on it. Marking back to RTBC, we can still improve the docs even as they are official.
Can you explain or link to why aligning the values of vendor-prefixed properties leads to version control trouble? In all other respects it is a win for development, so I want to understand the drawbacks.
I strongly concur with #20. Double hyphens distinguish between a component name like 'button-group' and variant like 'button--primary'. Double underscores simply follow that pattern.
Comment #22
sun@JohnAlbin moved the draft into actual/official handbook pages already, so RTBC seems wrong, and I think needs review for the concerns I've outlined in #19 is most appropriate.
re: #20:
The concrete examples that are showcased in the coding standards pages are showing
button--primary
selectors. However, exactly that specific example cannot be found in any front-end framework.Perhaps it's merely the example that is wrong? — That said, that example clearly is a variant.
Speaking of "button" — the current coding standards page does not clearly state yet that abbreviations/acronyms are discouraged; e.g. as in this concrete case, "btn" vs. "button".
Lastly, I forgot to mention that I'm also very concerned about using a custom CSS class {prefix} for Modernizr.
That library has emerged into a standard tool over the past years, and you can easily find soft-to-hard dependencies in all kind of other CSS/JS front-end libraries across the board. Therefore, I do not see why using custom prefixes is a good idea in any way.
That's a typical case of library adoption vs. Drupalism/pedantic-purity. The gains of just using the defaults (like everyone else) have a true impact on more than just Drupal-specific code. We should stick to the defaults.
Comment #23
sunThat said,
The code examples for sub__components are too abstract to make sense of. Do we really have no single example case for that in core already that people can relate to?
If not, can we think of any trivial no-brainer case/issue that we can quickly convert?
Comment #24
ry5n CreditAttribution: ry5n commented@sun I’m happy with ‘needs review’.
- I agree we should add the no-abbreviations recommendation
- The modernizr prefix is a good idea in theory (disambiguation), but in practice may cause issues with external libraries as you point out. I'd like to discus it further.
- The -- and __ naming conventions are from BEM.
A week or so ago I built a demo/proof-of-concept for the new CSS Architecture standards, which is rough but should provide a more complete example.
CSS Architecture Proof-of-Concept:
Demo
Code (Github)
Comment #25
sunThanks for pointing me to a Google Search result. According to those results, that claim is straight-out wrong though.
BEM introduces the double-underscore syntax for sub-components, but uses single hyphens even in its very own examples of elements/components that seemingly look like "variants."
Consequently, someone of us seems to have a pushed forward the double-underscore pattern much further into a space it was not intended for. Quite potentially to achieve artificial consistency.
This looks wrong to me. I'm happy to be convinced otherwise, but so far, I do not see any kind of evidence that double-hyphens for variants are used by anyone anywhere on this planet**.
** Obviously excluding Drupal's very own machine-based HTML ID/class generator functions, which .happen--to---produce-----weird-stuff----at-times.
Comment #26
ry5n CreditAttribution: ry5n commented@sun You are correct about BEM not being the originator of the double-dash notation: for some reason I was not aware of that. It seems that Nicolas Gallagher modified them, which is where I learned about the idea. So, two things:
Nicolas Gallagher’s article: http://nicolasgallagher.com/about-html-semantics-front-end-architecture/
…and his framework (early stages): https://github.com/necolas/suit
Harry Roberts: http://csswizardry.com/2013/01/mindbemding-getting-your-head-round-bem-s...
…and his framework, which he wants to move to this notation: https://gist.github.com/csswizardry/3822990
Philip Walton: http://engineering.appfolio.com/2012/11/16/css-architecture/
And, this from just 4 hours ago :) : https://twitter.com/stubbornella/status/305094843464568832
Comment #27
carwin CreditAttribution: carwin commentedI agree with @sun about the examples being less than perfect. Maybe a good way to test some theories is with some actual Drupal markup that we can consider being a "component" with some variations and sub-components as @sun suggests. Also the point about Drupal's class generator functions .producing-weird--stuff-----at-times is a good one, although perhaps that will need to be a separate issue once these new coding standards are committed.
I don't have the time at the moment to search for some exquisite example of Drupal markup to use for this, but I understand where @ry5n and @KrisBulman are coming from. So here's a completely made-up one, that for various reasons, probably shouldn't be real. Please don't judge me for this :P
The idea there being:
If that (admittedly poor) example doesn't make sense, then maybe I could use more clarification on the actual usage we might get out of this naming pattern within Drupal as well, or maybe a better example is needed (I didn't try very hard). Something else to consider is that if it turns out that this pattern doesn't work for Drupal, we probably shouldn't try to force it to. As much as I respect Harry Roberts, Nicholas Gallagher, et al -- they aren't mystical front-end messiahs. We've gotta be pragmatic.
For the record, I quite like BEM and it's variations.
Comment #28
ry5n CreditAttribution: ry5n commented@carwin a real-world example, with several components (see also #24 in this thread): https://github.com/ry5n/drupalui
Comment #29
carwin CreditAttribution: carwin commented@ry5n Yep, I saw both the things you mentioned, I just didn't see a lengthy example that used both "--" and "__", which I think is what @sun was partially after when suggesting we look for a "no-brainer" case we could convert in #23. Can you help us come up with an example that might use both? That would be great for documentation.
At any rate, the examples you posted are nice, thanks for sharing.
Comment #30
ry5n CreditAttribution: ry5n commented@carwin Oh, sorry. I thought maybe you had missed the example, and I also didn’t read closely enough – I get it now, you’re looking for a single case where we have both -- and __. That *should* be infrequent. I’ll see if I can think of an example and post it here.
Comment #31
ry5n CreditAttribution: ry5n commentedOK, here’s an example: the progress bar from the proposed Seven style guide:
Here’s what we’d probably need (at minimum) for the full-size Progress component:
When it comes time to create the small variant, we won’t need to write much actual CSS, but we’ll likely need to apply a few rules to each of the label, cancel, track and bar. To do this, there are basically two options, the first of which uses the combined -- and __.
Option A (modifier classes all the way down):
Option B (top-level modifier + descendent selectors):
The real difference here is in the HTML they would pair with. With option A, the markup for the progress track would look like this:
<div class="progress__track progress--small__track"></div>
, and similarly for each child element. That’s verbose, but we get to use single class selectors for the variant’s child objects, and if we ever needed a small progress track outside of a Progress component, it would just work. However, our goals may be just as well served by switching to option B, even though we’re doing the “scoping” thing that we usually want to avoid. In this case, however, we have already avoided overly-generic class names by namespacing our child objects within the Progress component. By the same token, we really shouldn’t ever need a Progress track outside of the Progress element, since it’s by definition “attached” to that component. So, we’re still serving most of our goals well, and we greatly simplify the required markup: we only need to add the variant class to the component itself:<div class="progress progress--small"><!-- all internals remain the same --></div>
.I think these are both acceptable strategies. Yes, we should try to be consistent, but I also think we need to start implementing this stuff before we’ll know for sure which works better in practice.
Finally, I hope this example clarifies how these naming conventions interact, how they can help (imagine how much more confusing this example would be with only single dashes) and also what we still need to figure out.
Comment #32
droplet CreditAttribution: droplet commentedI'm really don't like BEM. Interesting in who (open source projects) using it now ?
Comment #33
geerlingguy CreditAttribution: geerlingguy commentedI'm in agreement that BEM looks a bit ugly. I have seen very little about it in design communities, and don't see it being any kind of industry standard (I could be wrong, but I doubt it).
It makes CSS seem more annoying, and 'programmer-ese', with all the double-dashes, and underscores especially.
Comment #34
carwin CreditAttribution: carwin commented@ry5n, thanks for your efforts and explanation, I think you've covered the options well -- I'm marking this back to RTBC now.
My stance on this is that we need to go into this expecting to see some issues arise, but to keep from stagnating it's important that we push forward. I would rather have a standard I have minor disagreements with than absolutely no standard at all. We've gotta have something to work with.
I'd also like to tie up the loose ends that we can in this thread and break out those that require heavy brain lifting into their own issues.
concerns from #19
All pages
Formatting guidelines
Architecture
File organization
concerns from #22
Non-Lethal To-Do:
Comment #35
msmithcti CreditAttribution: msmithcti commentedI think ry5n has done a great job of explaining the BEM style naming convention so far. Aside from the benefit of being able to clearly see the function of a class from only its name, following naming conventions like this (and the "l-" prefix described in SMACSS) makes the developer think about architecture. This is a great way of enforcing many of the other important architectural concepts described in the guidelines.
As for examples, I've just finished overriding most of core CSS and markup for Omega 4 and one of the big goals was to implement these D8 standards, including BEM, which should provide many Drupal specific examples.
Comment #36
droplet CreditAttribution: droplet commentedWhen I read the BEM article, I feel good. But..
I did some research and found only ONE project using it: inuit.css.
the creator is the BEM article writer:
http://csswizardry.com/2013/01/mindbemding-getting-your-head-round-bem-s...
Another article:
http://coding.smashingmagazine.com/2012/04/16/a-new-front-end-methodolog...
At least, this concept has been introduced to front-end developer ONE and more than ONE year. Why no other projects using it?
Futhermore, I try to find a real exmaple. This is Yandex who created BEM
http://company.yandex.com/
http://images.yandex.com/
http://images.yandex.com/yandsearch?text=flowering%20cherry%20tree
I'm TOTALLY lost in it when I look at the source code. Extremely ugly and bad coding.
some concepts I agreed in this article go away from my head.
Comment #37
sunThanks, @carwin!
AFAICS, these are the remaining tasks:
#19:
All pages
Formatting guidelines
Architecture
#22:
This is a generic coding standard that applies to all code in Drupal. You cannot (or should not be able to) find any obscure function or variable names anywhere in Drupal that haven't been intentionally abbreviated (
t()
andl()
are the most well-known exceptions).Comment #38
carwin CreditAttribution: carwin commented@sun, I created a gist containing a quick re-write of what we have so far to use the RFC documentation spec: https://gist.github.com/carwin/5093583
So far, you're the only one who has had a comment on this subject, so I wanted to get your approval on that before modifying the document on d.o. Please feel free to fix or comment on any errors you see there.
Also, is this the latest RFC styleguide? http://www.rfc-editor.org/rfc-style-guide/rfc-style?
Comment #39
ry5n CreditAttribution: ry5n commentedI’ve made a number of clarifications and additions to the CSS standards to address #37:
In the Formatting Guidelines:
- Moved “blank lines” section from Formatting into Whitespace
In CSS Architecture:
- Added the progress bar example from above as a new Case Study section;
- Added the recommendation to use dasherized full words for class names and no abbreviations.
- Removed the recommendation for custom Modernizr classes. Modernizr is no longer mentioned in the docs.
I don’t think there’s anything more to discuss about using Nicolas Gallagher’s modified BEM conventions (component, component__element, component--variant). The mobile initiative meetings discussed the idea of using the newer Montage style as an alternative, but decided against.
I’m not convinced we need to re-write this with RFC language. @carwin, thanks for making a start on it; if we decide to make the change it will come in handy.
IMO The RFC language is really the only thing left; unless someone wants to push for that, I want to mark this RTBC again.
Comment #40
KrisBulman CreditAttribution: KrisBulman commentedGreat work @ry5n! Since everything but RFC has been completed, I am throwing my vote in for RTBC. I see no reason for RFC language to be a blocker here.
Comment #41
carwin CreditAttribution: carwin commented+1 for not letting this be blocked by RFC re-write. We're 6 months into this issue and I really don't think there's any more to it at this time. RTBC - let's commit this thing.
Are there other places within the Drupal documentation where RFC is being used?
Comment #42
JohnAlbinThere's no reason to use 100% RFC language. We're not writing a spec. We are writing guidelines and standards and we should language that helps us to teach these concepts.
Thank you all for addressing the other concerns brought up in this issue. Good work!
Marking this as fixed. :-)
Comment #43.0
(not verified) CreditAttribution: commentedUpdate all the remaining tasks as "completed".