From 182bcc977aec49d802bb7b511faf618b2df1d4ea Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 11 Jul 2016 12:48:00 -0600 Subject: Initial incomplete draft of the Frontend Development Guidelines. [ci skip] --- doc/development/frontend.md | 61 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 doc/development/frontend.md (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md new file mode 100644 index 00000000000..39460b72ec0 --- /dev/null +++ b/doc/development/frontend.md @@ -0,0 +1,61 @@ +# Frontend Guidelines + +This document describes various guidelines to follow to ensure good and +consistent performance of GitLab. + +## Performance + +### Per-page JavaScript + +Per-page JavaScript is JavaScript that is loaded only where necessary. + +There are some potential problems to per-page JavaScript as well. For one, + +### Minimizing page size + +A smaller page size means the page loads faster (especially important on mobile +and poor connections), the page is parsed faster by the browser, and less data is used for +users with capped data plans. + +General tips: + +- Don't add fonts that are unnecessary. +- Prefer font formats with better compression, e.g. WOFF2 is better than WOFF is better than TFF. +- Compress and minify assets wherever possible (For CSS/JS, Sprockets does this for us). +- If a piece of functionality can be reasonably done without adding extra libraries, prefer not to use extra libraries. +- Use per-page JavaScripts as described above to remove libraries that are only loaded on certain pages. + +## Accessibility + +The [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. + +## Security + +### Content Security Policy + + + +### Subresource Integrity + + + +### Including external resources + +External fonts, CSS, JavaScript should never be used with the exception of +Google Analytics - 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 reasonably be used without an iframe. + +### Avoiding inline scripts and styles + +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 site. + +[chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools +[audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules -- cgit v1.2.1 From ccbaed208b5ea3d79bafed7267d644147308556a Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 25 Jul 2016 12:15:09 -0600 Subject: Add more information on page-specific JS. [ci skip] --- doc/development/frontend.md | 64 +++++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 19 deletions(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 39460b72ec0..c2dd130c258 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -5,17 +5,40 @@ consistent performance of GitLab. ## Performance -### Per-page JavaScript +### Page-specific JavaScript -Per-page JavaScript is JavaScript that is loaded only where necessary. +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. -There are some potential problems to per-page JavaScript as well. For one, +In cases where libraries are only used on a few specific pages, we use +"Page-specific JavaScript" to prevent the main `application.js` file from +becoming unnecessarily large. + +Steps to split page-specific JavaScript from the main `application.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 precompiled assets in +`config/application.rb`. + - For example: `config.assets.precompile << "graphs/graphs_bundle.js"`. +1. Add any necessary libraries to `app/assets/javascripts/lib/`, all files directly descendant from this directory will be precompiled as separate assets. In this case, `chart.js` would be added. +1. In the relevant views, add the scripts to the page with the following: + +```haml +- content_for :page_specific_javascripts do + = page_specific_javascript_tag('lib/chart.js') + = page_specific_javascript_tag('graphs/graphs_bundle.js') +``` + +The above loads `chart.js` and `graphs_bundle.js` for only this page. `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. ### Minimizing page size A smaller page size means the page loads faster (especially important on mobile -and poor connections), the page is parsed faster by the browser, and less data is used for -users with capped data plans. +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: @@ -23,7 +46,7 @@ General tips: - Prefer font formats with better compression, e.g. WOFF2 is better than WOFF is better than TFF. - Compress and minify assets wherever possible (For CSS/JS, Sprockets does this for us). - If a piece of functionality can be reasonably done without adding extra libraries, prefer not to use extra libraries. -- Use per-page JavaScripts as described above to remove libraries that are only loaded on certain pages. +- Use page-specific JavaScripts as described above to dynamically load libraries that are only needed on certain pages. ## Accessibility @@ -35,27 +58,30 @@ Accessibility best-practices and more in-depth information is available on ## Security -### Content Security Policy - - - -### Subresource Integrity - - - ### Including external resources -External fonts, CSS, JavaScript should never be used with the exception of -Google Analytics - 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 reasonably be used without an iframe. +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], our intention is to +disable inline scripts entirely using Content Security Policy at some point in +the future. + 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 site. +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. + +[d3]: https://d3js.org/ +[chartjs]: http://www.chartjs.org/ [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules +[xss]: https://en.wikipedia.org/wiki/Cross-site_scripting -- cgit v1.2.1 From 79c2f60e9172cb7ce41ae75ae47938d6ceddd4c4 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 25 Jul 2016 12:29:43 -0600 Subject: Further revisions/additions [ci skip] --- doc/development/frontend.md | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index c2dd130c258..f6ed740a61a 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -1,7 +1,7 @@ -# Frontend Guidelines +# Frontend Development Guidelines -This document describes various guidelines to follow to ensure good and -consistent performance of GitLab. +This document describes various guidelines to ensure consistency and quality +across GitLab's frontend. ## Performance @@ -20,10 +20,12 @@ Steps to split page-specific JavaScript from the main `application.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. In `graphs_bundle.js` add the line `//= require_tree .`, this adds all other files in the directory to the bundle. +1. Add any necessary libraries to `app/assets/javascripts/lib/`, all files directly descendant from this directory will be precompiled as separate assets. In this case, `chart.js` would be added. 1. Add the new "bundle" file to the list of precompiled assets in `config/application.rb`. - For example: `config.assets.precompile << "graphs/graphs_bundle.js"`. -1. Add any necessary libraries to `app/assets/javascripts/lib/`, all files directly descendant from this directory will be precompiled as separate assets. In this case, `chart.js` would be added. +1. Move code reliant on these libraries into the `graphs` directory. 1. In the relevant views, add the scripts to the page with the following: ```haml @@ -32,7 +34,9 @@ Steps to split page-specific JavaScript from the main `application.js`: = page_specific_javascript_tag('graphs/graphs_bundle.js') ``` -The above loads `chart.js` and `graphs_bundle.js` for only this page. `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. +The above loads `chart.js` and `graphs_bundle.js` for only this page. `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. ### Minimizing page size @@ -42,7 +46,7 @@ data is used for users with capped data plans. General tips: -- Don't add fonts that are unnecessary. +- Don't add unnecessary fonts. - Prefer font formats with better compression, e.g. WOFF2 is better than WOFF is better than TFF. - Compress and minify assets wherever possible (For CSS/JS, Sprockets does this for us). - If a piece of functionality can be reasonably done without adding extra libraries, prefer not to use extra libraries. @@ -80,8 +84,15 @@ 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. +## Style Guides and Linting + +See the relevant style guides for details and information on linting: + +- [SCSS][scss-style-guide] + [d3]: https://d3js.org/ [chartjs]: http://www.chartjs.org/ [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules [xss]: https://en.wikipedia.org/wiki/Cross-site_scripting +[scss-style-guide]: scss_styleguide.md -- cgit v1.2.1 From 3b3e9d17b9f238ba1eab6c0a7e121c92f5f5d5c4 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 25 Jul 2016 13:23:19 -0600 Subject: Add CSP and SRI information [ci skip] --- doc/development/frontend.md | 74 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 3 deletions(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index f6ed740a61a..ff75fe14393 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -13,7 +13,7 @@ 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 `application.js` file from +"page-specific JavaScript" to prevent the main `application.js` file from becoming unnecessarily large. Steps to split page-specific JavaScript from the main `application.js`: @@ -21,7 +21,7 @@ Steps to split page-specific JavaScript from the main `application.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. In `graphs_bundle.js` add the line `//= require_tree .`, this adds all other files in the directory to the bundle. -1. Add any necessary libraries to `app/assets/javascripts/lib/`, all files directly descendant from this directory will be precompiled as separate assets. In this case, `chart.js` would be added. +1. Add any necessary libraries to `app/assets/javascripts/lib/`, all files directly descendant from this directory will be precompiled as separate assets, in this case `chart.js` would be added. 1. Add the new "bundle" file to the list of precompiled assets in `config/application.rb`. - For example: `config.assets.precompile << "graphs/graphs_bundle.js"`. @@ -62,6 +62,62 @@ Accessibility best-practices and more in-depth information is available on ## Security +[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. + + + ### Including external resources External fonts, CSS, and JavaScript should never be used with the exception of @@ -84,7 +140,7 @@ 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. -## Style Guides and Linting +## Style guides and linting See the relevant style guides for details and information on linting: @@ -94,5 +150,17 @@ See the relevant style guides for details and information on linting: [chartjs]: http://www.chartjs.org/ [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules +[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/ +[sprockets-sri]: https://github.com/rails/sprockets-rails#sri-support [xss]: https://en.wikipedia.org/wiki/Cross-site_scripting [scss-style-guide]: scss_styleguide.md -- cgit v1.2.1 From d4e2c4eff57291157e3990fe412c095d3a7c8de9 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Wed, 27 Jul 2016 11:21:55 -0600 Subject: Add short Testing section, minor fixes. --- doc/development/frontend.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index ff75fe14393..05ea10e754c 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -47,7 +47,7 @@ data is used for users with capped data plans. General tips: - Don't add unnecessary fonts. -- Prefer font formats with better compression, e.g. WOFF2 is better than WOFF is better than TFF. +- 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 does this for us). - If a piece of functionality can be reasonably done without adding extra libraries, prefer not to use extra libraries. - Use page-specific JavaScripts as described above to dynamically load libraries that are only needed on certain pages. @@ -146,6 +146,12 @@ See the relevant style guides for details and information on linting: - [SCSS][scss-style-guide] +## Testing + +Feature tests should be written for all new features as well as any regressions to prevent them from occuring again. + +See [the Testing Standards and Style Guidelines](testing.md) for more information. + [d3]: https://d3js.org/ [chartjs]: http://www.chartjs.org/ [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools -- cgit v1.2.1 From 7730ec2d1cc778f45ea65191b37d347f000ffecb Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Wed, 27 Jul 2016 11:51:08 -0600 Subject: Add Overview section detailing our frontend stack. [ci skip] --- doc/development/frontend.md | 49 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 5 deletions(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 05ea10e754c..cded244c0d4 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -3,8 +3,28 @@ This document describes various guidelines to ensure consistency and quality across GitLab's frontend. +## Overview + +GitLab is built on top of [Ruby on Rails][rails] using [Haml][haml] (with +[Hamlit][hamlit] for performance reasons, be wary of [the limitations this comes +with][hamlit-limits]), [SCSS][scss], and plain JavaScript with +[ES6 by way of Babel][es6]. + +The asset pipeline is [Sprockets][sprockets], which handles the concatenation, +minification, and compression of our assets. + +[jQuery][jquery] is used throughout the application's JavaScript, with +[Vue.js][vue] for particularly advanced, dynamic elements. + ## Performance +### 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. + ### Page-specific JavaScript Certain pages may require the use of a third party library, such as [d3][d3] for @@ -54,6 +74,8 @@ General tips: ## Accessibility +### Resources + The [Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools] are useful for testing for potential accessibility problems in GitLab. @@ -62,6 +84,8 @@ Accessibility best-practices and more in-depth information is available on ## 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. @@ -142,16 +166,31 @@ readability. ## Style guides and linting -See the relevant style guides for details and information on linting: +See the relevant style guides for our guidelines and for information on linting: - [SCSS][scss-style-guide] ## Testing -Feature tests should be written for all new features as well as any regressions to prevent them from occuring again. - -See [the Testing Standards and Style Guidelines](testing.md) for more information. - +Feature tests should be written for all new features as well as any regressions +to prevent them from occurring again. + +See [the Testing Standards and Style Guidelines](testing.md) for more +information. + +[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/ +[es6]: https://babeljs.io/ +[sprockets]: https://github.com/rails/sprockets +[jquery]: https://jquery.com/ +[vue]: http://vuejs.org/ +[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/ [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools -- cgit v1.2.1 From 059656ad21430c165de57f4de639f5e30e727d4c Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Thu, 4 Aug 2016 14:05:22 -0600 Subject: Add a section on vue and one on supported browsers. --- doc/development/frontend.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index cded244c0d4..9073a11a2f3 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -16,6 +16,14 @@ minification, and compression of our assets. [jQuery][jquery] is used throughout the application's JavaScript, with [Vue.js][vue] for particularly advanced, dynamic elements. +### Vue + +For more complex frontend features, we recommend using Vue.js. It shares +some ideas with React.js while being smaller and – arguably – easier to get +into. + +To get started with Vue, read through [their documentation][vue-docs]. + ## Performance ### Resources @@ -178,6 +186,10 @@ to prevent them from occurring again. See [the Testing Standards and Style Guidelines](testing.md) for more information. +## Supported browsers + +For our currently-supported browsers, see our [requirements][requirements]. + [rails]: http://rubyonrails.org/ [haml]: http://haml.info/ [hamlit]: https://github.com/k0kubun/hamlit @@ -187,6 +199,7 @@ information. [sprockets]: https://github.com/rails/sprockets [jquery]: https://jquery.com/ [vue]: http://vuejs.org/ +[vue-docs]: http://vuejs.org/guide/index.html [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 @@ -209,3 +222,4 @@ information. [sprockets-sri]: https://github.com/rails/sprockets-rails#sri-support [xss]: https://en.wikipedia.org/wiki/Cross-site_scripting [scss-style-guide]: scss_styleguide.md +[requirements]: ../install/requirements.md#supported-web-browsers -- cgit v1.2.1 From fc94c637b12eabfbf82452eee331458da7193bd6 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sun, 11 Sep 2016 15:12:42 -0600 Subject: Update Frontend Docs based on feedback. --- doc/development/frontend.md | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 9073a11a2f3..f879cd57e25 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -1,13 +1,13 @@ # Frontend Development Guidelines This document describes various guidelines to ensure consistency and quality -across GitLab's frontend. +across GitLab's frontend team. ## Overview -GitLab is built on top of [Ruby on Rails][rails] using [Haml][haml] (with -[Hamlit][hamlit] for performance reasons, be wary of [the limitations this comes -with][hamlit-limits]), [SCSS][scss], and plain JavaScript with +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 [ES6 by way of Babel][es6]. The asset pipeline is [Sprockets][sprockets], which handles the concatenation, @@ -19,8 +19,7 @@ minification, and compression of our assets. ### Vue For more complex frontend features, we recommend using Vue.js. It shares -some ideas with React.js while being smaller and – arguably – easier to get -into. +some ideas with React.js as well as Angular. To get started with Vue, read through [their documentation][vue-docs]. @@ -62,9 +61,10 @@ Steps to split page-specific JavaScript from the main `application.js`: = page_specific_javascript_tag('graphs/graphs_bundle.js') ``` -The above loads `chart.js` and `graphs_bundle.js` for only this page. `chart.js` +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. +and reused for other pages that also rely on the library. For an example, see +[this Haml file][page-specific-js-example]. ### Minimizing page size @@ -74,17 +74,17 @@ data is used for users with capped data plans. General tips: -- Don't add unnecessary fonts. +- 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 does this for us). -- If a piece of functionality can be reasonably done without adding extra libraries, prefer not to use extra libraries. -- Use page-specific JavaScripts as described above to dynamically load libraries that are only needed on certain pages. +- 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. ## Accessibility ### Resources -The [Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools] +[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 @@ -160,13 +160,11 @@ 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], our intention is to -disable inline scripts entirely using Content Security Policy at some point in -the future. +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 site. +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 @@ -180,8 +178,9 @@ See the relevant style guides for our guidelines and for information on linting: ## Testing -Feature tests should be written for all new features as well as any regressions -to prevent them from occurring again. +Feature tests need to be written for all new features. Regression tests +also need to be written for all bug fixes to prevent them from occurring +again in the future. See [the Testing Standards and Style Guidelines](testing.md) for more information. @@ -206,6 +205,7 @@ For our currently-supported browsers, see our [requirements][requirements]. [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 [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules [observatory-cli]: https://github.com/mozilla/http-observatory-cli) -- cgit v1.2.1 From abfb4f6e32ac13929678039e324307ee3ef2af43 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Wed, 12 Oct 2016 19:00:38 +0200 Subject: Document Capybara errors from es6 in es5 file. --- doc/development/frontend.md | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index f879cd57e25..56c8516508e 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -223,3 +223,14 @@ For our currently-supported browsers, see our [requirements][requirements]. [xss]: https://en.wikipedia.org/wiki/Cross-site_scripting [scss-style-guide]: scss_styleguide.md [requirements]: ../install/requirements.md#supported-web-browsers + +## Common Errors + +### Rspec (Capybara/Poltergeist) chokes on general JavaScript errors + +If you see very generic JavaScript errors (e.g. `jQuery is undefined`) being thrown in tests, but +can't reproduce them manually, you may have included `ES6`-style JavaScript in files that don't +have the `.js.es6` file extension. Either use ES5-friendly JavaScript or rename the file you're +working in (`git mv .js> `). + + -- cgit v1.2.1 From f285f4790ff4a2188fbac75fc4a70b7f31c740bb Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Tue, 25 Oct 2016 16:30:14 +0000 Subject: Add ES array methods as cause of Phantom.js errors. --- doc/development/frontend.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 56c8516508e..54890989ea1 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -224,13 +224,18 @@ For our currently-supported browsers, see our [requirements][requirements]. [scss-style-guide]: scss_styleguide.md [requirements]: ../install/requirements.md#supported-web-browsers -## Common Errors +## Gotchas -### Rspec (Capybara/Poltergeist) chokes on general JavaScript errors +### Phantom.JS (used by Teaspoon & Rspec) chokes, returning vague JavaScript errors If you see very generic JavaScript errors (e.g. `jQuery is undefined`) being thrown in tests, but can't reproduce them manually, you may have included `ES6`-style JavaScript in files that don't have the `.js.es6` file extension. Either use ES5-friendly JavaScript or rename the file you're -working in (`git mv .js> `). +working in (`git mv `). + +Similar errors will be thrown if you're using +any of the [array methods introduced in ES6](http://www.2ality.com/2014/05/es6-array-methods.html) +whether or not you've updated the file extension. + -- cgit v1.2.1 From cd3c8e9b1279dce29c5283b54d9b3305be73110f Mon Sep 17 00:00:00 2001 From: Winnie Date: Thu, 27 Oct 2016 13:15:54 +0000 Subject: Document how to run frontend tests --- doc/development/frontend.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 56c8516508e..4fb56444917 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -185,6 +185,20 @@ again in the future. See [the Testing Standards and Style Guidelines](testing.md) for more information. +### Running frontend tests + +`rake teaspoon` runs the frontend-only (JavaScript) tests. +It consists of two subtasks: + +- `rake teaspoon:fixtures` (re-)generates fixtures +- `rake teaspoon:tests` actually executes the tests + +As long as the fixtures don't change, `rake teaspoon:tests` is sufficient +(and saves you some time). + +Please note: Not all of the frontend fixtures are generated. Some are still static +files. These will not be touched by `rake teaspoon:fixtures`. + ## Supported browsers For our currently-supported browsers, see our [requirements][requirements]. -- cgit v1.2.1 From 7f0ac04db59cb290961bd77a4e0d18cffd248151 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Fri, 30 Sep 2016 17:30:21 +0200 Subject: Document convention for singleton use in front-end code. --- doc/development/frontend.md | 49 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 4fb56444917..735b41314ef 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -199,6 +199,55 @@ As long as the fixtures don't change, `rake teaspoon:tests` is sufficient Please note: Not all of the frontend fixtures are generated. Some are still static files. These will not be touched by `rake teaspoon:fixtures`. +## 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). + +``` +// bad + +gl.MyThing = { + prop1: 'hello', + method1: () => {} +}; + +// good + +class MyThing { + constructor() { + this.prop1 = 'hello'; + } + method1() {} +} + +gl.MyThing = new MyThing(); + +// best +let singleton; + +class MyThing { + constructor() { + if (!singleton) { + singleton = this; + singleton.init(); + } + return singleton; + } + + init() { + this.prop1 = 'hello'; + } + + method1() {} +} + +gl.MyThing = MyThing; +``` + ## Supported browsers For our currently-supported browsers, see our [requirements][requirements]. -- cgit v1.2.1 From 2159c8792b289bb27f9947fb834fbd497efeeb28 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Tue, 1 Nov 2016 20:40:01 +0100 Subject: Fix spacing in code sample. --- doc/development/frontend.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 735b41314ef..c6aafc926ee 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -227,14 +227,15 @@ class MyThing { gl.MyThing = new MyThing(); // best + let singleton; class MyThing { constructor() { if (!singleton) { - singleton = this; - singleton.init(); - } + singleton = this; + singleton.init(); + } return singleton; } @@ -246,6 +247,7 @@ class MyThing { } gl.MyThing = MyThing; + ``` ## Supported browsers -- cgit v1.2.1 From 06dcb0776eb2160ecff4910b9459f31ca6368507 Mon Sep 17 00:00:00 2001 From: Bryce Johnson Date: Thu, 3 Nov 2016 09:57:30 +0000 Subject: Add tip for using Chrome to run and debug teaspoon tests. --- doc/development/frontend.md | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index ece8f880542..1d7d9127a64 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -196,6 +196,12 @@ It consists of two subtasks: As long as the fixtures don't change, `rake teaspoon:tests` is sufficient (and saves you some time). +If you need to debug your tests and/or application code while they're +running, navigate to [localhost:3000/teaspoon](http://localhost:3000/teaspoon) +in your browser, open DevTools, and run tests for individual files by clicking +on them. This is also much faster than setting up and running tests from the +command line. + Please note: Not all of the frontend fixtures are generated. Some are still static files. These will not be touched by `rake teaspoon:fixtures`. -- cgit v1.2.1 From 0c36b810abede73b3cf59f951e7467dbac54ec98 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Sun, 6 Nov 2016 13:26:44 -0500 Subject: Fix broken link to observatory cli on Frontend Dev Guide --- doc/development/frontend.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/development/frontend.md') diff --git a/doc/development/frontend.md b/doc/development/frontend.md index 1d7d9127a64..ec8f2d6531c 100644 --- a/doc/development/frontend.md +++ b/doc/development/frontend.md @@ -228,7 +228,7 @@ For our currently-supported browsers, see our [requirements][requirements]. [page-specific-js-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/13bb9ed77f405c5f6ee4fdbc964ecf635c9a223f/app/views/projects/graphs/_head.html.haml#L6-8 [chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools [audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules -[observatory-cli]: https://github.com/mozilla/http-observatory-cli) +[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 -- cgit v1.2.1