Coding standards luca

[LucaPipolo]

This PR contains the changes me and @sun discussed yesterday:

• Remove excessive white-space / blank lines.

• Remove banners at beginning of files / replace them with Doc blocks.

• Remove unnecessary comments // Component variables

• Change rule order to margin > border > padding.

• Replace our custom stylelint rules with AirBnB standards with https://github.com/stylelint/stylelint-config-standard

  Background info: AirBnB's own standards are currently based on csslint, but Initial Airbnb stylelint shared config stylelint-config-airbnb airbnb/css#23 converts them to stylelint.

Notes

• This PR is intentionally incomplete, in order to discuss the proposed changes first before possibly wasting time. Only agreed-on changes will be performed in all files.

Details

1. in our opinion all files are a bit too spacey: lot of useless blank lines that does not help the readability. I agree to keep some one (e.g. after comment blocks or between nested rules) but not all of them.
2. the properties order still seems a bit weird to us. Why don't use the SMACSS default order? @colourgarden you reverted the commit where I introduced it but can't remember why and the decision seems not documented anywhere. 😔

[sun]

i changed the base branch of this PR to proposed-changes-luca instead of master so that the actually proposed changes are visible/clear.

[sun]

Updated the PR summary. Checking a checkbox means that we agree on the proposed change and that it can be implemented for all files. Unchecked checkboxes will wait until there is consensus.

[colourgarden]

Thanks for your efforts here. My feedback to the bullet points below.

1. Remove excessive white-space / blank lines.
The whitespace is certainly not useless or pointless. It's a pretty simple system - one line between related blocks and two lines between unrelated blocks. I find these extra lines very useful when scanning a file - there's instant recognition of a new block.

This convention follows CSS Guidelines and is fairly widely adopted (including by Bootstrap). My take is that whitespace is free so why not use it liberally.

What are your particular concerns with the whitespace/blank lines?

2. Remove banners at beginning of files / replace them with Doc blocks.
Reason for change?

In the compiled (but uncompressed) CSS, these banner comments make it very clear where a new component/section starts which is very useful when reading through that file. Why do we want to remove these visual markers?

3. Remove unnecessary comments
Yep I guess they are pretty pointless. Happy to remove. I'd like to see two lines above and beneath to provide separation. Somewhat related to point 1.

4. Change rule order to margin > border > padding.
Why? The current order is based on the box model. Internal space (width->height->padding->border) and then external space (margin).

I'm actually indifferent to the border property because, with border-box sizing, it is just presentational styling but padding should certainly be grouped with width and height.

5. Replace our custom stylelint rules with AirBnB standards.
What are the specific changes here? Our stylelint file is based on the stylelint-config-standard and I then spent a couple of hours going through the docs rule-by-rule and adding specific settings I thought were sensible and testing them.

I've been using this property order for 5-6 years and find it logical and easy-to-predict.

1. Positioning - where the component is placed in the document
2. Display/Box-model - how it is sized and reacts to its siblings
3. Presentational - background, color, transitions etc

It's endorsed by Nicolas Gallagher - https://github.com/necolas/idiomatic-css - and various other major libraries like Bootstrap, Inuit and Photon.

As long as we agree on something and stick to it that's the most important thing - consistency. I'd just like to see specifics before we make changes because I've put a lot of time into setting everything up so I'd prefer not to change things just on personal preferences.

Thanks!

[sun]

1. "The lines of a function should fit on a screen" is the opposing programming guideline (don't have time to dig out sources/supporting material, but I assume all of you are aware of that principle anyway); too much white-space/blank lines cause relevant lines to have a higher chance to be outside of the visible screen viewport, which is known to reduce the code overview, readability, and slows down understanding.

2. The banners are duplicating the directly following Doc block in most cases, causing the banners to be redundant and unnecessary. Some of the banners currently have more than one line (i.e., a description/reference), which is another sign that they're duplicating the Doc blocks.

3. 👍

4. I've only ever seen the logical order following the box-model, from outside to inside, especially because the outside margin affects positioning (which also comes before):

   1. margin
   2. border
   3. padding

   In contrast, the current order in Bacon is:

   1. padding
   2. border
   3. margin

   Unless I'm mistaken, that order is not in line with any kind of code that we've ever written at netzstrategen. (And Drupal core/contrib for that matter.) If there's a non-obvious benefit to this order that we're not aware of, then we should propose, explain, and discuss that.

   Is there an option in stylelint to lift the restriction and just say that border/margin/padding - as a group - should come at a certain position, without requiring any particular order of the three properties?

   I just want to make sure we're not slowing us down due to standards that essentially ask all of us to write code in kind of the reverse order that we're used to.

   We may as well shortcut and define to sort all properties alphabetically. (Just to mention the option, it is a valid one. In fact, Drupal coding standards use alphabetic sorting as fallback, also to universally cater for new properties that have no special placement yet. Nicolas Gallagher also mentions the option.)

   When in doubt, in general we should not define strict rules/standards on which we cannot reach consensus. We can still discuss and introduce an agreed-on rule at a later point in time.

5. Exactly right, I agree we shouldn't base decisions on personal preferences. 😉 Let's just ensure we're following more widely known standards and don't conflict with Drupal standards (and/or their currently ongoing upgrades to stylelint/AirBnB).

   Speaking of, how do our currently proposed rules differ from or compare against https://github.com/drupal/drupal/blob/8.4.x/core/.stylelintrc.json ?

Thanks!

[fabianmarz]

What happened to this PR here? Why did it felt asleep after such long writings? 😴

[LucaPipolo]

@fabianmarz maybe because was too much writing?! 😂

However agree with you… how to proceed here? Who should take care of what? Can we have a resume of what left?

[sun]

We replaced our coding style standards with more relaxed industry standards by now, so I believe this issue is obsolete.