Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS Coding Standards style.css file #503

Closed
mkrdip opened this issue May 29, 2014 · 6 comments
Closed

CSS Coding Standards style.css file #503

mkrdip opened this issue May 29, 2014 · 6 comments

Comments

@mkrdip
Copy link

mkrdip commented May 29, 2014

Each selector should be on its own line. One example:

ol, ul {
list-style: none;
}

Should be:

ol, 
ul {
list-style: none;
}

Reference: http://make.wordpress.org/core/handbook/coding-standards/css/

mkrdip added a commit to mkrdip/_s that referenced this issue May 29, 2014
According to http://make.wordpress.org/core/handbook/coding-standards/css/ 
Each selector should be on its own line

One example:
```
ol, ul {
list-style: none;
}
```
Should be:
```
ol, 
ul {
list-style: none;
}


Automattic#503
@SeanTOSCD
Copy link
Contributor

That's been talked about a few times before and passed on because it's a little bit of a preference issue, though WordPress has a desired standard. I like to handle this on a case-by-case basis. I'm not putting h1 through h6 on separate lines in my own CSS. No way. I will do that for most selectors, though. @philiparthurmoore feels differently. #286

My personal opinion is that the selector structure should be looked at case-by-case in _s and groups of really short selectors, like in the reset, should remain as they are. Not really for me, but because I think it's a simpler starting point for everyone in general.

@philiparthurmoore
Copy link
Collaborator

My thoughts on this have changed slightly. In most cases I think that selectors should be on their own lines. This is because I heavily rely on diff's to know what has happened to code.

This...

#a, #b, #c, #d, #e, #f, #g, #h, #i, #j, #k, #l, #m, #n, #o {
    background: black;
}

Becomes an annoying headache when a code diff comes in and I have to search what's been changed, for example...

#a, #b, #c, #d, #f, #g, #h, #i, #j, #z, #k, #l, #m, #n, #o {
    background: black;
}

If all of those selectors were on an individual line, it would be much, much easier to maintain the codebase given the number of contributions and input from external sources. Also, if CSS is being minified in production, why is it so bad to have it, without exception, on separate lines? This includes resets.

Too often we play the "preference" card or "make your own fork" card when shutting down discussions of standards or enhancements within _s without actually giving compelling arguments about why they are the way they are, and if the way they are is correct, and not simply correct due to "that's how it's always been done" or "that's how it's done in the default themes". I think that it's a good practice to revisit these issues normally, because things change.

That said, if resets are kept bunched up, then the least that should happen to them is that they are grouped thematically, which _s seems to currently do fairly well.

That is to say this...

dl, dt, dd, h4, h5, h6, ol, ul, li, h1, h2, h3 { }

...looks terrible, while this...

h1, h2, h3, h4, h5, h6, dl, dt, dd, ol, ul, li { }

...does not.

This, to me, looks bad:

dfn, cite, em, i {
    font-style: italic;
}

And so does this:

code, kbd, tt, var {
    font: 15px Monaco, Consolas, "Andale Mono", "DejaVu Sans Mono", monospace;
}

And so does this:

h1, h2, h3, h4, h5, h6 {
    clear: both;
}

This is largely a matter of preference, yes, so I guess the only thing I can say from personal experience of reading line after line of code is that when the flow of code reading is disrupted by my eyes having to switch from non-separate-lines to separate-lines, I'm thrown off and often make mistakes. Knowing what to expect in the code I'm reading, under all circumstances, minimizes those mistakes.

@philiparthurmoore
Copy link
Collaborator

Too often we play the "preference" card or "make your own fork" card when shutting down discussions of standards or enhancements within _s without actually giving compelling arguments about why they are the way they are, and if the way they are is correct, and not simply correct due to "that's how it's always been done" or "that's how it's done in the default themes". I think that it's a good practice to revisit these issues normally, because things change.

That "we" includes me, by the way. :)

@SeanTOSCD
Copy link
Contributor

It sounds to me like we said the same exact thing lol.

I agree 100% that "appeal to tradition" is one of the worst logical fallacies any of us could commit in this context as standards, needs, and circumstances develop over time. What can be just as illogical, though, is making changes just to avoid the appearance or feeling of developing a tradition. _s being a starter theme for all with no exact usage standards is the main thing here, from what I've gathered watching you all develop the code over the last year or so. Many things have changed and many more things should/will change. However, with this particular "issue" I think every single one of us will be hard-pressed to come up with a reason that is not personal preference without implying exactly how _s should be used once a random project's development has begun. And if the floor is open for those kinds of implications, there are a number of conflicting changes we can all bring to the table, I'm sure.

@philiparthurmoore
Copy link
Collaborator

Extremely well-stated, Sean.

@obenland
Copy link
Member

Extremely well-stated, Sean.

Indeed.

_s strives to be as compliant with WordPress coding standards as possible. In many places we even go above and beyond of what is suggested.

In this particular case we have to acknowledge that all of the affected styles come from outside projects. We use a …unique… mix of Eric Meyer's reset, Normalize.css, and even Blueprint, that don't follow the WordPress guidelines. If we were only using SASS, this would be a non issue because we'd treat that part as an external library and just pull it into our compiled stylesheet.

But that doesn't mean we can't have the same mindset without using SASS. At some point I'd like to see us moving to Normalize.css only (#3, #44, #174, #267). That would help for this issue by being a self-contained part of style.css that we could exclude from the CSS coding guidelines.

For now I'd suggest to think of it as an external library and not change it though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants