Commit 37707ac5 authored by Jacob Schatz's avatar Jacob Schatz

Merge branch 'css-style-guide' into 'master'

CSS style guide

Working towards what was discussed in #13552, this adds the [SCSS Linter gem](https://github.com/brigade/scss-lint) for style guide conformity in CI.

TODO:
- [x] Agree on and write SCSS Style Guide Documentation.
- [x] Document the `scss-lint` config file.
- [x] Figure out how best to run this in CI, right now it's taking longer than I would think it should.
- [x] Use CSSComb for auto-correction (Maybe just include a CSSComb config file and have developers run the node package manually if they're interested in using it?).

My logic for not using CSSComb in the first place is that, AFAIK, we don't currently require Node/NPM, even in the dev environment. Maybe I'm wrong about that? `scss-lint` is a Ruby implementation of a similar concept, but it doesn't include autocorrect.

Is there a way we can run get GitLab CI to run `scss-lint` without having to put together the full GitLab application? Seeing as it's just static analysis, it seems like a waste of time/resources.

/cc @jschatz1 @rspeicher

See merge request !3069
parents 0602091f 8d8b457c
{
"always-semicolon": true,
"color-case": "lower",
"block-indent": " ",
"color-shorthand": true,
"element-case": "lower",
"space-before-colon": "",
"space-after-colon": " ",
"space-before-combinator": " ",
"space-after-combinator": " ",
"space-between-declarations": "\n",
"space-before-opening-brace": " ",
"space-after-opening-brace": "\n",
"space-before-closing-brace": "\n",
"unitless-zero": true
}
...@@ -122,6 +122,14 @@ rubocop: ...@@ -122,6 +122,14 @@ rubocop:
- ruby - ruby
- mysql - mysql
scss-lint:
stage: test
script:
- bundle exec rake scss_lint
tags:
- ruby
allow_failure: true
brakeman: brakeman:
stage: test stage: test
script: script:
......
# Linter Documentation:
# https://github.com/brigade/scss-lint/blob/master/lib/scss_lint/linter/README.md
scss_files: 'app/assets/stylesheets/**/*.scss'
exclude:
- 'app/assets/stylesheets/pages/emojis.scss'
linters:
BangFormat:
enabled: false
BorderZero:
enabled: false
ColorKeyword:
enabled: false
ColorVariable:
enabled: false
Comment:
enabled: false
DeclarationOrder:
enabled: false
# `scss-lint:disable` control comments should be preceded by a comment
# explaining why these linters are being disabled for this file.
# See https://github.com/brigade/scss-lint#disabling-linters-via-source for
# more information.
DisableLinterReason:
enabled: true
DuplicateProperty:
enabled: false
EmptyLineBetweenBlocks:
enabled: false
EmptyRule:
enabled: false
FinalNewline:
enabled: false
# HEX colors should use three-character values where possible.
HexLength:
enabled: true
# HEX color values should use lower-case colors to differentiate between
# letters and numbers, e.g. `#E3E3E3` vs. `#e3e3e3`.
HexNotation:
enabled: true
IdSelector:
enabled: false
ImportPath:
enabled: false
ImportantRule:
enabled: false
# Indentation should always be done in increments of 2 spaces.
Indentation:
enabled: true
width: 2
LeadingZero:
enabled: false
MergeableSelector:
enabled: false
NameFormat:
enabled: false
NestingDepth:
enabled: false
PlaceholderInExtend:
enabled: false
PropertySortOrder:
enabled: false
PropertySpelling:
enabled: false
PseudoElement:
enabled: false
QualifyingElement:
enabled: false
SelectorDepth:
enabled: false
# Selectors should always use hyphenated-lowercase, rather than camelCase or
# snake_case.
SelectorFormat:
enabled: true
convention: hyphenated_lowercase
# Prefer the shortest shorthand form possible for properties that support it.
Shorthand:
enabled: true
# Each property should have its own line, except in the special case of
# single line rulesets.
SingleLinePerProperty:
enabled: true
allow_single_line_rule_sets: true
SingleLinePerSelector:
enabled: false
SpaceAfterComma:
enabled: false
# Properties should be formatted with a single space separating the colon
# from the property's value.
SpaceAfterPropertyColon:
enabled: true
# Properties should be formatted with no space between the name and the
# colon.
SpaceAfterPropertyName:
enabled: true
SpaceAroundOperator:
enabled: false
SpaceBeforeBrace:
enabled: false
StringQuotes:
enabled: false
TrailingSemicolon:
enabled: false
TrailingWhitespace:
enabled: false
UnnecessaryMantissa:
enabled: false
UnnecessaryParentReference:
enabled: false
VendorPrefix:
enabled: false
# Omit length units on zero values, e.g. `0px` vs. `0`.
ZeroUnit:
enabled: true
...@@ -427,6 +427,7 @@ merge request: ...@@ -427,6 +427,7 @@ merge request:
1. [Rails](https://github.com/bbatsov/rails-style-guide) 1. [Rails](https://github.com/bbatsov/rails-style-guide)
1. [Testing](https://github.com/thoughtbot/guides/tree/master/style/testing) 1. [Testing](https://github.com/thoughtbot/guides/tree/master/style/testing)
1. [CoffeeScript](https://github.com/thoughtbot/guides/tree/master/style/coffeescript) 1. [CoffeeScript](https://github.com/thoughtbot/guides/tree/master/style/coffeescript)
1. [SCSS styleguide][scss-styleguide]
1. [Shell commands](doc/development/shell_commands.md) created by GitLab 1. [Shell commands](doc/development/shell_commands.md) created by GitLab
contributors to enhance security contributors to enhance security
1. [Database Migrations](doc/development/migration_style_guide.md) 1. [Database Migrations](doc/development/migration_style_guide.md)
...@@ -494,6 +495,7 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor ...@@ -494,6 +495,7 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor
[rss-source]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#source-code-layout [rss-source]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#source-code-layout
[rss-naming]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#naming [rss-naming]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#naming
[doc-styleguide]: doc/development/doc_styleguide.md "Documentation styleguide" [doc-styleguide]: doc/development/doc_styleguide.md "Documentation styleguide"
[scss-styleguide]: doc/development/scss_styleguide.md "SCSS styleguide"
[gitlab-design]: https://gitlab.com/gitlab-org/gitlab-design [gitlab-design]: https://gitlab.com/gitlab-org/gitlab-design
[free Antetype viewer (Mac OSX only)]: https://itunes.apple.com/us/app/antetype-viewer/id824152298?mt=12 [free Antetype viewer (Mac OSX only)]: https://itunes.apple.com/us/app/antetype-viewer/id824152298?mt=12
[`gitlab1.atype` file]: https://gitlab.com/gitlab-org/gitlab-design/tree/master/gitlab1.atype/ [`gitlab1.atype` file]: https://gitlab.com/gitlab-org/gitlab-design/tree/master/gitlab1.atype/
......
...@@ -283,6 +283,7 @@ group :development, :test do ...@@ -283,6 +283,7 @@ group :development, :test do
gem 'spring-commands-teaspoon', '~> 0.0.2' gem 'spring-commands-teaspoon', '~> 0.0.2'
gem 'rubocop', '~> 0.35.0', require: false gem 'rubocop', '~> 0.35.0', require: false
gem 'scss_lint', '~> 0.47.0', require: false
gem 'coveralls', '~> 0.8.2', require: false gem 'coveralls', '~> 0.8.2', require: false
gem 'simplecov', '~> 0.10.0', require: false gem 'simplecov', '~> 0.10.0', require: false
gem 'flog', require: false gem 'flog', require: false
......
...@@ -717,6 +717,9 @@ GEM ...@@ -717,6 +717,9 @@ GEM
sawyer (0.6.0) sawyer (0.6.0)
addressable (~> 2.3.5) addressable (~> 2.3.5)
faraday (~> 0.8, < 0.10) faraday (~> 0.8, < 0.10)
scss_lint (0.47.1)
rake (>= 0.9, < 11)
sass (~> 3.4.15)
sdoc (0.3.20) sdoc (0.3.20)
json (>= 1.1.3) json (>= 1.1.3)
rdoc (~> 3.10) rdoc (~> 3.10)
...@@ -1007,6 +1010,7 @@ DEPENDENCIES ...@@ -1007,6 +1010,7 @@ DEPENDENCIES
ruby-fogbugz (~> 0.2.1) ruby-fogbugz (~> 0.2.1)
sanitize (~> 2.0) sanitize (~> 2.0)
sass-rails (~> 5.0.0) sass-rails (~> 5.0.0)
scss_lint (~> 0.47.0)
sdoc (~> 0.3.20) sdoc (~> 0.3.20)
seed-fu (~> 2.3.5) seed-fu (~> 2.3.5)
select2-rails (~> 3.5.9) select2-rails (~> 3.5.9)
......
# SCSS styleguide
This style guide recommends best practices for SCSS to make styles easy to read,
easy to maintain, and performant for the end-user.
## Rules
### Naming
CSS classes should use the `lowercase-hyphenated` format rather than
`snake_case` or `camelCase`.
```scss
// Bad
.class_name {
color: #fff;
}
// Bad
.className {
color: #fff;
}
// Good
.class-name {
color: #fff;
}
```
### Formatting
You should always use a space before a brace, braces should be on the same
line, each property should each get its own line, and there should be a space
between the property and its value.
```scss
// Bad
.container-item {
width: 100px; height: 100px;
margin-top: 0;
}
// Bad
.container-item
{
width: 100px;
height: 100px;
margin-top: 0;
}
// Bad
.container-item{
width:100px;
height:100px;
margin-top:0;
}
// Good
.container-item {
width: 100px;
height: 100px;
margin-top: 0;
}
```
Note that there is an exception for single-line rulesets, although these are
not typically recommended.
```scss
p { margin: 0; padding: 0; }
```
### Colors
HEX (hexadecimal) colors short-form should use shortform where possible, and
should use lower case letters to differenciate between letters and numbers, e.
g. `#E3E3E3` vs. `#e3e3e3`.
```scss
// Bad
p {
color: #ffffff;
}
// Bad
p {
color: #FFFFFF;
}
// Good
p {
color: #fff;
}
```
### Indentation
Indentation should always use two spaces for each indentation level.
```scss
// Bad, four spaces
p {
color: #f00;
}
// Good
p {
color: #f00;
}
```
### Semicolons
Always include semicolons after every property. When the stylesheets are
minified, the semicolons will be removed automatically.
```scss
// Bad
.container-item {
width: 100px;
height: 100px
}
// Good
.container-item {
width: 100px;
height: 100px;
}
```
### Shorthand
The shorthand form should be used for properties that support it.
```scss
// Bad
margin: 10px 15px 10px 15px;
padding: 10px 10px 10px 10px;
// Good
margin: 10px 15px;
padding: 10px;
```
### Zero Units
Omit length units on zero values, they're unnecessary and not including them
is slightly more performant.
```scss
// Bad
.item-with-padding {
padding: 0px;
}
// Good
.item-with-padding {
padding: 0;
}
```
### Selectors with a `js-` Prefix
Do not use any selector prefixed with `js-` for styling purposes. These
selectors are intended for use only with JavaScript to allow for removal or
renaming without breaking styling.
## Linting
We use [SCSS Lint][scss-lint] to check for style guide conformity. It uses the
ruleset in `.scss-lint.yml`, which is located in the home directory of the
project.
To check if any warnings will be produced by your changes, you can run `rake
scss_lint` in the GitLab directory. SCSS Lint will also run in GitLab CI to
catch any warnings.
If the Rake task is throwing warnings you don't understand, SCSS Lint's
documentation includes [a full list of their linters][scss-lint-documentation].
### Fixing issues
If you want to automate changing a large portion of the codebase to conform to
the SCSS style guide, you can use [CSSComb][csscomb]. First install
[Node][node] and [NPM][npm], then run `npm install csscomb -g` to install
CSSComb globally (system-wide). Run it in the GitLab directory with
`csscomb app/assets/stylesheets` to automatically fix issues with CSS/SCSS.
Note that this won't fix every problem, but it should fix a majority.
[csscomb]: https://github.com/csscomb/csscomb.js
[node]: https://github.com/nodejs/node
[npm]: https://www.npmjs.com/
[scss-lint]: https://github.com/brigade/scss-lint
[scss-lint-documentation]: https://github.com/brigade/scss-lint/blob/master/lib/scss_lint/linter/README.md
unless Rails.env.production?
require 'scss_lint/rake_task'
SCSSLint::RakeTask.new do |t|
t.config = '.scss-lint.yml'
# See https://github.com/brigade/scss-lint/issues/726
# Hack, otherwise linter won't respect scss_files option in config file.
t.files = []
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment