Update our CSS style guide

[luixxiul]

Your use case

What would you like to do?

Conform CSS style rules of both element-web and matrix-react-sdk to our style guide further.

Why would you like to do it?

To improve maintainability. Currently the CSS style guide is partly conformed, and it should be adopted to our entire style rules codebase more.

How would you like to achieve it?

Check tsx files, confirm how style rules should be changed, implement Percy tests for possible scenarios, apply the change.

Have you considered any alternatives?

Dropping the style guide altogether should not be an alternative, of course :-D

Additional context

No response

[weeman1337]

Thank you for creating the issue @luixxiul . Can you refine the description to be more specific about what rules to implement from the style guide? Currently it looks to general/abstract to me.

[luixxiul]

Maybe I just do not really understand the guide right, but I thought the not many style rules did not follow the second rule:

Class names should denote the component which defines them, followed by any context:
    mx_MyFoo
    mx_MyFoo_avatar
    mx_MyFoo_avatar--user

For example, if you look at res/css/views/rooms/_RoomHeader.pcss, there are declarations whose selectors are like these:mx_RoomHeader_forgetButton, mx_RoomHeader_appsButton.

But if we strictly apply the rule, these should actually be as below: mx_RoomHeader_button--forget, mx_RoomHeader_button--apps (here), because these are variants defined by a component with the selector mx_RoomHeader_button.

These surely might be not natural English-wise ("forget button" and "apps button" should sound natural), but that should be how the selectors need to be written, as long as we stick to the style guide (hopefully this rule could be applied by a linter). I'd like to ask you to correct me if I get that wrong way :-)

[weeman1337]

Thank you for your explanation @luixxiul . The style guide is a bit vague on this point. Before actioning it, let me put the issue up for discussion with the web app team. I will get back to you here.

[luixxiul]

Following the guide, for example, mx_AccessibleButton should not be like mx_Button--accessible because we do not have mx_Button element which would work as the main button component.

On the other hand, if we are perfectly strict, mx_AccessibleButton_disabled can (and should) be replaced with mx_AccessibleButton--disabled as this case the disabled accessible button is the variant of the mx_AccessibleButton. Also mx_AccessibleButton_kind_primary_outline should also be mx_AccessibleButton_kind_primary--outline, for example.

[weeman1337]

@luixxiul I've just tried to answer you comment in the PR. But then you removed it. Now posting here:

Question: Does that mean that everyone can do what they want?

Basically yes. In this case, we think it is better to adapt the code style to the current code than to spend time developing a 100% correct system for naming CSS classes. It would be tricky for a linter to understand semantics. Means this would highly increase the effort for reviewers. Also Design Compound should fix this in the future™.

But we should keep the mx_Component rule as a basis.

[luixxiul]

I understood, thanks! Hopefully this will not create a conflict between a PR submitter and a reviewer, regarding the naming policy :crossed_fingers: