Update CSS coding standards to adapt to the Stylelint configuration. - #162
Update CSS coding standards to adapt to the Stylelint configuration.#162afercia wants to merge 4 commits into
Conversation
|
Hi @afercia Thanks for this PR. This will need a Make post as it changes the guidelines and something like the 1 vs 2 blank lines change may seem like a small change, but it will cause code churn in the current codebase, as well as invalidate patches/cause extra work for committers to update pre-existing patches before they can be committed. Also, no need to add a changelog entry. The changelog is not used and there is a proposal open to remove it. See #67. |
|
Hi @jrfnl |
|
A note about a small mistake in the current docs:
is used as example twice. In the first example snippet is indicated as 'correct'. while in the second example snippet is indicated (rightly) as 'incorrect' with the explicit explanation: |
I don't think that change is correct. The way I read it, the correct versus incorrect is about the word "text" being between double quotes (correct) or without quotes (incorrect). It has nothing to do with the AFAICS, this is an example to demonstrate the following rule listed above it:
|
Yes, it's in an example for another rule. Still, it's very confusing. The same selector is used in a 'correct' example and then in an 'incorrect' example. I'm not sure this helps clarity for users. That said, right now there are 1815 violations of 'over-qualified selectors' in Core so... 🤷🏻 |
| - Other | ||
|
|
||
| Things that are not yet used in core itself, such as CSS3 animations, may not have a prescribed place above but likely would fit into one of the above in a logical manner. Just as CSS is evolving, so our standards will evolve with it. | ||
| Things that are not yet used in core itself may not have a prescribed place above but likely would fit into one of the above in a logical manner. Just as CSS is evolving, so our standards will evolve with it. |
There was a problem hiding this comment.
Removed such as CSS3 animations, because they are now used in Core.
|
|
||
| - It is generally advisable to keep media queries grouped by media at the bottom of the stylesheet. | ||
| - An exception is made for the `wp-admin.css` file in core, as it is very large and each section essentially represents a stylesheet of its own. Media queries are therefore added at the bottom of sections as applicable. | ||
| - Rule sets for media queries should be indented one level in. |
There was a problem hiding this comment.
Removed all this point because wp-admin.css is no longer a huge file split into sections so that the reference to this as an exception doesn't make sense any longer.
Hi @jrfnl |
@afercia I appreciate the ask, but I currently don't have the bandwidth to review such a post. Maybe @GaryJones or @ntwb can help ? |
2 participants
In https://core.trac.wordpress.org/ticket/29792 / WordPress/wordpress-develop#12934 I'm proposing to add Stylelint configuration and scripts to Core to enforce the CSS coding standards.
The work there is almost done. However, some points of the current CSS coding standards either can't be enforced via Stylelint of hare too difficult to enforce.
As such, I'm proposing a few changes. I will also discuss this with the involved Core teams and consider to publish a post on Make, if necessary.
The most important change is about the class selector pattern. I'm proposing to use the BEM-based naming convention because:
stylelint-disablecomment.Other changes are very minor, for example standardize the required number of empty lines to 1. Stylelint isn't abel to apply this rule conditionally to section comments which currently require 2 empty lines. After all, having 2 lines before a section comment doesn't add much value and in my opinion it's best to have a tool that can apply reliably 1 empty line ratehr than requiring exceptions that can't be linted.