From 0201825c45e9949292cc818c67e06bef26c55f36 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 8 Feb 2016 11:05:07 +0000 Subject: [PATCH 1/2] Add linting for SASS and Javascript Similar to how PEP8 enforces Python style, there are tools for front end code: - jshint[1] for Javascript - sass-lint[2] for SASS This commit adds the Gulp plugins for both, and sets up some sensible rules (which can be iterated on). It also adds a command to `./scripts/run_tests.sh`, so that any errors in the front end code will fail the build before it has a chance to be deployed. 1. http://jshint.com/ 2. https://www.npmjs.com/package/sass-lint --- .sass-lint.yml | 29 +++++++++++++++++++++++++++++ gulpfile.babel.js | 19 +++++++++++++++++++ package.json | 8 +++++++- scripts/run_tests.sh | 7 +++++-- 4 files changed, 60 insertions(+), 3 deletions(-) create mode 100644 .sass-lint.yml diff --git a/.sass-lint.yml b/.sass-lint.yml new file mode 100644 index 000000000..f36a2a515 --- /dev/null +++ b/.sass-lint.yml @@ -0,0 +1,29 @@ +options: + merge-default-rules: true + +rules: + extends-before-mixins: 2 + extends-before-declarations: 2 + placeholder-in-extend: 2 + mixins-before-declarations: + - 2 + - + exclude: + - media + no-warn: 1 + no-debug: 1 + no-ids: 1 + no-important: 2 + hex-notation: + - 2 + - + style: uppercase + indentation: + - 2 + - + size: 2 + leading-zero: 0 + nesting-depth: 0 + property-sort-order: 0 + shorthand-values: 0 + variable-for-property: 0 diff --git a/gulpfile.babel.js b/gulpfile.babel.js index 3a58ef99c..8c6ae6802 100644 --- a/gulpfile.babel.js +++ b/gulpfile.babel.js @@ -7,6 +7,7 @@ // - - - - - - - - - - - - - - - var gulp = require('gulp'), plugins = require('gulp-load-plugins')(), + stylish = require('jshint-stylish'), // 2. CONFIGURATION // - - - - - - - - - - - - - - - @@ -83,6 +84,24 @@ gulp.task('watchForChanges', function() { gulp.watch(paths.src + 'images/**/*', ['images']); }); +gulp.task('lint:sass', () => gulp + .src(paths.src + '/stylesheets/**/*.scss') + .pipe(plugins.sassLint()) + .pipe(plugins.sassLint.format(stylish)) + .pipe(plugins.sassLint.failOnError()) +); + +gulp.task('lint:js', () => gulp + .src(paths.src + 'javascripts/**/*.js') + .pipe(plugins.jshint({'esversion': 6, 'esnext': false})) + .pipe(plugins.jshint.reporter(stylish)) + .pipe(plugins.jshint.reporter('fail')) +); + +gulp.task('lint', + ['lint:sass', 'lint:js'] +); + // Default: compile everything gulp.task('default', ['copy:govuk_template:template', 'copy:govuk_template:assets', 'javascripts', 'sass', 'images'] diff --git a/package.json b/package.json index 99da9f9e8..6827f496d 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "node": "5.0.0" }, "scripts": { - "test": "echo \"Error: no test specified\" && exit 1", + "test": "gulp lint", "postinstall": "./node_modules/bower/bin/bower install", "build": "gulp", "watch": "gulp watch" @@ -35,5 +35,11 @@ "gulp-uglify": "1.5.1", "jquery": "1.11.2", "query-command-supported": "1.0.0" + }, + "devDependencies": { + "gulp-jshint": "2.0.0", + "gulp-sass-lint": "1.1.1", + "jshint": "2.9.1", + "jshint-stylish": "2.1.0" } } diff --git a/scripts/run_tests.sh b/scripts/run_tests.sh index e0a5a06cc..af5971aa8 100755 --- a/scripts/run_tests.sh +++ b/scripts/run_tests.sh @@ -25,9 +25,12 @@ function display_result { pep8 . display_result $? 1 "Code style check" +npm test +display_result $? 2 "Front end code style check" + ## Code coverage #py.test --cov=app tests/ -#display_result $? 2 "Code coverage" +#display_result $? 3 "Code coverage" py.test -v -display_result $? 3 "Unit tests" \ No newline at end of file +display_result $? 4 "Unit tests" From 3f365058ef7dcba4a51dc6482bd36ee3c0d3739c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 8 Feb 2016 11:11:55 +0000 Subject: [PATCH 2/2] Fix problems found by SASS Lint --- app/assets/stylesheets/app.scss | 39 ++++++++++++------- app/assets/stylesheets/components/banner.scss | 37 +++++++++++------- .../stylesheets/components/browse-list.scss | 17 ++++++-- .../stylesheets/components/email-message.scss | 6 +-- .../stylesheets/components/file-upload.scss | 9 +++-- .../stylesheets/components/navigation.scss | 21 ++++++---- .../stylesheets/components/page-footer.scss | 24 ++++++------ .../stylesheets/components/sms-message.scss | 30 +------------- app/assets/stylesheets/components/table.scss | 14 ++++--- .../stylesheets/components/textbox.scss | 5 ++- app/templates/components/sms-message.html | 11 +----- app/templates/views/styleguide.html | 6 --- 12 files changed, 111 insertions(+), 108 deletions(-) diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index e88b13ae4..19c15a659 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -4,39 +4,50 @@ background-color: $red; } -#global-header #logo { - white-space: nowrap; - font-size: 27px; - line-height: 32px; +#global-header { - img { - padding-right: 8px; + #logo { + + white-space: nowrap; + font-size: 27px; + line-height: 32px; + + img { + padding-right: 8px; + } + + } + + .header-proposition { + #proposition-links { + li { + padding: 0 0 0 15px; + } + } } } @include media(desktop) { - #proposition-menu { float: right; } - - #global-header .header-proposition #proposition-links li { - padding: 0 0 0 15px; - } - } -a:visited { - color: $link-colour; +a { + &:visited { + color: $link-colour; + } } .form-control-5em { + width: 100%; @include media(tablet) { width: 5em; } + } .column-main { diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index 8b79307ab..737a13a92 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -32,29 +32,35 @@ } .banner-info { - @extend .banner; + + @extend %banner; background: $govuk-blue; color: $white; - a:link, a:visited { - color: $white; - text-decoration: underline; - } + a { + + &:link, + &:visited { + color: $white; + text-decoration: underline; + } + + &:hover { + color: $light-blue-25; + } - a:hover { - color: $light-blue-25; } } .banner-dangerous { - @extend .banner; + @extend %banner; + @include bold-19; background: $white; color: $error-colour; border: 5px solid $error-colour; margin: 15px 0; - @include bold-19; text-align: left; .button { @@ -66,15 +72,18 @@ .banner-tip { - @extend .banner; + @extend %banner; background: $yellow; color: $text-colour; text-align: left; font-weight: bold; - a:link, a:visited { - color: $text-colour; - text-decoration: underline; + a { + &:link, + &:visited { + color: $text-colour; + text-decoration: underline; + } } ol { @@ -87,7 +96,7 @@ @extend %banner; background: $white; color: $text-colour; - background-image: file-url("icon-important-2x.png"); + background-image: file-url('icon-important-2x.png'); background-size: 34px 34px; background-position: 0 0; background-repeat: no-repeat; diff --git a/app/assets/stylesheets/components/browse-list.scss b/app/assets/stylesheets/components/browse-list.scss index abb95ecc0..26781ad65 100644 --- a/app/assets/stylesheets/components/browse-list.scss +++ b/app/assets/stylesheets/components/browse-list.scss @@ -7,14 +7,25 @@ margin-bottom: $gutter-two-thirds; } - a.browse-list-link { + &-link { @include bold-24; - &-destructive, - &-destructive:visited { + &-destructive { + @include bold-24; color: $error-colour; + + &:visited, + &:link { + @include bold-24; + color: $error-colour; + } + + &:hover { + color: $mellow-red; + } + } } diff --git a/app/assets/stylesheets/components/email-message.scss b/app/assets/stylesheets/components/email-message.scss index c351e4263..2eec50050 100644 --- a/app/assets/stylesheets/components/email-message.scss +++ b/app/assets/stylesheets/components/email-message.scss @@ -4,13 +4,13 @@ border: 1px solid $border-colour; &-subject { - border-bottom: 1px solid $border-colour;; - padding: 10px; @include bold-19; + border-bottom: 1px solid $border-colour; + padding: 10px; } &-body { - border-bottom: 1px solid white; + border-bottom: 1px solid $white; padding: 10px; overflow: hidden; max-height: 103px; diff --git a/app/assets/stylesheets/components/file-upload.scss b/app/assets/stylesheets/components/file-upload.scss index 5750b24d8..f772d4230 100644 --- a/app/assets/stylesheets/components/file-upload.scss +++ b/app/assets/stylesheets/components/file-upload.scss @@ -15,10 +15,11 @@ overflow: hidden; position: absolute; z-index: -1; - } - &-field:focus + .file-upload-button { - outline: 3px solid $yellow; + &:focus + .file-upload-button { + outline: 3px solid $yellow; + } + } &-button { @@ -27,9 +28,9 @@ } &-filename { + @include bold-19; display: inline-block; padding-left: $gutter-half; - @include bold-19; } } diff --git a/app/assets/stylesheets/components/navigation.scss b/app/assets/stylesheets/components/navigation.scss index 82032dea9..978b52b63 100644 --- a/app/assets/stylesheets/components/navigation.scss +++ b/app/assets/stylesheets/components/navigation.scss @@ -2,7 +2,8 @@ padding: 20px 0 0 0; - ul, h2 { + ul, + h2 { @include core-19; border-bottom: 1px solid $border-colour; margin: 10px 20px 15px 0; @@ -18,14 +19,18 @@ margin: 10px 0 0 0; } - a:link, - a:visited { - text-decoration: none; - } + a { + + &:link, + &:visited { + text-decoration: none; + } + + &:hover { + color: $link-hover-colour; + text-decoration: underline; + } - a:hover { - color: $link-hover-colour; - text-decoration: underline; } } diff --git a/app/assets/stylesheets/components/page-footer.scss b/app/assets/stylesheets/components/page-footer.scss index 2a5e07613..1df6405f1 100644 --- a/app/assets/stylesheets/components/page-footer.scss +++ b/app/assets/stylesheets/components/page-footer.scss @@ -14,16 +14,20 @@ line-height: 40px; padding: 0 0 0 5px; - a:visited, - a:link { - color: $error-colour; - display: inline-block; - vertical-align: center; - } + a { + + &:visited, + &:link { + color: $error-colour; + display: inline-block; + vertical-align: center; + } + + &:hover, + &:active { + color: $mellow-red; + } - a:hover, - a:active { - color: $mellow-red; } } @@ -33,8 +37,6 @@ margin-top: $gutter; } - .button {} - .button-destructive { @include button($error-colour); padding: 0.52632em 0.78947em 0.26316em 0.78947em; diff --git a/app/assets/stylesheets/components/sms-message.scss b/app/assets/stylesheets/components/sms-message.scss index d019a2b60..41b0e0dfb 100644 --- a/app/assets/stylesheets/components/sms-message.scss +++ b/app/assets/stylesheets/components/sms-message.scss @@ -2,7 +2,7 @@ .sms-message-wrapper { width: 100%; box-sizing: border-box; - padding: $gutter/2; + padding: $gutter-half; background: $panel-colour; border: 1px solid $panel-colour; border-radius: 5px; @@ -36,33 +36,7 @@ z-index: 50; } -label.sms-message-option { - - display: block; - position: relative; - - &.selected { - - .sms-message-wrapper-with-radio { - background: $white; - border: 1px solid $text-colour; - } - - } - - &.focused { - - outline: none; - - .sms-message-wrapper-with-radio { - box-shadow: 0 0 0 3px $yellow; - } - - } - -} - .sms-message-use-link { - margin-top: 70px; @include bold-19; + margin-top: 70px; } diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index f92472c12..650509f24 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -25,9 +25,13 @@ color: $error-colour; font-weight: bold; - a:link, - a:visited { - color: $error-colour; + a { + + &:link, + &:visited { + color: $error-colour; + } + } } @@ -50,7 +54,7 @@ } .table-field-right-aligned { - @extend .table-field; + @extend %table-field; text-align: right; } @@ -62,8 +66,8 @@ } .table-show-more-link { + @include bold-16; margin-top: -20px; border-bottom: 1px solid $border-colour; padding-bottom: 10px; - @include bold-16; } diff --git a/app/assets/stylesheets/components/textbox.scss b/app/assets/stylesheets/components/textbox.scss index 224750586..3bd01944a 100644 --- a/app/assets/stylesheets/components/textbox.scss +++ b/app/assets/stylesheets/components/textbox.scss @@ -1,5 +1,7 @@ .textbox-highlight { + $tag-background: rgba($light-blue, 0.7); + &-wrapper { position: relative; } @@ -73,8 +75,7 @@ border-radius: 3px; overflow: hidden; display: inline; - box-shadow: inset 0.75em 0 0 0 rgba($light-blue, .7), - inset -0.75em 0 0 0 rgba($light-blue, .7); + box-shadow: inset 0.75em 0 0 0 $tag-background, inset -0.75em 0 0 0 $tag-background; } } diff --git a/app/templates/components/sms-message.html b/app/templates/components/sms-message.html index 21efc76d7..044f3f5c7 100644 --- a/app/templates/components/sms-message.html +++ b/app/templates/components/sms-message.html @@ -1,9 +1,6 @@ {% macro sms_message( - body, recipient=None, name=None, id=None, edit_link=None, input_name=None, input_index=None + body, recipient=None, name=None, id=None, edit_link=None ) %} - {% if input_name %} - - {% endif %} {% endmacro %} diff --git a/app/templates/views/styleguide.html b/app/templates/views/styleguide.html index da99a5d9a..9c0d2006c 100644 --- a/app/templates/views/styleguide.html +++ b/app/templates/views/styleguide.html @@ -140,12 +140,6 @@ name='Two week reminder', edit_link='#' ) }} - {{ sms_message( - "Your vehicle tax for ((registration number)) is due on ((date)). Renew online at www.gov.uk/vehicle-tax", - name="Reminder", - input_name="template", - input_index=1 - ) }}