CSS Coding Standards: Comments

I'm not sure whether we need that @file block. It's nice and all, but most often, I have no idea what to put in there - other than the totally obvious "Styles for the Foo module." (?!)

In general, "should be" in our coding standards means "must be".

For recommendations we state "may be" or actually clearly as in "It is recommended (but not required) to ..."

In this case, however, I'd say it makes no sense to document it in the coding standards at all, since we already state we're following CSSDoc, so if someone really wants to use a @file block, he can follow CSSDoc rules.

Also, I just recognized that we have no coding standard for "inline comments", while reviewing #223341-14: menus cannot be created using <ol>

+++ themes/garland/style.css Locally Modified (Based On 1.74)
@@ -94,7 +94,7 @@
 /* Default to menu leaf bullet for unordered list items. "ul" used here so it can cascade to list items and "li.leaf" to override the system leaf image. */
-ul, li.leaf {
+ul, ul li.leaf {

That comment (not changed in that patch) clearly exceeds 80 chars. Bad. But then again, turning this into

/**
 * Default to menu leaf bullet for unordered list items. "ul" used here so it can
 * cascade to list items and "li.leaf" to override the system leaf image.
 */

would be wrong, IMHO. :( Ideas?

Hm, @group? Apparently, I haven't read the CSSDoc draft yet. ;) I'm used to @defgroup, @ingroup, @addtogroup for official groups forming separate API documentation pages, and @name for member groups.

Skimming it now, some interesting things to consider:

1. Common CSS sections are: reset, layout, typography, content

   yeah :) Would be cool if we would follow that high-level structure, too.

2. Actually, a
   

   /**
    * @file
    * Garland theme
    */
   

   would, if parsed into an API documentation like api.drupal.org, nicely be displayed as the title for a file (instead of only seeing 4 identically named style.css files).

   Alternative titles like "IE6 overrides for Garland theme." would actually make sense, too.

3. CSSDoc clarifies that @group is a vendor-specific tag of CSSEdit, and they are awaiting feedback from the vendor to clarify how it actually works.

   Looking at your screenshot, I'm a bit confused by how it works - it seems to randomly intercept any @group and @end statements without any real grouping or limitation for the ending tag:

   /**
    * @group Garland layout
    */
   
   /**
    * @end
    */
   

   Look, as a developer who already worked on a couple of text parsers, I have serious issues to accept that such a loosely tag pairing is going to work out and provide the needed flexibility in the end.

   Most of us are rather used to PHPDoc's grouping: (you can replace "defgroup" with "name" here to get similar member groups as with the vendor-specific @group statement above.

   /**
    * @defgroup garland_layout Garland layout
    * @{
    * Optional description for the Garland layout. First line used as teaser in overview listings.
    *
    * The "@{" opens a grouping/section (what kind depends on the tag before it),
    * and groups can be hierarchical. The "@}" below closes this group, identified
    * by its internal name.
    */
   
   /**
    * @} End of "defgroup garland_layout".
    */
   

   Actually, the entire CSSDoc draft contains much blah blah about sections and grouping, but not a single statement on how groups are currently supposed to work. It doesn't seem like they took the time to actually read the resources they are constantly referring to (PHPDoc/JavaDoc/Doxygen).

   Note that I'm totally open to discuss and don't want to force us in any direction, but for me, it would make hella more sense to follow existing PHPDoc/JavaDoc/Doxygen patterns on grouping. :)

4. Interestingly, CSSDoc abbreviates Firefox with "fx", not "FF". However, they also state that UAs can be named freely.
5. Unfortunately, they also share no ideas on inline comments. :(

Strictly technically speaking, it's the

/**
 * @{
 */

/**
 * @}
 */

that forms a grouping in Doxygen/PHPDoc/JavaDoc.

The preceding @foo tag name is just a matter of *Doc standardization and just needs to be identified by the parser. So if we forget about the additional logic of defgroup/ingroup/addtogroup, then it's actually not important whether it's a @name, @group, or @wonky.

However, to help the parser to identify start and end points of groups, as well as to allow other tags to re-use names, like @link group_name More info about Group-Name @endlink, Doxygen/PHPDoc/JavaDoc requires you to at least provide an internal name for any grouping:

/**
 * @group garland_layout
 * @{
 */

/**
 * @} End of "group garland_layout".
 */

Of course, the additional logic of defgroup/ingroup/addtogroup would especially be useful for Drupal's modular concept:

--- modules/system/system-behaviors.css
/**
 * @defgroup collapsible Collapsible fieldsets
 * @{
 */

...

/**
 * @} End of "defgroup collapsible".
 */

--- themes/garland/style.css
/**
 * @ingroup collapsible
 * @{
 */

...

/**
 * @} End of "ingroup collapsible".
 */

Clearly - if this would be parsed into something like api.drupal.org, you'd get a totally very nice awesome wonderful overview of all styles belonging to a certain topic. :)

FYI: I just did a major revamp of the CSS coding standards page: http://drupal.org/node/302199

None of the details discussed here are contained yet though.

Not trying to stir the pot here but i just thought i'd add my $0.02....

As far as grouping is concerned, to be quite honest the grouping suggested by Doxygen/PHPDoc/JavaDoc standards doesn't sit right with me as a designer, its too much. I like to group my styles like Jacine but I don't think it should be part of a CSS coding standard... imo.

I'm interested in removing the "draft" label from the standards document, so I can be comfortable referencing the standards (both for myself and others) knowing they're actual standards and not something that may change tomorrow. To that end, can we remove anything that's disagreeable and at least publish a shorter list of what is agreeable, while we discuss new additions in issues? Currently we effectively have no standards, and something is far better than nothing.

Also, these standards affect contrib as well as core, right? Should this be moved somewhere more visible to the wider community? I only found this issue when I opened an issue under documentation, where I assumed this stuff would be discussed: #1140268: Document reasoning behind CSS and markup standards

Would love to make some progress here. Just had to "solve" the multi-line inline CSS comment scenario somehow for #1010080-34: Input elements overlap sticky tableheader

In particular, the only options I would have had for that patch were:

 table.sticky-header {
   background-color: #fff;
   margin-top: 0;
+  /* Render sticky table headers above any other (relative) positioned elements,
+     but leave sufficient room for z-indexes in a theme, and lastly try to not
+     conflict with "page-level widgets", such as toolbar. */
+  z-index: 100;
 }

or

+++ b/modules/system/system.base.css
@@ -137,6 +137,10 @@ div.tree-child-horizontal {
 table.sticky-header {
   background-color: #fff;
   margin-top: 0;
+  /**
+   * Render sticky table headers above any other (relative) positioned elements,
+   * but leave sufficient room for z-indexes in a theme, and lastly try to not
+   * conflict with "page-level widgets", such as toolbar.
+   */
+  z-index: 100;
 }

The latter felt a bit wrong outside of a real CSSDoc block, so I went with the former. But then again, with the former, the indentation of subsequent comment lines feels a bit odd when writing it (not so much when you look at the result).

When I looked at that patch at first I thought it was pretty good, but reviewing Jacines comment above tends to make me think its better to be above as per her example, maybe we should never have multi-line inline comments in CSS and restrict them to 1 line, anything multi-line goes above? Declaration blocks are seldom extremely long (as compared to say a function), so above would be fine in my book - its still pretty easy to scan the declaration block and find what what the comment is referring to.

Strictly speaking, it is not the responsibility of coding standards to specify what to do and what not; the purpose of coding standards is to rule how you do it if you do it.

Of course, there are exceptions to all rules, and therefore coding standards sometimes also need to rule out inane ideas in the first place. However, I don't think this applies to this particular case, as generally prohibiting usage of multi-line inline comments is highly debatable.

In general, there are different levels of code comments, which apply to all programming languages:

1. File-level comments:

   Describing the bird-level scope/functionality of the file contents.

2. Group/section-level comments:

   Explaining the correlations, intentions, and the non-obvious pertaining to multiple implementations/declarations within the group/section.

3. Implementation/declaration-level comments:

   Always summarizing the effect ("what does this do?") of the implementation, optionally enhanced by a longer description that explains the non-obvious underlying reasoning for high-level details of the implementation.

   Intended audience: Technically oriented consumers/implementors.

4. Inline comments:

   Explaining each and every low-level detail pertaining to individual/inside/in-depth aspects within the implementation that need or should be explained to make the non-obvious obvious for anyone reading the actual code, but which are not necessarily required knowledge for everyone who merely scans implementations or attempts to use an implementation.

   Intended audience: Developers, people who might attempt to change the code, and therefore have to know about earlier reasonings for the existing in order to understand the consequences of their changes.

This means, one essential difference in comment types is the intended audience. It would not be wise to generally dismiss the notion of developer-centric inline comments.

Since I might be convinced that the given example is kinda borderline, let's take a different one: .element-invisible in system.base.css. I believe most of you participated in the original issue and follow-up issues, so kinda needless to state that those effective 3 lines of code required an evil and neverending nightmare of proposals, patches, manual browser-testing, accessibility testing, follow-up fixes, and whatnot. There are countless of non-obvious details in those innocent 3 lines, but none of them are documented. A new core contributor with lots of time could rightfully go ahead and attempt to "fix" that ugly mess of weird looking CSS with a much more simple solution, effectively wasting your time, my time, her time, everyone's time.

And there we have the separation: Such technical details are not relevant for the average Joe who attempts to use .element-invisible. The CSSDoc block explains the intention and usage, and thankfully provides a reasoning for why it's also using an evil !important - all of that is highly relevant for Joe, but any kind of technical reasonings explaining the WTF of actual code are not.

The technical details belong into the implementation as inline comments. Given aforementioned example, needless to say that some inline comments could become lengthier.

OK, well I think sun's arguments are reasonable and well thought out. Sometimes inline comments may indeed be lengthier.

I have no opinion on indentation, generally speaking in my own work I would indent the comment, mainly because I have OCD and want everything to line up like little soldiers...

If we're taking a vote, I'm with Jacine's second example, moving the comment above the code block.
Otherwise, I'd go with sun's first example, indented but with no leading *.

As far as comments go, I think moving them above the css as per Jacine's second example is better – but I have some bias, inline comments break CSSEdit's autocomplete for properites after the comment (yes I'm another CSSEdit addict).

For grouping comments - if we could use CSSEdit's @group syntax or something compatible that would be excellent but I also don't like the idea of anything so vendor specific. The question is, are there *any* other editors that provide a UI around grouping comments? And if so what do they use?
In the absence of any others @group is at least human readable…

Partially related to this discussion: #1270218: [policy] CSS coding standards: Vertical spacing between selectors

Given that:

• #1270218: [policy] CSS coding standards: Vertical spacing between selectors is now fixed (which specifies that rulesets with no interleaving blank lines must be related logically.
• CSSDoc is DEAD. See my comment here: http://groups.drupal.org/node/277223#comment-876638
• And that we've agreed to some new CSS comment styles in our now-official CSS coding standards

I'm closing this. If you feel there is some useful bits in this discussion that should be added to the new coding standards, please feel free to update the issue summary and then re-open.

I think the question on inline comments as outlined in #14 + #15 is not resolved yet.

But OTOH, it might be pointless to even try to come up with anything "sane", since the CSS language itself doesn't really allow for anything sane.

That said, the documentation scopes and intended audiences as outlined in #17 might be something that would be worth to incorporate?