diff options
Diffstat (limited to 'doc/development/fe_guide/style')
| -rw-r--r-- | doc/development/fe_guide/style/html.md | 2 | ||||
| -rw-r--r-- | doc/development/fe_guide/style/index.md | 2 | ||||
| -rw-r--r-- | doc/development/fe_guide/style/javascript.md | 8 | ||||
| -rw-r--r-- | doc/development/fe_guide/style/scss.md | 186 | ||||
| -rw-r--r-- | doc/development/fe_guide/style/vue.md | 6 |
5 files changed, 27 insertions, 177 deletions
diff --git a/doc/development/fe_guide/style/html.md b/doc/development/fe_guide/style/html.md index 61a724cf3c7..7fedbc6ce0d 100644 --- a/doc/development/fe_guide/style/html.md +++ b/doc/development/fe_guide/style/html.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # HTML style guide diff --git a/doc/development/fe_guide/style/index.md b/doc/development/fe_guide/style/index.md index 0d73b16b5d3..89a3d874184 100644 --- a/doc/development/fe_guide/style/index.md +++ b/doc/development/fe_guide/style/index.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # GitLab development style guides diff --git a/doc/development/fe_guide/style/javascript.md b/doc/development/fe_guide/style/javascript.md index b8e7429eb2c..8e3538e891d 100644 --- a/doc/development/fe_guide/style/javascript.md +++ b/doc/development/fe_guide/style/javascript.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_js.html' --- @@ -13,13 +13,13 @@ 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: **Tip:** +NOTE: 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, +when mutating data. This minimizes mutations in functions, which aligns with [Airbnb's style guide](https://github.com/airbnb/javascript#testing--for-real). ```javascript @@ -237,7 +237,7 @@ document.addEventListener("DOMContentLoaded", function(event) { ### 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 +Move them into separate functions instead. This makes tests easier to write and avoids violating the [Single Responsibility Principle](https://en.wikipedia.org/wiki/Single_responsibility_principle). ```javascript diff --git a/doc/development/fe_guide/style/scss.md b/doc/development/fe_guide/style/scss.md index c6ee1e64a80..a4cae12c4f3 100644 --- a/doc/development/fe_guide/style/scss.md +++ b/doc/development/fe_guide/style/scss.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_scss.html' --- @@ -12,7 +12,7 @@ easy to maintain, and performant for the end-user. ## Rules -Our CSS is a mixture of current and legacy approaches. That means sometimes it may be difficult to follow this guide to the letter; it means you will definitely run into exceptions, where following the guide is difficult to impossible without outsized effort. In those cases, you may work with your reviewers and maintainers to identify an approach that does not fit these rules. Please endeavor to limit these cases. +Our CSS is a mixture of current and legacy approaches. That means sometimes it may be difficult to follow this guide to the letter; it means you are likely to run into exceptions, where following the guide is difficult to impossible without outsized effort. In those cases, you may work with your reviewers and maintainers to identify an approach that does not fit these rules. Please endeavor to limit these cases. ### Utility Classes @@ -26,7 +26,7 @@ Classes in [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/a Avoid [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/). -NOTE: **Note:** +NOTE: While migrating [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/) to the [GitLab UI](https://gitlab.com/gitlab-org/gitlab-ui/-/blob/master/doc/css.md#utilities) utility classes, note both the classes for margin and padding differ. The size scale used at @@ -79,10 +79,8 @@ CSS classes should use the `lowercase-hyphenated` format rather than ``` Class names should be used instead of tag name selectors. -Using tag name selectors are discouraged in CSS because -they can affect unintended elements in the hierarchy. -Also, since they are not meaningful names, they do not -add meaning to the code. +Using tag name selectors is discouraged because they can affect +unintended elements in the hierarchy. ```scss // Bad @@ -94,136 +92,23 @@ ul { .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 +// Best +// prefer an existing utility class over adding existing styles +```0 -The shorthand form should be used for properties that support it. +Class names are also preferable to IDs. Rules that use IDs +are not-reusable, as there can only be one affected element on +the page. ```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; +#my-element { + padding: 0; } // Good -.item-with-padding { +.my-element { padding: 0; } ``` @@ -234,27 +119,11 @@ 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 an existing one. - There isn't a similar one we can use instead. ## Linting @@ -263,8 +132,8 @@ We use [SCSS Lint](https://github.com/sds/scss-lint) to check for style guide co 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/CD to +To check if any warnings are produced by your changes, run `rake +scss_lint` in the GitLab directory. SCSS Lint also runs in GitLab CI/CD to catch any warnings. If the Rake task is throwing warnings you don't understand, SCSS Lint's @@ -278,23 +147,4 @@ the SCSS style guide, you can use [CSSComb](https://github.com/csscomb/csscomb.j 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 information](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. +Note that this doesn't fix every problem, but it should fix a majority. diff --git a/doc/development/fe_guide/style/vue.md b/doc/development/fe_guide/style/vue.md index 6b6ca9a6c71..b85c1b1de35 100644 --- a/doc/development/fe_guide/style/vue.md +++ b/doc/development/fe_guide/style/vue.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Vue.js style guide @@ -98,7 +98,7 @@ Please check this [rules](https://github.com/vuejs/eslint-plugin-vue#bulb-rules) We discourage the use of the spread operator in this specific case in order to keep our codebase explicit, discoverable, and searchable. - This applies in any place where we'll benefit from the above, such as + This applies in any place where we would benefit from the above, such as when [initializing Vuex state](../vuex.md#why-not-just-spread-the-initial-state). The pattern above also enables us to easily parse non scalar values during instantiation. @@ -667,7 +667,7 @@ The goal of this accord is to make sure we are all on the same page. 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. We 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 one 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 its 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. |