Commit 2369050a authored by Alfredo Sumaran's avatar Alfredo Sumaran

Merge branch '28713-fe-style-guide' into 'master'

Creates Frontend Style guide

Closes #28713

See merge request !9961
parents 3574963b 8c3bdc85
...@@ -82,7 +82,7 @@ If a contributor is no longer actively working on a submitted merge request ...@@ -82,7 +82,7 @@ If a contributor is no longer actively working on a submitted merge request
we can decide that the merge request will be finished by one of our we can decide that the merge request will be finished by one of our
[Merge request coaches][team] or close the merge request. We make this decision [Merge request coaches][team] or close the merge request. We make this decision
based on how important the change is for our product vision. If a Merge request based on how important the change is for our product vision. If a Merge request
coach is going to finish the merge request we assign the coach is going to finish the merge request we assign the
~"coach will finish" label. ~"coach will finish" label.
## Helping others ## Helping others
...@@ -479,8 +479,7 @@ merge request: ...@@ -479,8 +479,7 @@ merge request:
1. [Rails](https://github.com/bbatsov/rails-style-guide) 1. [Rails](https://github.com/bbatsov/rails-style-guide)
1. [Newlines styleguide][newlines-styleguide] 1. [Newlines styleguide][newlines-styleguide]
1. [Testing](doc/development/testing.md) 1. [Testing](doc/development/testing.md)
1. [JavaScript (ES6)](https://github.com/airbnb/javascript) 1. [JavaScript styleguide][js-styleguide]
1. [JavaScript (ES5)](https://github.com/airbnb/javascript/tree/es5-deprecated/es5)
1. [SCSS styleguide][scss-styleguide] 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
...@@ -549,7 +548,8 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor ...@@ -549,7 +548,8 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor
[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
[changelog]: doc/development/changelog.md "Generate a changelog entry" [changelog]: doc/development/changelog.md "Generate a changelog entry"
[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" [js-styleguide]: doc/development/fe_guide/style_guide_js.md "JavaScript styleguide"
[scss-styleguide]: doc/development/fe_guide/style_guide_scss.md "SCSS styleguide"
[newlines-styleguide]: doc/development/newlines_styleguide.md "Newlines styleguide" [newlines-styleguide]: doc/development/newlines_styleguide.md "Newlines styleguide"
[UX Guide for GitLab]: http://docs.gitlab.com/ce/development/ux_guide/ [UX Guide for GitLab]: http://docs.gitlab.com/ce/development/ux_guide/
[license-finder-doc]: doc/development/licensing.md [license-finder-doc]: doc/development/licensing.md
......
---
title: Adds Frontend Styleguide to documentation
merge_request: 9961
author:
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
- [SQL Migration Style Guide](migration_style_guide.md) for creating safe SQL migrations - [SQL Migration Style Guide](migration_style_guide.md) for creating safe SQL migrations
- [Testing standards and style guidelines](testing.md) - [Testing standards and style guidelines](testing.md)
- [UX guide](ux_guide/index.md) for building GitLab with existing CSS styles and elements - [UX guide](ux_guide/index.md) for building GitLab with existing CSS styles and elements
- [Frontend guidelines](frontend.md) - [Frontend guidelines](fe_guide/index.md)
- [SQL guidelines](sql.md) for working with SQL queries - [SQL guidelines](sql.md) for working with SQL queries
- [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers - [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers
- [`Gemfile` guidelines](gemfile.md) - [`Gemfile` guidelines](gemfile.md)
......
# Accessibility
## Resources
[Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools]
are useful for testing for potential accessibility problems in GitLab.
Accessibility best-practices and more in-depth information is available on
[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools.
[chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools
[audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules
# Architecture
When you are developing a new feature that requires architectural design, or if
you are changing the fundamental design of an existing feature, make sure it is
discussed with one of the Frontend Architecture Experts.
A Frontend Architect is an expert who makes high-level Frontend design decisions
and decides on technical standards, including coding standards and frameworks.
Architectural decisions should be accessible to everyone, so please document
them in the relevant Merge Request discussion or by updating our documentation
when appropriate.
You can find the Frontend Architecture experts on the [team page][team-page].
## Examples
You can find documentation about the desired architecture for a new feature
built with Vue.js [here][vue-section].
[team-page]: https://about.gitlab.com/team
[vue-section]: vue.md#frontend.html#how-to-build-a-new-feature-with-vue-js
# Design Patterns
## Singletons
When exactly one object is needed for a given task, prefer to define it as a
`class` rather than as an object literal. Prefer also to explicitly restrict
instantiation, unless flexibility is important (e.g. for testing).
```javascript
// bad
const MyThing = {
prop1: 'hello',
method1: () => {}
};
export default MyThing;
// good
class MyThing {
constructor() {
this.prop1 = 'hello';
}
method1() {}
}
export default new MyThing();
// best
export default class MyThing {
constructor() {
if (!this.prototype.singleton) {
this.init();
this.prototype.singleton = this;
}
return this.prototype.singleton;
}
init() {
this.prop1 = 'hello';
}
method1() {}
}
```
## Manipulating the DOM in a JS Class
When writing a class that needs to manipulate the DOM guarantee a container option is provided.
This is useful when we need that class to be instantiated more than once in the same page.
Bad:
```javascript
class Foo {
constructor() {
document.querySelector('.bar');
}
}
new Foo();
```
Good:
```javascript
class Foo {
constructor(opts) {
document.querySelector(`${opts.container} .bar`);
}
}
new Foo({ container: '.my-element' });
```
You can find an example of the above in this [class][container-class-example];
[container-class-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js
# Frontend Development Guidelines
This document describes various guidelines to ensure consistency and quality
across GitLab's frontend team.
## Overview
GitLab is built on top of [Ruby on Rails][rails] using [Haml][haml] with
[Hamlit][hamlit]. Be wary of [the limitations that come with using
Hamlit][hamlit-limits]. We also use [SCSS][scss] and plain JavaScript with
modern ECMAScript standards supported through [Babel][babel] and ES module
support through [webpack][webpack].
We also utilize [webpack][webpack] to handle the bundling, minification, and
compression of our assets.
Working with our frontend assets requires Node (v4.3 or greater) and Yarn
(v0.17 or greater). You can find information on how to install these on our
[installation guide][install].
[jQuery][jquery] is used throughout the application's JavaScript, with
[Vue.js][vue] for particularly advanced, dynamic elements.
### Browser Support
For our currently-supported browsers, see our [requirements][requirements].
---
## [Architecture](architecture.md)
How we go about making fundamental design decisions in GitLab's frontend team
or make changes to our frontend development guidelines.
---
## [Testing](testing.md)
How we write frontend tests, run the GitLab test suite, and debug test related
issues.
---
## [Design Patterns](design_patterns.md)
Common JavaScript design patterns in GitLab's codebase.
---
## [Vue.js Best Practices](vue.md)
Vue specific design patterns and practices.
---
## Style Guides
### [JavaScript Style Guide](style_guide_js.md)
We use eslint to enforce our JavaScript style guides. Our guide is based on
the excellent [Airbnb][airbnb-js-style-guide] style guide with a few small
changes.
### [SCSS Style Guide](style_guide_scss.md)
Our SCSS conventions which are enforced through [scss-lint][scss-lint].
---
## [Performance](performance.md)
Best practices for monitoring and maximizing frontend performance.
---
## [Security](security.md)
Frontend security practices.
---
## [Accessibility](accessibility.md)
Our accessibility standards and resources.
[rails]: http://rubyonrails.org/
[haml]: http://haml.info/
[hamlit]: https://github.com/k0kubun/hamlit
[hamlit-limits]: https://github.com/k0kubun/hamlit/blob/master/REFERENCE.md#limitations
[scss]: http://sass-lang.com/
[babel]: https://babeljs.io/
[webpack]: https://webpack.js.org/
[jquery]: https://jquery.com/
[vue]: http://vuejs.org/
[airbnb-js-style-guide]: https://github.com/airbnb/javascript
[scss-lint]: https://github.com/brigade/scss-lint
[install]: ../../install/installation.md#4-node
[requirements]: ../../install/requirements.md#supported-web-browsers
# Performance
## Best Practices
### Realtime Components
When writing code for realtime features we have to keep a couple of things in mind:
1. Do not overload the server with requests.
1. It should feel realtime.
Thus, we must strike a balance between sending requests and the feeling of realtime.
Use the following rules when creating realtime solutions.
1. The server will tell you how much to poll by sending `Poll-Interval` in the header.
Use that as your polling interval. This way it is easy for system administrators to change the
polling rate.
A `Poll-Interval: -1` means you should disable polling, and this must be implemented.
1. A response with HTTP status `4XX` or `5XX` should disable polling as well.
1. Use a common library for polling.
1. Poll on active tabs only. Use a common library to find out which tab currently has eyes on it.
Please use [Focus](https://gitlab.com/andrewn/focus). Specifically [Eyeballs Detector](https://gitlab.com/andrewn/focus/blob/master/lib/eyeballs-detector.js).
1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval will be
controlled by the server.
1. The backend code will most likely be using etags. You do not and should not check for status
`304 Not Modified`. The browser will transform it for you.
## Reducing Asset Footprint
### Page-specific JavaScript
Certain pages may require the use of a third party library, such as [d3][d3] for
the User Activity Calendar and [Chart.js][chartjs] for the Graphs pages. These
libraries increase the page size significantly, and impact load times due to
bandwidth bottlenecks and the browser needing to parse more JavaScript.
In cases where libraries are only used on a few specific pages, we use
"page-specific JavaScript" to prevent the main `main.js` file from
becoming unnecessarily large.
Steps to split page-specific JavaScript from the main `main.js`:
1. Create a directory for the specific page(s), e.g. `graphs/`.
1. In that directory, create a `namespace_bundle.js` file, e.g. `graphs_bundle.js`.
1. Add the new "bundle" file to the list of entry files in `config/webpack.config.js`.
- For example: `graphs: './graphs/graphs_bundle.js',`.
1. Move code reliant on these libraries into the `graphs` directory.
1. In `graphs_bundle.js` add CommonJS `require('./path_to_some_component.js');` statements to load any other files in this directory. Make sure to use relative urls.
1. In the relevant views, add the scripts to the page with the following:
```haml
- content_for :page_specific_javascripts do
= page_specific_javascript_bundle_tag('lib_chart')
= page_specific_javascript_bundle_tag('graphs')
```
The above loads `chart.js` and `graphs_bundle.js` for this page only. `chart.js`
is separated from the bundle file so it can be cached separately from the bundle
and reused for other pages that also rely on the library. For an example, see
[this Haml file][page-specific-js-example].
### Code Splitting
> *TODO* flesh out this section once webpack is ready for code-splitting
### Minimizing page size
A smaller page size means the page loads faster (especially important on mobile
and poor connections), the page is parsed more quickly by the browser, and less
data is used for users with capped data plans.
General tips:
- Don't add new fonts.
- Prefer font formats with better compression, e.g. WOFF2 is better than WOFF, which is better than TTF.
- Compress and minify assets wherever possible (For CSS/JS, Sprockets and webpack do this for us).
- If some functionality can reasonably be achieved without adding extra libraries, avoid them.
- Use page-specific JavaScript as described above to dynamically load libraries that are only needed on certain pages.
-------
## Additional Resources
- [WebPage Test][web-page-test] for testing site loading time and size.
- [Google PageSpeed Insights][pagespeed-insights] grades web pages and provides feedback to improve the page.
- [Profiling with Chrome DevTools][google-devtools-profiling]
- [Browser Diet][browser-diet] is a community-built guide that catalogues practical tips for improving web page performance.
[web-page-test]: http://www.webpagetest.org/
[pagespeed-insights]: https://developers.google.com/speed/pagespeed/insights/
[google-devtools-profiling]: https://developers.google.com/web/tools/chrome-devtools/profile/?hl=en
[browser-diet]: https://browserdiet.com/
[d3]: https://d3js.org/
[chartjs]: http://www.chartjs.org/
[page-specific-js-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/13bb9ed77f405c5f6ee4fdbc964ecf635c9a223f/app/views/projects/graphs/_head.html.haml#L6-8
# Security
### Resources
[Mozilla’s HTTP Observatory CLI][observatory-cli] and the
[Qualys SSL Labs Server Test][qualys-ssl] are good resources for finding
potential problems and ensuring compliance with security best practices.
<!-- Uncomment these sections when CSP/SRI are implemented.
### Content Security Policy (CSP)
Content Security Policy is a web standard that intends to mitigate certain
forms of Cross-Site Scripting (XSS) as well as data injection.
Content Security Policy rules should be taken into consideration when
implementing new features, especially those that may rely on connection with
external services.
GitLab's CSP is used for the following:
- Blocking plugins like Flash and Silverlight from running at all on our pages.
- Blocking the use of scripts and stylesheets downloaded from external sources.
- Upgrading `http` requests to `https` when possible.
- Preventing `iframe` elements from loading in most contexts.
Some exceptions include:
- Scripts from Google Analytics and Piwik if either is enabled.
- Connecting with GitHub, Bitbucket, GitLab.com, etc. to allow project importing.
- Connecting with Google, Twitter, GitHub, etc. to allow OAuth authentication.
We use [the Secure Headers gem][secure_headers] to enable Content
Security Policy headers in the GitLab Rails app.
Some resources on implementing Content Security Policy:
- [MDN Article on CSP][mdn-csp]
- [GitHub’s CSP Journey on the GitHub Engineering Blog][github-eng-csp]
- The Dropbox Engineering Blog's series on CSP: [1][dropbox-csp-1], [2][dropbox-csp-2], [3][dropbox-csp-3], [4][dropbox-csp-4]
### Subresource Integrity (SRI)
Subresource Integrity prevents malicious assets from being provided by a CDN by
guaranteeing that the asset downloaded is identical to the asset the server
is expecting.
The Rails app generates a unique hash of the asset, which is used as the
asset's `integrity` attribute. The browser generates the hash of the asset
on-load and will reject the asset if the hashes do not match.
All CSS and JavaScript assets should use Subresource Integrity.
Some resources on implementing Subresource Integrity:
- [MDN Article on SRI][mdn-sri]
- [Subresource Integrity on the GitHub Engineering Blog][github-eng-sri]
-->
### Including external resources
External fonts, CSS, and JavaScript should never be used with the exception of
Google Analytics and Piwik - and only when the instance has enabled it. Assets
should always be hosted and served locally from the GitLab instance. Embedded
resources via `iframes` should never be used except in certain circumstances
such as with ReCaptcha, which cannot be used without an `iframe`.
### Avoiding inline scripts and styles
In order to protect users from [XSS vulnerabilities][xss], we will disable
inline scripts in the future using Content Security Policy.
While inline scripts can be useful, they're also a security concern. If
user-supplied content is unintentionally left un-sanitized, malicious users can
inject scripts into the web app.
Inline styles should be avoided in almost all cases, they should only be used
when no alternatives can be found. This allows reusability of styles as well as
readability.
[observatory-cli]: https://github.com/mozilla/http-observatory-cli
[qualys-ssl]: https://www.ssllabs.com/ssltest/analyze.html
[secure_headers]: https://github.com/twitter/secureheaders
[mdn-csp]: https://developer.mozilla.org/en-US/docs/Web/Security/CSP
[github-eng-csp]: http://githubengineering.com/githubs-csp-journey/
[dropbox-csp-1]: https://blogs.dropbox.com/tech/2015/09/on-csp-reporting-and-filtering/
[dropbox-csp-2]: https://blogs.dropbox.com/tech/2015/09/unsafe-inline-and-nonce-deployment/
[dropbox-csp-3]: https://blogs.dropbox.com/tech/2015/09/csp-the-unexpected-eval/
[dropbox-csp-4]: https://blogs.dropbox.com/tech/2015/09/csp-third-party-integrations-and-privilege-separation/
[mdn-sri]: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity
[github-eng-sri]: http://githubengineering.com/subresource-integrity/
[xss]: https://en.wikipedia.org/wiki/Cross-site_scripting
# Style guides and linting
See the relevant style guides for our guidelines and for information on linting:
## JavaScript
We defer to [Airbnb][airbnb-js-style-guide] on most style-related
conventions and enforce them with eslint.
See [our current .eslintrc][eslintrc] for specific rules and patterns.
### Common
#### ESlint
- **Never** disable eslint rules unless you have a good reason. You may see a lot of legacy files with `/* eslint-disable some-rule, some-other-rule */` at the top, but legacy files are a special case. Any time you develop a new feature or refactor an existing one, you should abide by the eslint rules.
- **Never Ever EVER** disable eslint globally for a file
```javascript
// bad
/* eslint-disable */
// better
/* eslint-disable some-rule, some-other-rule */
// best
// nothing :)
```
- If you do need to disable a rule for a single violation, try to do it as locally as possible
```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();
```
- When they are needed _always_ place ESlint directive comment blocks on the first line of a script, followed by any global declarations, then a blank newline prior to any imports or code.
```javascript
// bad
/* global Foo */
/* eslint-disable no-new */
import Bar from './bar';
// good
/* eslint-disable no-new */
/* global Foo */
import Bar from './bar';
```
- **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 */
```
#### Modules, Imports, and Exports
- Use ES module syntax to import modules
```javascript
// bad
require('foo');
// good
import Foo from 'foo';
// bad
module.exports = Foo;
// good
export default Foo;
```
- Relative paths
Unless you are writing a test, always reference other scripts using relative paths instead of `~`
In **app/assets/javascripts**:
```javascript
// bad
import Foo from '~/foo'
// good
import Foo from '../foo';
```
In **spec/javascripts**:
```javascript
// bad
import Foo from '../../app/assets/javascripts/foo'
// good
import Foo from '~/foo';
```
- Avoid using IIFE. Although we have a lot of examples of files which wrap their contents in IIFEs (immediately-invoked function expressions), 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.
- Avoid adding to the global namespace.
```javascript
// bad
window.MyClass = class { /* ... */ };
// good
export default class MyClass { /* ... */ }
```
- Side effects are forbidden in any script which contains exports
```javascript
// bad
export default class MyClass { /* ... */ }
document.addEventListener("DOMContentLoaded", function(event) {
new MyClass();
}
```
#### Data Mutation and Pure functions
- 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);
```
- Avoid constructors with side-effects
#### Parse Strings into Numbers
- `parseInt()` is preferable over `Number()` or `+`
```javascript
// bad
+'10' // 10
// good
Number('10') // 10
// better
parseInt('10', 10);
```
### Vue.js
#### Basic Rules
- Only include one Vue.js component per file.
- Export components as plain objects:
```javascript
export default {
template: `<h1>I'm a component</h1>
}
```
#### Naming
- **Extensions**: Use `.vue` extension for Vue components.
- **Reference Naming**: Use PascalCase for Vue components and camelCase for their instances:
```javascript
// bad
import cardBoard from 'cardBoard';
// good
import CardBoard from 'cardBoard'
// bad
components: {
CardBoard: CardBoard
};
// good
components: {
cardBoard: CardBoard
};
```
- **Props Naming**: Avoid using DOM component prop names.
```javascript
// bad
<component class="btn">
// good
<component cssClass="btn">
```
#### Alignment
- Follow these alignment styles for the template method:
```javascript
// bad
<component v-if="bar"
param="baz" />
// good
<component
v-if="bar"
param="baz"
/>
// if props fit in one line then keep it on the same line
<component bar="bar" />
```
#### Quotes
- 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
- Props should be declared as an object
```javascript
// bad
props: ['foo']
// good
props: {
foo: {
type: String,
required: false,
default: 'bar'
}
}
```
- 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'
}
}
```
- Default key should always be provided if the prop is not required:
```javascript
// bad
props: {
foo: {
type: String,
required: false,
}
}
// good
props: {
foo: {
type: String,
required: false,
default: 'bar'
}
}
// good
props: {
foo: {
type: String,
required: true
}
}
```
#### Data
- `data` method should always be a function
```javascript
// bad
data: {
foo: 'foo'
}
// good
data() {
return {
foo: 'foo'
};
}
```
#### Directives
- Shorthand `@` is preferable over `v-on`
```javascript
// bad
<component v-on:click="eventHandler"/>
// good
<component @click="eventHandler"/>
```
- Shorthand `:` is preferable over `v-bind`
```javascript
// bad
<component v-bind:class="btn"/>
// good
<component :class="btn"/>
```
#### Closing tags
- Prefer self closing component tags
```javascript
// bad
<component></component>
// good
<component />
```
#### Ordering
- Order for a Vue Component:
1. `name`
2. `props`
3. `data`
4. `components`
5. `computedProps`
6. `methods`
7. lifecycle methods
1. `beforeCreate`
2. `created`
3. `beforeMount`
4. `mounted`
5. `beforeUpdate`
6. `updated`
7. `activated`
8. `deactivated`
9. `beforeDestroy`
10. `destroyed`
8. `template`
## SCSS
- [SCSS](style_guide_scss.md)
[airbnb-js-style-guide]: https://github.com/airbnb/javascript
[eslintrc]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.eslintrc
...@@ -35,7 +35,7 @@ between the property and its value. ...@@ -35,7 +35,7 @@ between the property and its value.
```scss ```scss
// Bad // Bad
.container-item { .container-item {
width: 100px; height: 100px; width: 100px; height: 100px;
margin-top: 0; margin-top: 0;
} }
...@@ -63,7 +63,7 @@ between the property and its value. ...@@ -63,7 +63,7 @@ between the property and its value.
} }
``` ```
Note that there is an exception for single-line rulesets, although these are Note that there is an exception for single-line rulesets, although these are
not typically recommended. not typically recommended.
```scss ```scss
...@@ -72,7 +72,7 @@ p { margin: 0; padding: 0; } ...@@ -72,7 +72,7 @@ p { margin: 0; padding: 0; }
### Colors ### Colors
HEX (hexadecimal) colors should use shorthand where possible, and should use HEX (hexadecimal) colors should use shorthand where possible, and should use
lower case letters to differentiate between letters and numbers, e.g. `#E3E3E3` lower case letters to differentiate between letters and numbers, e.g. `#E3E3E3`
vs. `#e3e3e3`. vs. `#e3e3e3`.
...@@ -111,7 +111,7 @@ p { ...@@ -111,7 +111,7 @@ p {
### Semicolons ### Semicolons
Always include semicolons after every property. When the stylesheets are Always include semicolons after every property. When the stylesheets are
minified, the semicolons will be removed automatically. minified, the semicolons will be removed automatically.
```scss ```scss
...@@ -144,7 +144,7 @@ padding: 10px; ...@@ -144,7 +144,7 @@ padding: 10px;
### Zero Units ### Zero Units
Omit length units on zero values, they're unnecessary and not including them Omit length units on zero values, they're unnecessary and not including them
is slightly more performant. is slightly more performant.
```scss ```scss
...@@ -161,36 +161,56 @@ is slightly more performant. ...@@ -161,36 +161,56 @@ is slightly more performant.
### Selectors with a `js-` Prefix ### Selectors with a `js-` Prefix
Do not use any selector prefixed with `js-` for styling purposes. These 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 selectors are intended for use only with JavaScript to allow for removal or
renaming without breaking styling. 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:
1. There isn't already one
2. There isn't a similar one we can use instead.
## Linting ## Linting
We use [SCSS Lint][scss-lint] to check for style guide conformity. It uses the 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 ruleset in `.scss-lint.yml`, which is located in the home directory of the
project. project.
To check if any warnings will be produced by your changes, you can run `rake 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 scss_lint` in the GitLab directory. SCSS Lint will also run in GitLab CI to
catch any warnings. catch any warnings.
If the Rake task is throwing warnings you don't understand, SCSS Lint's 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]. documentation includes [a full list of their linters][scss-lint-documentation].
### Fixing issues ### Fixing issues
If you want to automate changing a large portion of the codebase to conform to 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 the SCSS style guide, you can use [CSSComb][csscomb]. First install
[Node][node] and [NPM][npm], then run `npm install csscomb -g` to 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 globally (system-wide). Run it in the GitLab directory with
`csscomb app/assets/stylesheets` to automatically fix issues with CSS/SCSS. `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. Note that this won't fix every problem, but it should fix a majority.
### Ignoring issues ### Ignoring issues
If you want a line or set of lines to be ignored by the linter, you can use If you want a line or set of lines to be ignored by the linter, you can use
`// scss-lint:disable RuleName` ([more info][disabling-linters]): `// scss-lint:disable RuleName` ([more info][disabling-linters]):
```scss ```scss
...@@ -203,8 +223,8 @@ If you want a line or set of lines to be ignored by the linter, you can use ...@@ -203,8 +223,8 @@ If you want a line or set of lines to be ignored by the linter, you can use
``` ```
Make sure a comment is added on the line above the `disable` rule, otherwise the 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 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 style guide isn't being ignored, and to communicate to others why the style
guide is ignored in this instance. guide is ignored in this instance.
[csscomb]: https://github.com/csscomb/csscomb.js [csscomb]: https://github.com/csscomb/csscomb.js
......
# Frontend Testing
There are two types of tests you'll encounter while developing frontend code
at GitLab. We use Karma and Jasmine for JavaScript unit testing, and RSpec
feature tests with Capybara for integration testing.
Feature tests need to be written for all new features. Regression tests ought
to be written for all bug fixes to prevent them from recurring in the future.
See [the Testing Standards and Style Guidelines](/doc/development/testing.md)
for more information on general testing practices at GitLab.
## Karma test suite
GitLab uses the [Karma][karma] test runner with [Jasmine][jasmine] as its test
framework for our JavaScript unit tests. For tests that rely on DOM
manipulation we use fixtures which are pre-compiled from HAML source files and
served during testing by the [jasmine-jquery][jasmine-jquery] plugin.
### Running frontend tests
`rake karma` runs the frontend-only (JavaScript) tests.
It consists of two subtasks:
- `rake karma:fixtures` (re-)generates fixtures
- `rake karma:tests` actually executes the tests
As long as the fixtures don't change, `rake karma:tests` (or `yarn karma`)
is sufficient (and saves you some time).
### Live testing and focused testing
While developing locally, it may be helpful to keep karma running so that you
can get instant feedback on as you write tests and modify code. To do this
you can start karma with `npm run karma-start`. It will compile the javascript
assets and run a server at `http://localhost:9876/` where it will automatically
run the tests on any browser which connects to it. You can enter that url on
multiple browsers at once to have it run the tests on each in parallel.
While karma is running, any changes you make will instantly trigger a recompile
and retest of the entire test suite, so you can see instantly if you've broken
a test with your changes. You can use [jasmine focused][jasmine-focus] or
excluded tests (with `fdescribe` or `xdescribe`) to get karma to run only the
tests you want while you're working on a specific feature, but make sure to
remove these directives when you commit your code.
## RSpec Feature Integration Tests
Information on setting up and running RSpec integration tests with
[Capybara][capybara] can be found in the
[general testing guide](/doc/development/testing.md).
## Gotchas
### Errors due to use of unsupported JavaScript features
Similar errors will be thrown if you're using JavaScript features not yet
supported by the PhantomJS test runner which is used for both Karma and RSpec
tests. We polyfill some JavaScript objects for older browsers, but some
features are still unavailable:
- Array.from
- Array.first
- Async functions
- Generators
- Array destructuring
- For..Of
- Symbol/Symbol.iterator
- Spread
Until these are polyfilled appropriately, they should not be used. Please
update this list with additional unsupported features.
### RSpec errors due to JavaScript
By default RSpec unit tests will not run JavaScript in the headless browser
and will simply rely on inspecting the HTML generated by rails.
If an integration test depends on JavaScript to run correctly, you need to make
sure the spec is configured to enable JavaScript when the tests are run. If you
don't do this you'll see vague error messages from the spec runner.
To enable a JavaScript driver in an `rspec` test, add `js: true` to the
individual spec or the context block containing multiple specs that need
JavaScript enabled:
```ruby
# For one spec
it 'presents information about abuse report', js: true do
# assertions...
end
describe "Admin::AbuseReports", js: true do
it 'presents information about abuse report' do
# assertions...
end
it 'shows buttons for adding to abuse report' do
# assertions...
end
end
```
### Spinach errors due to missing JavaScript
> **Note:** Since we are discouraging the use of Spinach when writing new
> feature tests, you shouldn't ever need to use this. This information is kept
> available for legacy purposes only.
In Spinach, the JavaScript driver is enabled differently. In the `*.feature`
file for the failing spec, add the `@javascript` flag above the Scenario:
```
@javascript
Scenario: Developer can approve merge request
Given I am a "Shop" developer
And I visit project "Shop" merge requests page
And merge request 'Bug NS-04' must be approved
And I click link "Bug NS-04"
When I click link "Approve"
Then I should see approved merge request "Bug NS-04"
```
[capybara]: http://teamcapybara.github.io/capybara/
[jasmine]: https://jasmine.github.io/
[jasmine-focus]: https://jasmine.github.io/2.5/focused_specs.html
[jasmine-jquery]: https://github.com/velesin/jasmine-jquery
[karma]: http://karma-runner.github.io/
# Vue
For more complex frontend features, we recommend using Vue.js. It shares
some ideas with React.js as well as Angular.
To get started with Vue, read through [their documentation][vue-docs].
## When to use Vue.js
We recommend using Vue for more complex features. Here are some guidelines for when to use Vue.js:
- If you are starting a new feature or refactoring an old one that highly interacts with the DOM;
- For real time data updates;
- If you are creating a component that will be reused elsewhere;
## When not to use Vue.js
We don't want to refactor all GitLab frontend code into Vue.js, here are some guidelines for
when not to use Vue.js:
- Adding or changing static information;
- Features that highly depend on jQuery will be hard to work with Vue.js
As always, the Frontend Architectural Experts are available to help with any Vue or JavaScript questions.
## How to build a new feature with Vue.js
**Components, Stores and Services**
In some features implemented with Vue.js, like the [issue board][issue-boards]
or [environments table][environments-table]
you can find a clear separation of concerns:
```
new_feature
├── components
│ └── component.js.es6
│ └── ...
├── store
│ └── new_feature_store.js.es6
├── service
│ └── new_feature_service.js.es6
├── new_feature_bundle.js.es6
```
_For consistency purposes, we recommend you to follow the same structure._
Let's look into each of them:
**A `*_bundle.js` file**
This is the index file of your new feature. This is where the root Vue instance
of the new feature should be.
The Store and the Service should be imported and initialized in this file and provided as a prop to the main component.
Don't forget to follow [these steps.][page_specific_javascript]
**A folder for Components**
This folder holds all components that are specific of this new feature.
If you need to use or create a component that will probably be used somewhere
else, please refer to `vue_shared/components`.
A good thumb rule to know when you should create a component is to think if
it will be reusable elsewhere.
For example, tables are used in a quite amount of places across GitLab, a table
would be a good fit for a component. On the other hand, a table cell used only
in one table would not be a good use of this pattern.
You can read more about components in Vue.js site, [Component System][component-system]
**A folder for the Store**
The Store is a class that allows us to manage the state in a single
source of truth.
The concept we are trying to follow is better explained by Vue documentation
itself, please read this guide: [State Management][state-management]
**A folder for the Service**
The Service is used only to communicate with the server.
It does not store or manipulate any data.
We use [vue-resource][vue-resource-repo] to
communicate with the server.
The [issue boards service][issue-boards-service]
is a good example of this pattern.
## Style guide
Please refer to the Vue section of our [style guide](style_guide_js.md#vuejs)
for best practices while writing your Vue components and templates.
[vue-docs]: http://vuejs.org/guide/index.html
[issue-boards]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/boards
[environments-table]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/environments
[page_specific_javascript]: https://docs.gitlab.com/ce/development/frontend.html#page-specific-javascript
[component-system]: https://vuejs.org/v2/guide/#Composing-with-Components
[state-management]: https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch
[vue-resource-repo]: https://github.com/pagekit/vue-resource
[issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6
This diff is collapsed.
...@@ -42,6 +42,20 @@ To run several tests inside one directory: ...@@ -42,6 +42,20 @@ To run several tests inside one directory:
If you want to use [Spring](https://github.com/rails/spring) set If you want to use [Spring](https://github.com/rails/spring) set
`ENABLE_SPRING=1` in your environment. `ENABLE_SPRING=1` in your environment.
## Compile Frontend Assets
You shouldn't ever need to compile frontend assets manually in development, but
if you ever need to test how the assets get compiled in a production
environment you can do so with the following command:
```
RAILS_ENV=production NODE_ENV=production bundle exec rake gitlab:assets:compile
```
This will compile and minify all JavaScript and CSS assets and copy them along
with all other frontend assets (images, fonts, etc) into `/public/assets` where
they can be easily inspected.
## Generate API documentation for project services (e.g. Slack) ## Generate API documentation for project services (e.g. Slack)
``` ```
......
...@@ -34,16 +34,17 @@ GitLab uses [factory_girl] as a test fixture replacement. ...@@ -34,16 +34,17 @@ GitLab uses [factory_girl] as a test fixture replacement.
GitLab uses [Karma] to run its [Jasmine] JavaScript specs. They can be run on GitLab uses [Karma] to run its [Jasmine] JavaScript specs. They can be run on
the command line via `bundle exec karma`. the command line via `bundle exec karma`.
- JavaScript tests live in `spec/javascripts/`, matching the folder structure of - JavaScript tests live in `spec/javascripts/`, matching the folder structure
`app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js` has a corresponding of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js`
`spec/javascripts/behaviors/autosize_spec.js` file. has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file.
- Haml fixtures required for JavaScript tests live in - Haml fixtures required for JavaScript tests live in
`spec/javascripts/fixtures`. They should contain the bare minimum amount of `spec/javascripts/fixtures`. They should contain the bare minimum amount of
markup necessary for the test. markup necessary for the test.
> **Warning:** Keep in mind that a Rails view may change and > **Warning:** Keep in mind that a Rails view may change and
invalidate your test, but everything will still pass because your fixture invalidate your test, but everything will still pass because your fixture
doesn't reflect the latest view. doesn't reflect the latest view. Because of this we encourage you to
generate fixtures from actual rails views whenever possible.
- Keep in mind that in a CI environment, these tests are run in a headless - Keep in mind that in a CI environment, these tests are run in a headless
browser and you will not have access to certain APIs, such as browser and you will not have access to certain APIs, such as
...@@ -53,6 +54,8 @@ the command line via `bundle exec karma`. ...@@ -53,6 +54,8 @@ the command line via `bundle exec karma`.
[Karma]: https://github.com/karma-runner/karma [Karma]: https://github.com/karma-runner/karma
[Jasmine]: https://github.com/jasmine/jasmine [Jasmine]: https://github.com/jasmine/jasmine
For more information, see the [frontend testing guide](fe_guide/testing.md).
## RSpec ## RSpec
### General Guidelines ### General Guidelines
......
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