Skip to content
This repository has been archived by the owner on Feb 15, 2020. It is now read-only.

Draft/css coding styleguide #15

Merged
merged 6 commits into from
Jul 28, 2015
Merged

Draft/css coding styleguide #15

merged 6 commits into from
Jul 28, 2015

Conversation

msecret
Copy link
Contributor

@msecret msecret commented Jun 10, 2015

Marco Segreto added 2 commits June 10, 2015 12:01
v0.0.1 - To be reviewed and commented on by front end guild team.
v0.1.0 For review and comment by full dev team.

Has been through comment and review by front end guild team.
TODO:
- Table of contents.
- Continued feedback from gov pattern library team.
- Collapse rationale sections?
}

@include media($sm) {
margin-top: $amount +10em;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a space after +:

margin-top: $amount + 10em;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should this be wrapped in parentheses?

@shawnbot
Copy link
Contributor

shawnbot commented Jul 7, 2015

One other thing re: expressions. I think we should state clearly in the style guide that spaces should separate values and operators in Sass expressions. For instance:

// good
foo {
  font-size: ($font-size + 2em);
}

// bad
foo {
  font-size: ($font-size+2em);
  font-size: ($font-size +2em);
}

I suppose that this is also an important way to disambiguate negative values from the minus operator, although I can't think of a good example.

3. @include directives
4. properties
- Within properties, you may use alphabetical order or type order—just pick one and keep the whole project consistent.
- Put a new line before nested selectors.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want a new line if you're nesting a selector (@extend or @include) directly after the first selector, right? That is, with no properties between them. If so, can we make this explicit?

// Good
.module {
  .module__ele {
    color: #fff932;
  }
}

// Bad
.module {

  .module__ele {
    color: #fff932;
  }
}

@shawnbot
Copy link
Contributor

I move to merge this and make changes via issues. @msecret, @meiqimichelle?

@meiqimichelle
Copy link
Contributor

Suggest we wait for @msecret to weigh in -- he tries to keep this github version in line with the google doc, where most of the commenting happens, but I'm not sure if this github version represents the latest work.

In general, I agree that we need to get the latest here, deprecate the google doc (now that we have a relatively stable version), and move to a changes-via-issues model.

@msecret
Copy link
Contributor Author

msecret commented Jul 22, 2015

@shawnbot I included a rule for correct spacing in expressions: e426616#diff-03e8e7d4493dc9cdf873d08611a76b50R91

I've already deprecated the doc and am ready to publish.

msecret pushed a commit that referenced this pull request Jul 28, 2015
@msecret msecret merged commit 6b699f5 into master Jul 28, 2015
@jeremiak jeremiak deleted the draft/css_coding_styleguide branch January 11, 2016 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants