Commit 40777636 authored by Tristan Read's avatar Tristan Read Committed by Marcia Ramos

Combine frontend style guides

This is part of a wider effort to consolidate
frontend development guides. Issue here:
https://gitlab.com/gitlab-org/frontend/general/issues/25
parent 9c6bcdc0
...@@ -40,8 +40,8 @@ This saves you time as you don't have to wait for the same errors to be detected ...@@ -40,8 +40,8 @@ This saves you time as you don't have to wait for the same errors to be detected
[rss-source]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#source-code-layout [rss-source]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#source-code-layout
[rss-naming]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#naming-conventions [rss-naming]: https://github.com/rubocop-hq/ruby-style-guide/blob/master/README.adoc#naming-conventions
[doc-guidelines]: ../documentation/index.md "Documentation guidelines" [doc-guidelines]: ../documentation/index.md "Documentation guidelines"
[js-styleguide]: ../fe_guide/style_guide_js.md "JavaScript styleguide" [js-styleguide]: ../fe_guide/style/javascript.md "JavaScript styleguide"
[scss-styleguide]: ../fe_guide/style_guide_scss.md "SCSS styleguide" [scss-styleguide]: ../fe_guide/style/scss.md "SCSS styleguide"
[newlines-styleguide]: ../newlines_styleguide.md "Newlines styleguide" [newlines-styleguide]: ../newlines_styleguide.md "Newlines styleguide"
[testing]: ../testing_guide/index.md [testing]: ../testing_guide/index.md
[us-english]: https://en.wikipedia.org/wiki/American_English [us-english]: https://en.wikipedia.org/wiki/American_English
...@@ -76,15 +76,18 @@ Read the [frontend's FAQ](frontend_faq.md) for common small pieces of helpful in ...@@ -76,15 +76,18 @@ Read the [frontend's FAQ](frontend_faq.md) for common small pieces of helpful in
## Style Guides ## Style Guides
### [JavaScript Style Guide](style_guide_js.md) See the relevant style guides for our guidelines and for information on linting:
We use eslint to enforce our JavaScript style guides. Our guide is based on - [JavaScript](style/javascript.md). Our guide is based on
the excellent [Airbnb][airbnb-js-style-guide] style guide with a few small the excellent [Airbnb][airbnb-js-style-guide] style guide with a few small
changes. changes.
- [SCSS](style/scss.md): our SCSS conventions which are enforced through [`scss-lint`](https://github.com/brigade/scss-lint).
- [HTML](style/html.md). Guidelines for writing HTML code consistent with the rest of the codebase.
- [Vue](style/vue.md). Guidelines and conventions for Vue code may be found here.
### [SCSS Style Guide](style_guide_scss.md) ## Tooling
Our SCSS conventions which are enforced through [scss-lint](https://github.com/sds/scss-lint). Our code is automatically formatted with [Prettier](https://prettier.io) to follow our guidelines. Read our [Tooling guide](tooling.md) for more detail.
## [Performance](performance.md) ## [Performance](performance.md)
......
# HTML style guide
## Buttons
### Button type
Button tags requires a `type` attribute according to the [W3C HTML specification](https://www.w3.org/TR/2011/WD-html5-20110525/the-button-element.html#dom-button-type).
```html
// bad
<button></button>
// good
<button type="button"></button>
```
### Button role
If an HTML element has an `onClick` handler but is not a button, it should have `role="button"`. This is [more accessible](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role).
```html
// bad
<div onClick="doSomething"></div>
// good
<div role="button" onClick="doSomething"></div>
```
## Links
### Blank target
Use `rel="noopener noreferrer"` whenever your links open in a new window, i.e. `target="_blank"`. This prevents a security vulnerability [documented by JitBit](https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/).
```html
// bad
<a href="url" target="_blank"></a>
// good
<a href="url" target="_blank" rel="noopener noreferrer"></a>
```
### Fake links
**Do not use fake links.** Use a button tag if a link only invokes JavaScript click event handlers, which is more semantic.
```html
// bad
<a class="js-do-something" href="#"></a>
// good
<button class="js-do-something" type="button"></button>
```
# GitLab development style guides
See below the relevant style guides, guidelines, linting, and other information for developing GitLab.
## JavaScript style guide
We use `eslint` to enforce our [JavaScript style guides](javascript.md). Our guide is based on
the excellent [AirBnB](https://github.com/airbnb/javascript) style guide with a few small
changes.
## SCSS style guide
Our [SCSS conventions](scss.md) which are enforced through [`scss-lint`](https://github.com/brigade/scss-lint).
## HTML style guide
Guidelines for writing [HTML code](html.md) consistent with the rest of the codebase.
## Vue style guide
Guidelines and conventions for Vue code may be found within the [Vue style guide](vue.md).
---
disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_js.html'
---
# JavaScript style guide
We use [Airbnb's JavaScript Style Guide](https://github.com/airbnb/javascript) and it's accompanying
linter to manage most of our JavaScript style guidelines.
In addition to the style guidelines set by Airbnb, we also have a few specific rules
listed below.
> **Tip:**
You can run eslint locally by running `yarn eslint`
## Avoid forEach
Avoid forEach when mutating data. Use `map`, `reduce` or `filter` instead of `forEach`
when mutating data. This will minimize mutations in functions,
which aligns with [Airbnb's style guide](https://github.com/airbnb/javascript#testing--for-real).
```javascript
// bad
users.forEach((user, index) => {
user.id = index;
});
// good
const usersWithId = users.map((user, index) => {
return Object.assign({}, user, { id: index });
});
```
## Limit number of parameters
If your function or method has more than 3 parameters, use an object as a parameter
instead.
```javascript
// bad
function a(p1, p2, p3) {
// ...
};
// good
function a(p) {
// ...
};
```
## Avoid side effects in constructors
Avoid making asynchronous calls, API requests or DOM manipulations in the `constructor`.
Move them into separate functions instead. This will make tests easier to write and
code easier to maintain.
```javascript
// bad
class myClass {
constructor(config) {
this.config = config;
axios.get(this.config.endpoint)
}
}
// good
class myClass {
constructor(config) {
this.config = config;
}
makeRequest() {
axios.get(this.config.endpoint)
}
}
const instance = new myClass();
instance.makeRequest();
```
## Avoid classes to handle DOM events
If the only purpose of the class is to bind a DOM event and handle the callback, prefer
using a function.
```javascript
// bad
class myClass {
constructor(config) {
this.config = config;
}
init() {
document.addEventListener('click', () => {});
}
}
// good
const myFunction = () => {
document.addEventListener('click', () => {
// handle callback here
});
}
```
## Pass element container to constructor
When your class manipulates the DOM, receive the element container as a parameter.
This is more maintainable and performant.
```javascript
// bad
class a {
constructor() {
document.querySelector('.b');
}
}
// good
class a {
constructor(options) {
options.container.querySelector('.b');
}
}
```
## Use ParseInt
Use `ParseInt` when converting a numeric string into a number.
```javascript
// bad
Number('10')
// good
parseInt('10', 10);
```
## CSS Selectors - Use `js-` prefix
If a CSS class is only being used in JavaScript as a reference to the element, prefix
the class name with `js-`.
```html
// bad
<button class="add-user"></button>
// good
<button class="js-add-user"></button>
```
## ES Module Syntax
Use ES module syntax to import modules:
```javascript
// bad
const SomeClass = require('some_class');
// good
import SomeClass from 'some_class';
// bad
module.exports = SomeClass;
// good
export default SomeClass;
```
_Note:_ We still use `require` in `scripts/` and `config/` files.
## Absolute vs relative paths for modules
Use relative paths if the module you are importing is less than two levels up.
```javascript
// bad
import GitLabStyleGuide from '~/guides/GitLabStyleGuide';
// good
import GitLabStyleGuide from '../GitLabStyleGuide';
```
If the module you are importing is two or more levels up, use an absolute path instead:
```javascript
// bad
import GitLabStyleGuide from '../../../guides/GitLabStyleGuide';
// good
import GitLabStyleGuide from '~/GitLabStyleGuide';
```
Additionally, **do not add to global namespace**.
## Do not use `DOMContentLoaded` in non-page modules
Imported modules should act the same each time they are loaded. `DOMContentLoaded`
events are only allowed on modules loaded in the `/pages/*` directory because those
are loaded dynamically with webpack.
## Avoid XSS
Do not use `innerHTML`, `append()` or `html()` to set content. It opens up too many
vulnerabilities.
## ESLint
ESLint behaviour can be found in our [tooling guide](../tooling.md).
## IIFEs
Avoid using IIFEs (Immediately-Invoked Function Expressions). Although
we have a lot of examples of files which wrap their contents in IIFEs,
this is no longer necessary after the transition from Sprockets to webpack.
Do not use them anymore and feel free to remove them when refactoring legacy code.
## Global namespace and side effects
Avoid adding to the global namespace.
```javascript
// bad
window.MyClass = class { /* ... */ };
// good
export default class MyClass { /* ... */ }
```
Top-level side effects are forbidden in any script which contains `export`:
```javascript
// bad
export default class MyClass { /* ... */ }
document.addEventListener("DOMContentLoaded", function(event) {
new MyClass();
}
```
Avoid constructors with side effects whenever possible.
Side effects make constructors difficult to unit test and violate the [Single Responsibility Principle](https://en.wikipedia.org/wiki/Single_responsibility_principle).
```javascript
// Bad
export class Foo {
constructor() {
this.currentState = state.INITIAL;
document.getElementById('root').addEventListener('click', this.handleCallback)
}
handleCallback() {
}
}
// Good
export class Foo {
constructor() {
this.currentState = state.INITIAL;
}
initListener(element) {
element.addEventListener('click', this.handleCallback)
}
handleCallback() {
}
}
```
On the other hand, if a class only needs to extend a third-party or add
event listeners in some specific cases, they should be initialized outside
of the constructor.
## Pure Functions and Data Mutation
Strive to write many small pure functions and minimize where mutations occur
```javascript
// bad
const values = {foo: 1};
function impureFunction(items) {
const bar = 1;
items.foo = items.a * bar + 2;
return items.a;
}
const c = impureFunction(values);
// good
var values = {foo: 1};
function pureFunction (foo) {
var bar = 1;
foo = foo * bar + 2;
return foo;
}
var c = pureFunction(values.foo);
```
---
disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_scss.html'
---
# SCSS style guide
This style guide recommends best practices for SCSS to make styles easy to read,
easy to maintain, and performant for the end-user.
## Rules
### Utility Classes
As part of the effort for [cleaning up our CSS and moving our components into GitLab-UI](https://gitlab.com/groups/gitlab-org/-/epics/950)
led by the [GitLab UI WG](https://gitlab.com/gitlab-com/www-gitlab-com/merge_requests/20623) we prefer the use of utility classes over adding new CSS. However, complex CSS can be addressed by adding component classes.
#### Where are utility classes defined?
- [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/)
- [`common.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/framework/common.scss) (old)
- [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss) (new)
#### Where should I put new utility classes?
New utility classes should be added to [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss). Existing classes include:
| Name | Pattern | Example |
|------|---------|---------|
| Background color | `.bg-{variant}-{shade}` | `.bg-warning-400` |
| Text color | `.text-{variant}-{shade}` | `.text-success-500` |
| Font size | `.text-{size}` | `.text-2` |
- `{variant}` is one of 'primary', 'secondary', 'success', 'warning', 'error'
- `{shade}` is one of the shades listed on [colors](https://design.gitlab.com/product-foundations/colors/)
- `{size}` is a number from 1-6 from our [Type scale](https://design.gitlab.com/product-foundations/typography/)
#### When should I create component classes?
We recommend a "utility-first" approach.
1. Start with utility classes.
1. If composing utility classes into a component class removes code duplication and encapsulates a clear responsibility, do it.
This encourages an organic growth of component classes and prevents the creation of one-off unreusable classes. Also, the kind of classes that emerge from "utility-first" tend to be design-centered (e.g. `.button`, `.alert`, `.card`) rather than domain-centered (e.g. `.security-report-widget`, `.commit-header-icon`).
Examples of component classes that were created using "utility-first" include:
- [`.circle-icon-container`](https://gitlab.com/gitlab-org/gitlab/blob/579fa8b8ec7eb38d40c96521f517c9dab8c3b97a/app/assets/stylesheets/framework/icons.scss#L85)
- [`.d-flex-center`](https://gitlab.com/gitlab-org/gitlab/blob/900083d89cd6af391d26ab7922b3f64fa2839bef/app/assets/stylesheets/framework/common.scss#L425)
Inspiration:
- <https://tailwindcss.com/docs/utility-first/>
- <https://tailwindcss.com/docs/extracting-components/>
### Naming
Filenames should use `snake_case`.
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 should use shorthand where possible, and should use
lower case letters to differentiate 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.
### IDs
Don't use ID selectors in CSS.
```scss
// Bad
#my-element {
padding: 0;
}
// Good
.my-element {
padding: 0;
}
```
### Variables
Before adding a new variable for a color or a size, guarantee:
- There isn't already one
- There isn't a similar one we can use instead.
## Linting
We use [SCSS Lint](https://github.com/sds/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](https://github.com/sds/scss-lint/blob/master/lib/scss_lint/linter/README.md).
### 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.
### Ignoring issues
If you want a line or set of lines to be ignored by the linter, you can use
`// scss-lint:disable RuleName` ([more info](https://github.com/sds/scss-lint#disabling-linters-via-source)):
```scss
// This lint rule is disabled because it is supported only in Chrome/Safari
// scss-lint:disable PropertySpelling
body {
text-decoration-skip: ink;
}
// scss-lint:enable PropertySpelling
```
Make sure a comment is added on the line above the `disable` rule, otherwise the
linter will throw a warning. `DisableLinterReason` is enabled to make sure the
style guide isn't being ignored, and to communicate to others why the style
guide is ignored in this instance.
[csscomb]: https://github.com/csscomb/csscomb.js
[node]: https://github.com/nodejs/node
[npm]: https://www.npmjs.com/
# Vue.js style guide
## Linting
We default to [eslint-vue-plugin](https://github.com/vuejs/eslint-plugin-vue), with the `plugin:vue/recommended`.
Please check this [rules](https://github.com/vuejs/eslint-plugin-vue#bulb-rules) for more documentation.
## Basic Rules
1. The service has it's own file
1. The store has it's own file
1. Use a function in the bundle file to instantiate the Vue component:
```javascript
// bad
class {
init() {
new Component({})
}
}
// good
document.addEventListener('DOMContentLoaded', () => new Vue({
el: '#element',
components: {
componentName
},
render: createElement => createElement('component-name'),
}));
```
1. Do not use a singleton for the service or the store
```javascript
// bad
class Store {
constructor() {
if (!this.prototype.singleton) {
// do something
}
}
}
// good
class Store {
constructor() {
// do something
}
}
```
1. Use `.vue` for Vue templates. Do not use `%template` in HAML.
## Naming
1. **Extensions**: Use `.vue` extension for Vue components. Do not use `.js` as file extension ([#34371]).
1. **Reference Naming**: Use PascalCase for their instances:
```javascript
// bad
import cardBoard from 'cardBoard.vue'
components: {
cardBoard,
};
// good
import CardBoard from 'cardBoard.vue'
components: {
CardBoard,
};
```
1. **Props Naming:** Avoid using DOM component prop names.
1. **Props Naming:** Use kebab-case instead of camelCase to provide props in templates.
```javascript
// bad
<component class="btn">
// good
<component css-class="btn">
// bad
<component myProp="prop" />
// good
<component my-prop="prop" />
```
[#34371]: https://gitlab.com/gitlab-org/gitlab-foss/issues/34371
## Alignment
1. Follow these alignment styles for the template method:
1. With more than one attribute, all attributes should be on a new line:
```javascript
// bad
<component v-if="bar"
param="baz" />
<button class="btn">Click me</button>
// good
<component
v-if="bar"
param="baz"
/>
<button class="btn">
Click me
</button>
```
1. The tag can be inline if there is only one attribute:
```javascript
// good
<component bar="bar" />
// good
<component
bar="bar"
/>
// bad
<component
bar="bar" />
```
## Quotes
1. Always use double quotes `"` inside templates and single quotes `'` for all other JS.
```javascript
// bad
template: `
<button :class='style'>Button</button>
`
// good
template: `
<button :class="style">Button</button>
`
```
## Props
1. Props should be declared as an object
```javascript
// bad
props: ['foo']
// good
props: {
foo: {
type: String,
required: false,
default: 'bar'
}
}
```
1. Required key should always be provided when declaring a prop
```javascript
// bad
props: {
foo: {
type: String,
}
}
// good
props: {
foo: {
type: String,
required: false,
default: 'bar'
}
}
```
1. Default key should be provided if the prop is not required.
_Note:_ There are some scenarios where we need to check for the existence of the property.
On those a default key should not be provided.
```javascript
// good
props: {
foo: {
type: String,
required: false,
}
}
// good
props: {
foo: {
type: String,
required: false,
default: 'bar'
}
}
// good
props: {
foo: {
type: String,
required: true
}
}
```
## Data
1. `data` method should always be a function
```javascript
// bad
data: {
foo: 'foo'
}
// good
data() {
return {
foo: 'foo'
};
}
```
## Directives
1. Shorthand `@` is preferable over `v-on`
```javascript
// bad
<component v-on:click="eventHandler"/>
// good
<component @click="eventHandler"/>
```
1. Shorthand `:` is preferable over `v-bind`
```javascript
// bad
<component v-bind:class="btn"/>
// good
<component :class="btn"/>
```
1. Shorthand `#` is preferable over `v-slot`
```javascript
// bad
<template v-slot:header></template>
// good
<template #header></template>
```
## Closing tags
1. Prefer self closing component tags
```javascript
// bad
<component></component>
// good
<component />
```
## Component usage within templates
1. Prefer a component's kebab-cased name over other styles when using it in a template
```javascript
// bad
<MyComponent />
// good
<my-component />
```
## Ordering
1. Tag order in `.vue` file
```
<script>
// ...
</script>
<template>
// ...
</template>
// We don't use scoped styles but there are few instances of this
<style>
// ...
</style>
```
1. Properties in a Vue Component:
Check [order of properties in components rule](https://github.com/vuejs/eslint-plugin-vue/blob/master/docs/rules/order-in-components.md).
## `:key`
When using `v-for` you need to provide a *unique* `:key` attribute for each item.
1. If the elements of the array being iterated have an unique `id` it is advised to use it:
```html
<div
v-for="item in items"
:key="item.id"
>
<!-- content -->
</div>
```
1. When the elements being iterated don't have a unique id, you can use the array index as the `:key` attribute
```html
<div
v-for="(item, index) in items"
:key="index"
>
<!-- content -->
</div>
```
1. When using `v-for` with `template` and there is more than one child element, the `:key` values must be unique. It's advised to use `kebab-case` namespaces.
```html
<template v-for="(item, index) in items">
<span :key="`span-${index}`"></span>
<button :key="`button-${index}`"></button>
</template>
```
1. When dealing with nested `v-for` use the same guidelines as above.
```html
<div
v-for="item in items"
:key="item.id"
>
<span
v-for="element in array"
:key="element.id"
>
<!-- content -->
</span>
</div>
```
Useful links:
1. [`key`](https://vuejs.org/v2/guide/list.html#key)
1. [Vue Style Guide: Keyed v-for](https://vuejs.org/v2/style-guide/#Keyed-v-for-essential )
## Vue and Bootstrap
1. Tooltips: Do not rely on `has-tooltip` class name for Vue components
```javascript
// bad
<span
class="has-tooltip"
title="Some tooltip text">
Text
</span>
// good
<span
v-tooltip
title="Some tooltip text">
Text
</span>
```
1. Tooltips: When using a tooltip, include the tooltip directive, `./app/assets/javascripts/vue_shared/directives/tooltip.js`
1. Don't change `data-original-title`.
```javascript
// bad
<span data-original-title="tooltip text">Foo</span>
// good
<span title="tooltip text">Foo</span>
$('span').tooltip('_fixTitle');
```
## The JavaScript/Vue Accord
The goal of this accord is to make sure we are all on the same page.
1. When writing Vue, you may not use jQuery in your application.
1. If you need to grab data from the DOM, you may query the DOM 1 time while bootstrapping your application to grab data attributes using `dataset`. You can do this without jQuery.
1. You may use a jQuery dependency in Vue.js following [this example from the docs](https://vuejs.org/v2/examples/select2.html).
1. If an outside jQuery Event needs to be listen to inside the Vue application, you may use jQuery event listeners.
1. We will avoid adding new jQuery events when they are not required. Instead of adding new jQuery events take a look at [different methods to do the same task](https://vuejs.org/v2/api/#vm-emit).
1. You may query the `window` object 1 time, while bootstrapping your application for application specific data (e.g. `scrollTo` is ok to access anytime). Do this access during the bootstrapping of your application.
1. You may have a temporary but immediate need to create technical debt by writing code that does not follow our standards, to be refactored later. Maintainers need to be ok with the tech debt in the first place. An issue should be created for that tech debt to evaluate it further and discuss. In the coming months you should fix that tech debt, with it's priority to be determined by maintainers.
1. When creating tech debt you must write the tests for that code before hand and those tests may not be rewritten. e.g. jQuery tests rewritten to Vue tests.
1. You may choose to use VueX as a centralized state management. If you choose not to use VueX, you must use the *store pattern* which can be found in the [Vue.js documentation](https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch).
1. Once you have chosen a centralized state management solution you must use it for your entire application. i.e. Don't mix and match your state management solutions.
This diff is collapsed.
# SCSS styleguide ---
redirect_to: 'style/scss.md'
---
This style guide recommends best practices for SCSS to make styles easy to read, This document was moved to [another location](style/scss.md).
easy to maintain, and performant for the end-user.
## Rules
### Utility Classes
As part of the effort for [cleaning up our CSS and moving our components into GitLab-UI](https://gitlab.com/groups/gitlab-org/-/epics/950)
led by the [GitLab UI WG](https://gitlab.com/gitlab-com/www-gitlab-com/merge_requests/20623) we prefer the use of utility classes over adding new CSS. However, complex CSS can be addressed by adding component classes.
#### Where are utility classes defined?
- [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/)
- [`common.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/framework/common.scss) (old)
- [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss) (new)
#### Where should I put new utility classes?
New utility classes should be added to [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss). Existing classes include:
| Name | Pattern | Example |
|------|---------|---------|
| Background color | `.bg-{variant}-{shade}` | `.bg-warning-400` |
| Text color | `.text-{variant}-{shade}` | `.text-success-500` |
| Font size | `.text-{size}` | `.text-2` |
- `{variant}` is one of 'primary', 'secondary', 'success', 'warning', 'error'
- `{shade}` is one of the shades listed on [colors](https://design.gitlab.com/product-foundations/colors/)
- `{size}` is a number from 1-6 from our [Type scale](https://design.gitlab.com/product-foundations/typography/)
#### When should I create component classes?
We recommend a "utility-first" approach.
1. Start with utility classes.
1. If composing utility classes into a component class removes code duplication and encapsulates a clear responsibility, do it.
This encourages an organic growth of component classes and prevents the creation of one-off unreusable classes. Also, the kind of classes that emerge from "utility-first" tend to be design-centered (e.g. `.button`, `.alert`, `.card`) rather than domain-centered (e.g. `.security-report-widget`, `.commit-header-icon`).
Examples of component classes that were created using "utility-first" include:
- [`.circle-icon-container`](https://gitlab.com/gitlab-org/gitlab/blob/579fa8b8ec7eb38d40c96521f517c9dab8c3b97a/app/assets/stylesheets/framework/icons.scss#L85)
- [`.d-flex-center`](https://gitlab.com/gitlab-org/gitlab/blob/900083d89cd6af391d26ab7922b3f64fa2839bef/app/assets/stylesheets/framework/common.scss#L425)
Inspiration:
- <https://tailwindcss.com/docs/utility-first/>
- <https://tailwindcss.com/docs/extracting-components/>
### Naming
Filenames should use `snake_case`.
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 should use shorthand where possible, and should use
lower case letters to differentiate 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.
### IDs
Don't use ID selectors in CSS.
```scss
// Bad
#my-element {
padding: 0;
}
// Good
.my-element {
padding: 0;
}
```
### Variables
Before adding a new variable for a color or a size, guarantee:
- There isn't already one
- There isn't a similar one we can use instead.
## Linting
We use [SCSS Lint](https://github.com/sds/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](https://github.com/sds/scss-lint/blob/master/lib/scss_lint/linter/README.md).
### 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.
### Ignoring issues
If you want a line or set of lines to be ignored by the linter, you can use
`// scss-lint:disable RuleName` ([more info](https://github.com/sds/scss-lint#disabling-linters-via-source)):
```scss
// This lint rule is disabled because it is supported only in Chrome/Safari
// scss-lint:disable PropertySpelling
body {
text-decoration-skip: ink;
}
// scss-lint:enable PropertySpelling
```
Make sure a comment is added on the line above the `disable` rule, otherwise the
linter will throw a warning. `DisableLinterReason` is enabled to make sure the
style guide isn't being ignored, and to communicate to others why the style
guide is ignored in this instance.
[csscomb]: https://github.com/csscomb/csscomb.js
[node]: https://github.com/nodejs/node
[npm]: https://www.npmjs.com/
# Tooling
## ESLint
We use ESLint to encapsulate and enforce frontend code standards. Our configuration may be found in the [gitlab-eslint-config](https://gitlab.com/gitlab-org/gitlab-eslint-config) project.
### Disabling ESLint in new files
Do not disable ESLint when creating new files. Existing files may have existing rules
disabled due to legacy compatibility reasons but they are in the process of being refactored.
Do not disable specific ESLint rules. To avoid introducing technical debt, you may disable the following
rules only if you are invoking/instantiating existing code modules.
- [`no-new`](https://eslint.org/docs/rules/no-new)
- [`class-method-use-this`](https://eslint.org/docs/rules/class-methods-use-this)
NOTE: **Note:**
Disable these rules on a per-line basis. This makes it easier to refactor
in the future. E.g. use `eslint-disable-next-line` or `eslint-disable-line`.
### Disabling ESLint for a single violation
If you do need to disable a rule for a single violation, disable it for the smallest amount of code necessary:
```javascript
// bad
/* eslint-disable no-new */
import Foo from 'foo';
new Foo();
// better
import Foo from 'foo';
// eslint-disable-next-line no-new
new Foo();
```
### The `no-undef` rule and declaring globals
**Never** disable the `no-undef` rule. Declare globals with `/* global Foo */` instead.
When declaring multiple globals, always use one `/* global [name] */` line per variable.
```javascript
// bad
/* globals Flash, Cookies, jQuery */
// good
/* global Flash */
/* global Cookies */
/* global jQuery */
```
## Formatting with Prettier
Our code is automatically formatted with [Prettier](https://prettier.io) to follow our style guides. Prettier is taking care of formatting .js, .vue, and .scss files based on the standard prettier rules. You can find all settings for Prettier in `.prettierrc`.
### Editor
The easiest way to include prettier in your workflow is by setting up your preferred editor (all major editors are supported) accordingly. We suggest setting up prettier to run automatically when each file is saved. Find [here](https://prettier.io/docs/en/editors.html) the best way to set it up in your preferred editor.
Please take care that you only let Prettier format the same file types as the global Yarn script does (.js, .vue, and .scss). In VSCode by example you can easily exclude file formats in your settings file:
```
"prettier.disableLanguages": [
"json",
"markdown"
],
```
### Yarn Script
The following yarn scripts are available to do global formatting:
```
yarn prettier-staged-save
```
Updates all currently staged files (based on `git diff`) with Prettier and saves the needed changes.
```
yarn prettier-staged
```
Checks all currently staged files (based on `git diff`) with Prettier and log which files would need manual updating to the console.
```
yarn prettier-all
```
Checks all files with Prettier and logs which files need manual updating to the console.
```
yarn prettier-all-save
```
Formats all files in the repository with Prettier. (This should only be used to test global rule updates otherwise you would end up with huge MR's).
The source of these Yarn scripts can be found in `/scripts/frontend/prettier.js`.
#### Scripts during Conversion period
```
node ./scripts/frontend/prettier.js check-all ./vendor/
```
This will go over all files in a specific folder check it.
```
node ./scripts/frontend/prettier.js save-all ./vendor/
```
This will go over all files in a specific folder and save it.
### VSCode Settings
#### Select Prettier as default formatter
To select Prettier as a formatter, add the following properties to your User or Workspace Settings:
```javascript
{
"[html]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"[javascript]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"[vue]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
}
}
```
#### Format on Save
To automatically format your files with Prettier, add the following properties to your User or Workspace Settings:
```javascript
{
"[html]": {
"editor.formatOnSave": true
},
"[javascript]": {
"editor.formatOnSave": true
},
"[vue]": {
"editor.formatOnSave": true
},
}
```
...@@ -179,7 +179,7 @@ Check this [page](vuex.md) for more details. ...@@ -179,7 +179,7 @@ Check this [page](vuex.md) for more details.
## Style guide ## Style guide
Please refer to the Vue section of our [style guide](style_guide_js.md#vuejs) Please refer to the Vue section of our [style guide](style/vue.md)
for best practices while writing your Vue components and templates. for best practices while writing your Vue components and templates.
## Testing Vue Components ## Testing Vue Components
......
...@@ -15,10 +15,6 @@ Learn about all the dependencies that make up our frontend, including some of ou ...@@ -15,10 +15,6 @@ Learn about all the dependencies that make up our frontend, including some of ou
Learn about all the internal JavaScript modules that make up our frontend. Learn about all the internal JavaScript modules that make up our frontend.
## [Style guides](style/index.md)
Style guides to keep our code consistent.
## [Tips](tips.md) ## [Tips](tips.md)
Tips from our frontend team to develop more efficiently and effectively. Tips from our frontend team to develop more efficiently and effectively.
# HTML style guide ---
redirect_to: '../../fe_guide/style/html.md'
---
## Buttons This document was moved to [another location](../../fe_guide/style/html.md).
### Button type
Button tags requires a `type` attribute according to the [W3C HTML specification](https://www.w3.org/TR/2011/WD-html5-20110525/the-button-element.html#dom-button-type).
```html
// bad
<button></button>
// good
<button type="button"></button>
```
### Button role
If an HTML element has an `onClick` handler but is not a button, it should have `role="button"`. This is [more accessible](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role).
```html
// bad
<div onClick="doSomething"></div>
// good
<div role="button" onClick="doSomething"></div>
```
## Links
### Blank target
Use `rel="noopener noreferrer"` whenever your links open in a new window, i.e. `target="_blank"`. This prevents a security vulnerability [documented by JitBit](https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/).
```html
// bad
<a href="url" target="_blank"></a>
// good
<a href="url" target="_blank" rel="noopener noreferrer"></a>
```
### Fake links
**Do not use fake links.** Use a button tag if a link only invokes JavaScript click event handlers, which is more semantic.
```html
// bad
<a class="js-do-something" href="#"></a>
// good
<button class="js-do-something" type="button"></button>
```
# Style guides ---
redirect_to: '../../fe_guide/style/index.md'
---
## [HTML style guide](html.md) This document was moved to [another location](../../fe_guide/style/index.md).
## [SCSS style guide](scss.md)
## [JavaScript style guide](javascript.md)
## [Vue style guide](vue.md)
## Tooling
## [Prettier](prettier.md)
Our code is automatically formatted with [Prettier](https://prettier.io) to follow our guidelines.
# JavaScript style guide ---
redirect_to: '../../fe_guide/style/javascript.md'
---
We use [Airbnb's JavaScript Style Guide](https://github.com/airbnb/javascript) and it's accompanying This document was moved to [another location](../../fe_guide/style/javascript.md).
linter to manage most of our JavaScript style guidelines.
In addition to the style guidelines set by Airbnb, we also have a few specific rules
listed below.
> **Tip:**
You can run eslint locally by running `yarn eslint`
## Avoid forEach
Avoid forEach when mutating data. Use `map`, `reduce` or `filter` instead of `forEach`
when mutating data. This will minimize mutations in functions,
which aligns with [Airbnb's style guide](https://github.com/airbnb/javascript#testing--for-real).
```javascript
// bad
users.forEach((user, index) => {
user.id = index;
});
// good
const usersWithId = users.map((user, index) => {
return Object.assign({}, user, { id: index });
});
```
## Limit number of parameters
If your function or method has more than 3 parameters, use an object as a parameter
instead.
```javascript
// bad
function a(p1, p2, p3) {
// ...
};
// good
function a(p) {
// ...
};
```
## Avoid side effects in constructors
Avoid making asynchronous calls, API requests or DOM manipulations in the `constructor`.
Move them into separate functions instead. This will make tests easier to write and
code easier to maintain.
```javascript
// bad
class myClass {
constructor(config) {
this.config = config;
axios.get(this.config.endpoint)
}
}
// good
class myClass {
constructor(config) {
this.config = config;
}
makeRequest() {
axios.get(this.config.endpoint)
}
}
const instance = new myClass();
instance.makeRequest();
```
## Avoid classes to handle DOM events
If the only purpose of the class is to bind a DOM event and handle the callback, prefer
using a function.
```javascript
// bad
class myClass {
constructor(config) {
this.config = config;
}
init() {
document.addEventListener('click', () => {});
}
}
// good
const myFunction = () => {
document.addEventListener('click', () => {
// handle callback here
});
}
```
## Pass element container to constructor
When your class manipulates the DOM, receive the element container as a parameter.
This is more maintainable and performant.
```javascript
// bad
class a {
constructor() {
document.querySelector('.b');
}
}
// good
class a {
constructor(options) {
options.container.querySelector('.b');
}
}
```
## Use ParseInt
Use `ParseInt` when converting a numeric string into a number.
```javascript
// bad
Number('10')
// good
parseInt('10', 10);
```
## CSS Selectors - Use `js-` prefix
If a CSS class is only being used in JavaScript as a reference to the element, prefix
the class name with `js-`.
```html
// bad
<button class="add-user"></button>
// good
<button class="js-add-user"></button>
```
## Absolute vs relative paths for modules
Use relative paths if the module you are importing is less than two levels up.
```javascript
// bad
import GitLabStyleGuide from '~/guides/GitLabStyleGuide';
// good
import GitLabStyleGuide from '../GitLabStyleGuide';
```
If the module you are importing is two or more levels up, use an absolute path instead:
```javascript
// bad
import GitLabStyleGuide from '../../../guides/GitLabStyleGuide';
// good
import GitLabStyleGuide from '~/GitLabStyleGuide';
```
Additionally, **do not add to global namespace**.
## Do not use `DOMContentLoaded` in non-page modules
Imported modules should act the same each time they are loaded. `DOMContentLoaded`
events are only allowed on modules loaded in the `/pages/*` directory because those
are loaded dynamically with webpack.
## Avoid XSS
Do not use `innerHTML`, `append()` or `html()` to set content. It opens up too many
vulnerabilities.
## Disabling ESLint in new files
Do not disable ESLint when creating new files. Existing files may have existing rules
disabled due to legacy compatibility reasons but they are in the process of being refactored.
Do not disable specific ESLint rules. Due to technical debt, you may disable the following
rules only if you are invoking/instantiating existing code modules.
- [no-new](https://eslint.org/docs/rules/no-new)
- [class-method-use-this](https://eslint.org/docs/rules/class-methods-use-this)
> Note: Disable these rules on a per line basis. This makes it easier to refactor
> in the future. E.g. use `eslint-disable-next-line` or `eslint-disable-line`.
# Formatting with Prettier ---
redirect_to: '../../fe_guide/tooling.md#formatting-with-prettier'
---
Our code is automatically formatted with [Prettier](https://prettier.io) to follow our style guides. Prettier is taking care of formatting .js, .vue, and .scss files based on the standard prettier rules. You can find all settings for Prettier in `.prettierrc`. This document was moved to [another location](../../fe_guide/tooling.md#formatting-with-prettier).
## Editor
The easiest way to include prettier in your workflow is by setting up your preferred editor (all major editors are supported) accordingly. We suggest setting up prettier to run automatically when each file is saved. Find [here](https://prettier.io/docs/en/editors.html) the best way to set it up in your preferred editor.
Please take care that you only let Prettier format the same file types as the global Yarn script does (.js, .vue, and .scss). In VSCode by example you can easily exclude file formats in your settings file:
```
"prettier.disableLanguages": [
"json",
"markdown"
],
```
## Yarn Script
The following yarn scripts are available to do global formatting:
```
yarn prettier-staged-save
```
Updates all currently staged files (based on `git diff`) with Prettier and saves the needed changes.
```
yarn prettier-staged
```
Checks all currently staged files (based on `git diff`) with Prettier and log which files would need manual updating to the console.
```
yarn prettier-all
```
Checks all files with Prettier and logs which files need manual updating to the console.
```
yarn prettier-all-save
```
Formats all files in the repository with Prettier. (This should only be used to test global rule updates otherwise you would end up with huge MR's).
The source of these Yarn scripts can be found in `/scripts/frontend/prettier.js`.
### Scripts during Conversion period
```
node ./scripts/frontend/prettier.js check-all ./vendor/
```
This will go over all files in a specific folder check it.
```
node ./scripts/frontend/prettier.js save-all ./vendor/
```
This will go over all files in a specific folder and save it.
## VSCode Settings
### Select Prettier as default formatter
To select Prettier as a formatter, add the following properties to your User or Workspace Settings:
```javascript
{
"[html]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"[javascript]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
},
"[vue]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
}
}
```
### Format on Save
To automatically format your files with Prettier, add the following properties to your User or Workspace Settings:
```javascript
{
"[html]": {
"editor.formatOnSave": true
},
"[javascript]": {
"editor.formatOnSave": true
},
"[vue]": {
"editor.formatOnSave": true
},
}
```
# SCSS style guide
> TODO: Add content
# Vue style guide
> TODO: Add content
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