diff --git a/.csscomb.json b/.csscomb.json new file mode 100644 index 00000000000..e353e6a63d0 --- /dev/null +++ b/.csscomb.json @@ -0,0 +1,16 @@ +{ + "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 +} diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b4dad768fa4..2ad63548d78 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -122,6 +122,14 @@ rubocop: - ruby - mysql +scss-lint: + stage: test + script: + - bundle exec rake scss_lint + tags: + - ruby + allow_failure: true + brakeman: stage: test script: diff --git a/.scss-lint.yml b/.scss-lint.yml new file mode 100644 index 00000000000..e350b2073c3 --- /dev/null +++ b/.scss-lint.yml @@ -0,0 +1,158 @@ +# 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 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 30c97429040..7540fa1afcc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -427,6 +427,7 @@ merge request: 1. [Rails](https://github.com/bbatsov/rails-style-guide) 1. [Testing](https://github.com/thoughtbot/guides/tree/master/style/testing) 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 contributors to enhance security 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 [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 [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 [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/ diff --git a/Gemfile b/Gemfile index 2f14b9965b2..a0e8e796627 100644 --- a/Gemfile +++ b/Gemfile @@ -283,6 +283,7 @@ group :development, :test do gem 'spring-commands-teaspoon', '~> 0.0.2' gem 'rubocop', '~> 0.35.0', require: false + gem 'scss_lint', '~> 0.47.0', require: false gem 'coveralls', '~> 0.8.2', require: false gem 'simplecov', '~> 0.10.0', require: false gem 'flog', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 14b03bab5dd..f4f5649eb75 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -717,6 +717,9 @@ GEM sawyer (0.6.0) addressable (~> 2.3.5) faraday (~> 0.8, < 0.10) + scss_lint (0.47.1) + rake (>= 0.9, < 11) + sass (~> 3.4.15) sdoc (0.3.20) json (>= 1.1.3) rdoc (~> 3.10) @@ -1007,6 +1010,7 @@ DEPENDENCIES ruby-fogbugz (~> 0.2.1) sanitize (~> 2.0) sass-rails (~> 5.0.0) + scss_lint (~> 0.47.0) sdoc (~> 0.3.20) seed-fu (~> 2.3.5) select2-rails (~> 3.5.9) diff --git a/doc/development/scss_styleguide.md b/doc/development/scss_styleguide.md new file mode 100644 index 00000000000..6c48c25448b --- /dev/null +++ b/doc/development/scss_styleguide.md @@ -0,0 +1,194 @@ +# 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 diff --git a/lib/tasks/scss-lint.rake b/lib/tasks/scss-lint.rake new file mode 100644 index 00000000000..250fd8699e4 --- /dev/null +++ b/lib/tasks/scss-lint.rake @@ -0,0 +1,10 @@ +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