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

Do you follow our CSS property order standard? #9

Closed
cfarm opened this issue Feb 18, 2015 · 29 comments
Closed

Do you follow our CSS property order standard? #9

cfarm opened this issue Feb 18, 2015 · 29 comments
Labels

Comments

@cfarm
Copy link
Contributor

cfarm commented Feb 18, 2015

If you write CSS, do you follow our existing CSS property order standards?

CITATION
Property Declaration Grouping and Order

.selector {
    /* Display & Box Model */
    display: inline-block;
    box-sizing: border-box;
    width: 100px;
    height: 100px;
    padding: 10px;
    border: 10px solid #333;
    margin: 10px;
    overflow: hidden;

    /* Positioning */
    position: absolute;
    top: 0;
    right: 0;
    bottom: 0;
    left: 0;
    z-index: 10;

    /* Background */
    background: #000 url('../img/bg.png') no-repeat center top;
    /* Shorthand, if appropriate. Otherwise, background-color 
     * before background-image (e.g., if using CSS3 gradients) */

    /* Foreground */
    /* Alphabetic in this group, with the exception of 
     * declaring line-height immediately after font-size */
    color: #fff;
    font-family: sans-serif;
    font-size: 16px;
    line-height: 1;
    font-style: italic;
    font-weight: bold;
    text-align: right;

    /* Miscellaneous, alphabetic */
    cursor: pointer;
    list-style: none;
}

If so, please share your thoughts on its efficacy.

If not, why not?

I'll start:
No, because I just learned about it, and it wasn't obvious to me when editing existing CSS that there was a property order being followed (despite having used a similar standard in the past).

@cfarm cfarm added the question label Feb 18, 2015
@ascott1
Copy link
Member

ascott1 commented Feb 18, 2015

No, because I just learned about it, and it wasn't obvious to me when editing existing CSS that there was a property order being followed.

@Scotchester
Copy link
Contributor

Yes, because I wrote it :)

@Scotchester
Copy link
Contributor

In general, these existence of these existing front-end style guides is not well known/remembered, so it wouldn't surprise me that much existing CSS doesn't obviously follow it.

@cfarm
Copy link
Contributor Author

cfarm commented Feb 18, 2015

The fact that it's on an internal site we can't link to from here makes it even less obvious! 😺

@Scotchester
Copy link
Contributor

Yeah... don't get me started on that...

@jimmynotjim
Copy link
Contributor

No, because I just learned it but follow a similar standard I've been using personally for years.

Question, why not use alpha-ordering within each group? The browser doesn't care what order they're in unless you're overwriting something (which you shouldn't be). Alpha makes finding a property in unfamiliar code so much easier.

@anselmbradford
Copy link
Member

This is really begging for a tool that takes a block of CSS, parses the properties and orders them. Ideally this is the kind of thing we have linters and such doing for us.

@wpears
Copy link
Member

wpears commented Feb 18, 2015

@jimmynotjim personally I prefer semantic grouping (like our current property order standards) to a-z because you don't have to read all the selectors to see all the relevant ones.

eg, if my layout is getting borked because of padding, it would have been more obvious if I was changing stuff in the box model section.

This sort of thing gets caught by linters and an individual rule probably shouldn't be so long that there is so much cognitive overhead that a-z ordering would cause a problem... but I still prefer semantic grouping

selfie-1

@wpears
Copy link
Member

wpears commented Feb 18, 2015

@anselmbradford I detect a grunt task in the making

@Scotchester
Copy link
Contributor

@wpears I think Jimmy meant within each grouping of properties.

@jimmynotjim The reason for this, in my opinion, is that certain properties lend themselves to a particular, non-alphabetic order.

The display property controls whether or not the rest of the box-model properties do anything/how they do things. box-sizing controls how the box model properties are applied.

It seems logical to me to have the box model go inside-out, so that's why I like

width
height
padding
border
margin

I'm not totally sure if overflow belongs in this group or not; that one seems like a mushy one. But that's where Idiomatic put it, so I left it. I moved it to the bottom because it describes what happens if you take the above box rules and try to break them with the content inside.

If this group were alphabetical, it'd be

border
box-sizing
display
height
margin
overflow
padding
width

Yes, it's easy to find a specific property, because things are alphabetical, but it just seems... "wrong" to me to not have it in a more "functional" (is that the word?) order. Does that make sense?

@jimmynotjim
Copy link
Contributor

It makes sense, but what I'm saying is the browser doesn't care. There's no such thing as semantics in CSS and having to stop and think "should this go before or after" is wasted time that's automated by alpha ordering. For example, shouldn't height be next to width in your example? They're similar controls against the box-model. Alpha-ordering within each group removes the need to even think about it and allows you to carry on with your work.

@KimberlyMunoz
Copy link
Contributor

Semantic grouping seems okay for things like the box model and positioning, but the other categories seem like a bit much, especially since sorting alphabetically already gives it some kind of order (with the exception of font-size -> line-height) and is much easier to remember.

Compare:

    /* Background */
    background: #000 url('../img/bg.png') no-repeat center top;
    /* Shorthand, if appropriate. Otherwise, background-color 
     * before background-image (e.g., if using CSS3 gradients) */

    /* Foreground */
    /* Alphabetic in this group, with the exception of 
     * declaring line-height immediately after font-size */
    color: #fff;
    font-family: sans-serif;
    font-size: 16px;
    line-height: 1;
    font-style: italic;
    font-weight: bold;
    text-align: right;

    /* Miscellaneous, alphabetic */
    cursor: pointer;
    list-style: none;

to

    /* Other */
    background: #000 url('../img/bg.png') no-repeat center top;
    color: #fff;
    cursor: pointer;
    font-family: sans-serif;
    font-size: 16px;
    font-style: italic;
    font-weight: bold;
    line-height: 1;
    list-style: none;
    text-align: right;

@anselmbradford
Copy link
Member

@wpears Yesss

As an aside what does everyone think about using inline comments instead of block level comments in Less? e.g.

// Background
background: #000 url('../img/bg.png') no-repeat center top;
// Shorthand, if appropriate. Otherwise, background-color 
// before background-image (e.g., if using CSS3 gradients)

Instead of...

/* Background */
background: #000 url('../img/bg.png') no-repeat center top;
/* Shorthand, if appropriate. Otherwise, background-color 
* before background-image (e.g., if using CSS3 gradients) */

I just find if I want to quickly comment out a block of code while working, being able to quickly wrap a large section in a block level comment is great, but existing block level comments interspersed in the code make that not possible.

I know I know you can just configure your editor to quickly comment out a block of code however you want, but this doesn't seem like a strong argument in favor of block level comments. Are there others?

Also, the asterisks look messy.

@Scotchester
Copy link
Contributor

I know the browser doesn't care. It's purely about how it reads to a developer.

I think you're overstating the amount of effort it takes, and possibly understating the amount of effort alphabetizing takes. It's certainly not "automatic" for some people :) Once you get used to the "semantic" order, it's second nature.

I think it comes down to two different goals and how to reconcile them both:

  1. Writing the CSS as fast as possible. For you and others, this might be alphabetical. For me, it's in a "logical" order by how the properties function.
  2. Reading down the properties on a declaration and quickly getting a mental picture of what's going on.

I'm okay being overruled on this. I just place a higher value on that second thing. I think with alphabetical properties, you have to think harder to pull all the pieces together in your mind.

@Scotchester
Copy link
Contributor

@KimberlyMunoz I think that's a reasonable concession.

@Scotchester
Copy link
Contributor

@anselmbradford Yes, when writing Less, I fully support usage the // syntax. The only problem is that if you have comments you want to remain after Less is compiled (e.g., in the unminified CSS), you have to do them with /* ... */.

@jimmynotjim
Copy link
Contributor

@Scotchester totally agree. I used to do it the semantic way but had no choice when working on an existing project a few years ago. I realized afterwards, there were far too many of those little moments that built up over time before (including discussions in PR reviews when it was "incorrect"). With Alpha there is no question, it goes where it goes.

It's also one of those things that no matter the choice, you'll get used to it and have the mental model just as easily.

@Scotchester
Copy link
Contributor

I understand. It's a trade I'd make -- spending that extra time for what I believe to be easier to grok CSS for developers coming behind -- but if the group as a whole disagrees, so be it.

@jimmynotjim
Copy link
Contributor

Ha, I think I'm the only one so... :)

@Scotchester
Copy link
Contributor

Just trying to state as many times as possible that I'm open minded, because having disagreeing discussion in a text medium is very hard to do without erroneously ascribing poor intentions/tone to your opponent :)

@mistergone
Copy link
Member

I don't know if I understand the debate. Are you guys debating what the final ordering should be when it goes to production, or what the ordering should be in the project CSS?

I'm assuming the latter... so I ask... can't we write grunt tasks that alphabetize and group the properties and then people can have their cake and also their jelly (or however that saying goes)? So, if you pull down the project, you can run grunt alphacss or grunt groupcss and then the files are the way you like them?

And then whoever cares about the ordering can write the grunt tasks! 😆 🚀 🚀

@Scotchester
Copy link
Contributor

Those tasks seem like a great idea, @mistergone, but it will majorly complicate things because the source files are checked into version control. If you use something like GitGutter in Sublime (and you should, because it's life-changing), then as soon as you reorder everything, it thinks every line of CSS has changed. It would be impossible to use GitGutter to see which lines have deviated from HEAD, because they ALL have (other than the selectors and blank lines). Furthermore, you'd have to remember to run the command to set it back to whatever canonical standard we have before committing. We can't have the property order of all our styles changing every time someone different commits to a repo.

@mistergone
Copy link
Member

@Scotchester All good points. That does make me think that first we should talk about git hooks that check for precisely these sorts of things on commits... because those would be a good idea, anyway (including maybe linting and definitely a check for console.log() so I don't have to make commits where I remove a console.log() and feel great shame). 😭 🐢 🍰

@anselmbradford
Copy link
Member

Along the lines of what @mistergone is saying, what about a jshint model that hints at what should be updated, but doesn't update the files directly?
I'm not sure on requiring git hooks, but having an easy to turn them on would be nice.

@Scotchester
Copy link
Contributor

Git hooks is a very interesting idea. We started implementing some in our standard configs (checking for committing of our GHE URL, primarily). I would not mind beefing those up at all.

If auto-hinting/linting (does anyone know what the difference is?) is possible for this sort of thing, that'd be a welcome addition, too.

@jimmynotjim
Copy link
Contributor

Looks like grunt-recess and grunt-lesslint are the main linters people are using (going by NPM downloads). Recess also seems to have a SublimeLinter plugin for doing auto-linting.

@ascott1
Copy link
Member

ascott1 commented Feb 19, 2015

I like the CSS linting discussion, but I'm going to move it to a new issue so it doesn't get lost.

@ascott1 ascott1 mentioned this issue Feb 19, 2015
@ascott1
Copy link
Member

ascott1 commented Jun 26, 2015

Related to #59.

@ascott1 ascott1 closed this as completed Jul 17, 2015
@KimberlyMunoz
Copy link
Contributor

Closed with @Scotchester's additional reasoning for CSS order in #59

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

No branches or pull requests

8 participants