From e805393e210dd1941de7af5983c7f4326890dc98 Mon Sep 17 00:00:00 2001 From: Aidan Feldman Date: Sat, 3 Oct 2015 13:30:03 -0400 Subject: [PATCH 01/16] JS cleanup --- app/assets/javascripts/filter.js.coffee | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/filter.js.coffee b/app/assets/javascripts/filter.js.coffee index d0126b659..ec68eb0bf 100644 --- a/app/assets/javascripts/filter.js.coffee +++ b/app/assets/javascripts/filter.js.coffee @@ -5,7 +5,7 @@ class Filter addInput: ($el) -> $el.click () => @filter($el) # Initial state - if $el.is(":checked") + if $el.is(':checked') @filter($el) addRadios: () -> @@ -18,26 +18,25 @@ class Filter filter: ($el) -> value = $el.val() - if !$el.is(":checked") + if !$el.is(':checked') value = "!" + value @$("[data-filter-key=#{ @key }]").each (idx, el) -> - hidden = el.getAttribute("data-filter-value") != value + hidden = el.getAttribute('data-filter-value') != value el.setAttribute("aria-hidden", hidden.toString()) hideAll: () -> @$("[data-filter-key=#{ @key }]").attr("aria-hidden", true) - this.generateIn = ($scope) -> + @generateIn = ($scope) -> filters = {} - $scope.find("[data-filter-control]").each (idx, el) -> + $scope.find('[data-filter-control]').each (idx, el) -> key = el.getAttribute('data-filter-control') - if !filters.hasOwnProperty(key) - filters[key] = new Filter($scope, key) + filters[key] ||= new Filter($scope, key) filters $ -> # @todo - better scope - $scope = $(document) + $scope = $(document.body) filters = Filter.generateIn($scope) for key, filter of filters filter.hideAll() From e4a43baeac224f073e7ad674c58f13e0be1003d8 Mon Sep 17 00:00:00 2001 From: Aidan Feldman Date: Sat, 3 Oct 2015 14:31:21 -0400 Subject: [PATCH 02/16] refactor front-end filtering code --- app/assets/javascripts/filter.js.coffee | 66 +++++++++++-------------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/app/assets/javascripts/filter.js.coffee b/app/assets/javascripts/filter.js.coffee index ec68eb0bf..908f6ceaa 100644 --- a/app/assets/javascripts/filter.js.coffee +++ b/app/assets/javascripts/filter.js.coffee @@ -1,44 +1,38 @@ class Filter - constructor: ($root, @key) -> - @$ = (selector) -> $root.find(selector) - - addInput: ($el) -> - $el.click () => @filter($el) - # Initial state - if $el.is(':checked') - @filter($el) - - addRadios: () -> - @$("input:radio[data-filter-control=#{ @key }]").each (idx, control) => - @addInput($(control)) - - addChkBoxes: () -> - @$("input:checkbox[data-filter-control=#{ @key }]").each (idx, control) => - @addInput($(control)) - - filter: ($el) -> - value = $el.val() - if !$el.is(':checked') - value = "!" + value - @$("[data-filter-key=#{ @key }]").each (idx, el) -> - hidden = el.getAttribute('data-filter-value') != value - el.setAttribute("aria-hidden", hidden.toString()) - - hideAll: () -> - @$("[data-filter-key=#{ @key }]").attr("aria-hidden", true) + constructor: (@$root, @$control) -> + @key = @$control.data('filter-control') + @val = @$control.val() + + $: (selector) -> + @$root.find(selector) + + children: -> + @$("[data-filter-key=#{ @key }][data-filter-value=#{ @val }]") + + adjacentChildren: -> + @$("[data-filter-key=#{ @key }][data-filter-value!=#{ @val }]") + + isSelected: -> + @$control.is(':checked') + + filter: -> + if @isSelected() + @children().attr('aria-hidden', false) + @adjacentChildren().attr('aria-hidden', true) + else + @children().attr('aria-hidden', true) + + enable: -> + @filter() + @$control.change => @filter() @generateIn = ($scope) -> - filters = {} - $scope.find('[data-filter-control]').each (idx, el) -> - key = el.getAttribute('data-filter-control') - filters[key] ||= new Filter($scope, key) - filters + $scope.find('[data-filter-control]').map (idx, control) -> + new Filter($scope, $(control)) $ -> # @todo - better scope $scope = $(document.body) filters = Filter.generateIn($scope) - for key, filter of filters - filter.hideAll() - filter.addRadios() - filter.addChkBoxes() + for filter in filters + filter.enable() From 7cdcfc1a38542a10b7e4e81e01987e3bfe9a1a2e Mon Sep 17 00:00:00 2001 From: Aidan Feldman Date: Sat, 3 Oct 2015 15:39:31 -0400 Subject: [PATCH 03/16] disable inputs when they are hidden by a filter ...so that they aren't submitted with forms --- app/assets/javascripts/filter.js.coffee | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/filter.js.coffee b/app/assets/javascripts/filter.js.coffee index 908f6ceaa..2d189f360 100644 --- a/app/assets/javascripts/filter.js.coffee +++ b/app/assets/javascripts/filter.js.coffee @@ -1,3 +1,4 @@ +# TODO add tests class Filter constructor: (@$root, @$control) -> @key = @$control.data('filter-control') @@ -15,12 +16,21 @@ class Filter isSelected: -> @$control.is(':checked') + showChildren: -> + Filter.toggle(@children(), true) + + hideAdjacentChildren: -> + Filter.toggle(@adjacentChildren(), false) + + hideChildren: -> + Filter.toggle(@children(), false) + filter: -> if @isSelected() - @children().attr('aria-hidden', false) - @adjacentChildren().attr('aria-hidden', true) + @showChildren() + @hideAdjacentChildren() else - @children().attr('aria-hidden', true) + @hideChildren() enable: -> @filter() @@ -30,6 +40,12 @@ class Filter $scope.find('[data-filter-control]').map (idx, control) -> new Filter($scope, $(control)) + @toggle = ($inputWrappers, showOrHide) -> + # https://www.paciellogroup.com/blog/2012/05/html5-accessibility-chops-hidden-and-aria-hidden/ + $inputWrappers.attr('aria-hidden', !showOrHide) + # disable inputs so they aren't submitted with the form + $inputWrappers.find(':input').attr('disabled', !showOrHide) + $ -> # @todo - better scope $scope = $(document.body) From 36587454703f507020adcd24f871496ebf594306 Mon Sep 17 00:00:00 2001 From: Aidan Feldman Date: Sat, 3 Oct 2015 23:24:45 -0400 Subject: [PATCH 04/16] specify additional required fields on the WorkOrder form --- app/helpers/ncr/work_orders_helper.rb | 2 +- app/views/ncr/work_orders/form.html.haml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/ncr/work_orders_helper.rb b/app/helpers/ncr/work_orders_helper.rb index 95438b8b7..04362350a 100644 --- a/app/helpers/ncr/work_orders_helper.rb +++ b/app/helpers/ncr/work_orders_helper.rb @@ -25,7 +25,7 @@ def vendor_options(vendor = nil) def expense_type_radio_button(form, expense_type) content_tag :div, class: 'radio' do form.label :expense_type, value: expense_type do - radio = form.radio_button(:expense_type, expense_type, 'data-filter-control' => 'expense-type') + radio = form.radio_button(:expense_type, expense_type, 'data-filter-control' => 'expense-type', required: true) radio + expense_type end end diff --git a/app/views/ncr/work_orders/form.html.haml b/app/views/ncr/work_orders/form.html.haml index 2f6ff9cf6..0517879ca 100644 --- a/app/views/ncr/work_orders/form.html.haml +++ b/app/views/ncr/work_orders/form.html.haml @@ -8,7 +8,7 @@ = simple_form_for @model_instance, html: { multipart: true } do |f| = f.input :project_title = f.input :description - = field_set_tag "Expense type", class: "required" do + = field_set_tag "Expense type", class: 'required' do = expense_type_radio_button(f, 'BA60') = expense_type_radio_button(f, 'BA61') = f.input :emergency, disabled: @model_instance.persisted?, wrapper_html: { data: { filter_key: 'expense-type', filter_value: 'BA61' } } @@ -23,7 +23,7 @@ = f.input :direct_pay .form-group = label_tag :approver_email, "Approving official's email address", class: 'required' - = select_tag :approver_email, options_from_collection_for_select(approver_options, :to_s, :to_s, @approver_email), class: 'form-control js-selectize', disabled: @model_instance.approver_email_frozen?, include_blank: true, prompt: 'Type here to search' + = select_tag :approver_email, options_from_collection_for_select(approver_options, :to_s, :to_s, @approver_email), class: 'form-control js-selectize', disabled: @model_instance.approver_email_frozen?, include_blank: true, prompt: 'Type here to search', required: true - if @model_instance.new_record? = render partial: 'attachments' - else From a3e6943b4e12736cb6cd95d6ef4aa0c358249087 Mon Sep 17 00:00:00 2001 From: Aidan Feldman Date: Sat, 3 Oct 2015 23:28:08 -0400 Subject: [PATCH 05/16] enable client-side form validation --- app/assets/javascripts/selectizer.js.coffee | 4 ++++ config/initializers/simple_form.rb | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/selectizer.js.coffee b/app/assets/javascripts/selectizer.js.coffee index 1f6cbf8c6..b6c7b2851 100644 --- a/app/assets/javascripts/selectizer.js.coffee +++ b/app/assets/javascripts/selectizer.js.coffee @@ -3,6 +3,10 @@ class Selectizer @$el = $(el) @dataAttr = @$el.attr('data-attr') || 'default_field' + # `required` inputs don't work with Selectize + # https://github.com/brianreavis/selectize.js/issues/733 + @$el.removeAttr('required') + isFreeForm: -> @$el.is('input') diff --git a/config/initializers/simple_form.rb b/config/initializers/simple_form.rb index a243b4d77..96e242f71 100644 --- a/config/initializers/simple_form.rb +++ b/config/initializers/simple_form.rb @@ -117,8 +117,7 @@ # in this configuration, which is recommended due to some quirks from different browsers. # To stop SimpleForm from generating the novalidate option, enabling the HTML5 validations, # change this configuration to true. - # TODO enable, but make sure the filtered fields are only required when visible - config.browser_validations = false + config.browser_validations = true # Collection of methods to detect if a file type was given. # config.file_methods = [ :mounted_as, :file?, :public_filename ] From d07709e66831fe7dc775d6d70bfbe0af7138e97d Mon Sep 17 00:00:00 2001 From: Aidan Feldman Date: Sun, 4 Oct 2015 00:57:36 -0400 Subject: [PATCH 06/16] set up JS testing --- Gemfile | 1 + Gemfile.lock | 9 +++++++++ app/assets/javascripts/filter.js.coffee | 2 +- config/initializers/konacha.rb | 6 ++++++ doc/setup.md | 1 + spec/javascripts/filter_spec.js.coffee | 21 +++++++++++++++++++++ 6 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 config/initializers/konacha.rb create mode 100644 spec/javascripts/filter_spec.js.coffee diff --git a/Gemfile b/Gemfile index bfd9a93d8..35edcd990 100644 --- a/Gemfile +++ b/Gemfile @@ -49,6 +49,7 @@ gem 'workflow' group :test, :development do gem 'bullet', require: false # use BULLET_ENABLED=true gem 'database_cleaner' + gem 'konacha' gem 'pry-byebug' gem 'pry-rails' gem 'rspec-rails' diff --git a/Gemfile.lock b/Gemfile.lock index b4b3c6b53..41d45a2d7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -98,6 +98,7 @@ GEM coffee-script-source execjs coffee-script-source (1.9.1.1) + colorize (0.7.7) columnize (0.9.0) css_parser (1.3.7) addressable @@ -175,6 +176,13 @@ GEM turbolinks json (1.8.3) jwt (1.5.1) + konacha (3.7.0) + actionpack (>= 3.1, < 5) + capybara + colorize + railties (>= 3.1, < 5) + sprockets (>= 2, < 4) + tilt launchy (2.4.3) addressable (~> 2.3) letter_opener (1.4.1) @@ -419,6 +427,7 @@ DEPENDENCIES html_pipeline_rails jquery-rails jquery-turbolinks + konacha letter_opener letter_opener_web mail_view diff --git a/app/assets/javascripts/filter.js.coffee b/app/assets/javascripts/filter.js.coffee index 2d189f360..25e4661e2 100644 --- a/app/assets/javascripts/filter.js.coffee +++ b/app/assets/javascripts/filter.js.coffee @@ -1,5 +1,5 @@ # TODO add tests -class Filter +class @Filter constructor: (@$root, @$control) -> @key = @$control.data('filter-control') @val = @$control.val() diff --git a/config/initializers/konacha.rb b/config/initializers/konacha.rb new file mode 100644 index 000000000..d9fb65518 --- /dev/null +++ b/config/initializers/konacha.rb @@ -0,0 +1,6 @@ +if defined?(Konacha) + Konacha.configure do |config| + require 'capybara/poltergeist' + config.driver = :poltergeist + end +end diff --git a/doc/setup.md b/doc/setup.md index 38552c865..ff568d1fb 100644 --- a/doc/setup.md +++ b/doc/setup.md @@ -56,6 +56,7 @@ have it in your PATH. This is used for javascript and interface testing. ```bash ./bin/rspec +bin/rake konacha:run ``` ### Running tests as corresponding files are changed diff --git a/spec/javascripts/filter_spec.js.coffee b/spec/javascripts/filter_spec.js.coffee new file mode 100644 index 000000000..8e549049c --- /dev/null +++ b/spec/javascripts/filter_spec.js.coffee @@ -0,0 +1,21 @@ +#= require jquery +#= require filter + +describe 'Filter', -> + describe '#children()', -> + it "returns elements with the same key and value", -> + $content = $(' +
+ + + + +
+ ') + $control = $('') + + filter = new Filter($content, $control) + $children = filter.children() + expect($children.length).to.equal(1) + expect($children.data('filter-key')).to.equal('foo') + expect($children.data('filter-value')).to.equal(2) From 17498ef3d8ffdf8a2b6813001e3cfe308eda3467 Mon Sep 17 00:00:00 2001 From: Aidan Feldman Date: Sun, 4 Oct 2015 01:04:09 -0400 Subject: [PATCH 07/16] add a test for Filter#adjacentChildren() --- spec/javascripts/filter_spec.js.coffee | 32 ++++++++++++++++++-------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/spec/javascripts/filter_spec.js.coffee b/spec/javascripts/filter_spec.js.coffee index 8e549049c..04f1c19fd 100644 --- a/spec/javascripts/filter_spec.js.coffee +++ b/spec/javascripts/filter_spec.js.coffee @@ -2,20 +2,34 @@ #= require filter describe 'Filter', -> + getContent = -> + $(' +
+ + + + +
+ ') + describe '#children()', -> it "returns elements with the same key and value", -> - $content = $(' -
- - - - -
- ') $control = $('') - filter = new Filter($content, $control) + filter = new Filter(getContent(), $control) $children = filter.children() + expect($children.length).to.equal(1) expect($children.data('filter-key')).to.equal('foo') expect($children.data('filter-value')).to.equal(2) + + describe '#adjacentChildren()', -> + it "returns elements with the same key but a different value", -> + $control = $('') + + filter = new Filter(getContent(), $control) + $adjacentChildren = filter.adjacentChildren() + + expect($adjacentChildren.length).to.equal(1) + expect($adjacentChildren.data('filter-key')).to.equal('foo') + expect($adjacentChildren.data('filter-value')).to.equal(1) From d01bb776de7277fb924a075b093335425d9b3526 Mon Sep 17 00:00:00 2001 From: Aidan Feldman Date: Sun, 4 Oct 2015 01:16:42 -0400 Subject: [PATCH 08/16] add tests for Filter.toggle() --- spec/javascripts/filter_spec.js.coffee | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/spec/javascripts/filter_spec.js.coffee b/spec/javascripts/filter_spec.js.coffee index 04f1c19fd..57e5fce34 100644 --- a/spec/javascripts/filter_spec.js.coffee +++ b/spec/javascripts/filter_spec.js.coffee @@ -33,3 +33,26 @@ describe 'Filter', -> expect($adjacentChildren.length).to.equal(1) expect($adjacentChildren.data('filter-key')).to.equal('foo') expect($adjacentChildren.data('filter-value')).to.equal(1) + + describe '.toggle()', -> + it "disables the inputs", -> + $content = $(' +
+ + +
+ ') + Filter.toggle($content, false) + $input = $content.find('input') + expect($input.is(':disabled')).to.be.true + + it "disables the text areas", -> + $content = $(' +
+ +