CSS naming conventions

[elaihau]

In this code base, some CSS class names are camel cased

And we probably should go with the dash-separated names, with the following reasons:

https://csswizardry.com/2010/12/css-camel-case-seriously-sucks/

https://medium.freecodecamp.org/css-naming-conventions-that-will-save-you-hours-of-debugging-35cea737d849

[akosyakov]

Could you point which technical issue does it solve?

• We won't be able to do it generally since underlying packages like phosphor does not follow this convention.
• So far nobody cared and it was not an issue.
• It should be automatically enforced at the same time it should not affect the build time significantly. Otherwise, they won't be followed, PR reviews will get longer or the build.

[epatpol]

I think we should at least try to be consistent in the naming of the CSS classes defined inside Theia, unrelated to those from Phosphors. I.e we already have coding guidelines for .ts files but recently when I had to add a class to CSS I wasn't sure which case to use.

An example is .theia-TreeNodeSegmentGrow and .theia-app-sides or .theia-TabBar-hidden-content. AFAIK that's not from Phosphor.

I agree with you on the build time and reviews taking longer, we don't want to make the process slower for sure.

[elaihau]

I believe "being inconsistent with naming CSS classes" makes the code more difficult to read and maintain, and I believe that's also the reason why we want to follow naming conventions when we name the classes, functions, and variables in our Typescript / Javascript code.

Since we are already using tslint to pinpoint the readability and maintainability issues, I guess using something like css / scss lint is worth considering.

@akosyakov I agree with you regarding the fact that "it was not an issue". From my personal experience, using camel cased CSS class names causes more confusions (in both reading and debugging) when I used Angular / React to build the front end, while in theia project it is not the case.

[akosyakov]

@elaihau I pointed my concerns. You should try to introduce it (open a PR) and see whether:

• it is possible to auto-lint in VS Code and during the build without false positive
• it affects the build performance

[kittaakos]

I think it is a great idea. I wanted to file an issue when I had to debug the styles. It was a nightmare. Finally, I ended up adding more and more CamelCase classes because that was the convention in the tree.

it is possible to auto-lint in VS Code and during the build without false positive
it affects the build performance

I think we can do this without any sophisticated linters. Following a guideline is more than nothing, I agree with @epatpol and @elaihau. We should write up something, change the existing class names and stick to the guideline as much as possible.

[akosyakov]

ok, please go ahead and write up something

Changing all existing class names at once sounds like a big testing effort to me, but as a guideline for new classes is fine. We can iteratively refactor the old code as we work on relevant parts.

[kittaakos]

It would be great to prefix Theia classes with a t- instead of theia-. Something like t-tree-row, or t-search-box would be sufficient, I think. It is specific enough to avoid collisions, still not verbose, so you can debug it more comfortable.

[akosyakov]

We can consider to use css modules with webpack to avoid collisions : https://github.com/css-modules/css-modules then we don't need prefixes, webpack will ensure that class names are unique and not global. It will be possible to obfuscate and minimize names for production: https://develoger.com/how-to-obfuscate-css-class-names-with-react-and-webpack-20e2b5c49cda. To define classes we will need to use d.ts files instead of string constants. It can auto convert hyphen delimited to camel case names.

[kittaakos]

We decided to

• use the lower-case-with-dases,
• prefix them with theia when used a global classes, and
• update our coding-guidelines.

[kittaakos]

https://github.com/theia-ide/theia/wiki/Coding-Guidelines#css