From e2f94caa961680b11ab845536cca04a4940ebdc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matej=20Zrim=C5=A1ek?= Date: Wed, 13 Jul 2016 18:47:06 +0200 Subject: [PATCH] Step creation is now completelly validated on client-side also, to avoid front-end problems after server-side validation. This was needed as lots of issues were caused by this. Also step creation user experience is enriched and refactoring of related code was done. --- app/assets/javascripts/my_modules/results.js | 1 + app/assets/javascripts/protocols/steps.js | 110 +++++++++++++----- .../javascripts/sitewide/form_errors.js | 65 +++++++++-- app/assets/stylesheets/extend/bootstrap.scss | 8 +- app/views/result_assets/_edit.html.erb | 2 +- app/views/result_assets/_new.html.erb | 2 +- app/views/steps/_edit.html.erb | 4 +- app/views/steps/_form_assets.html.erb | 2 + app/views/steps/_form_checklists.html.erb | 2 +- app/views/steps/_new.html.erb | 4 +- config/locales/en.yml | 2 +- 11 files changed, 154 insertions(+), 48 deletions(-) diff --git a/app/assets/javascripts/my_modules/results.js b/app/assets/javascripts/my_modules/results.js index aa2c45a98..8ace1793f 100644 --- a/app/assets/javascripts/my_modules/results.js +++ b/app/assets/javascripts/my_modules/results.js @@ -311,6 +311,7 @@ function startFileUpload(ev, btn) { var origAssetId = assetInput ? assetInput.value : null; var url = '/asset_signature.json'; + animateSpinner(); $form.clear_form_errors(); directUpload(form, origAssetId, url, function (assetId) { diff --git a/app/assets/javascripts/protocols/steps.js b/app/assets/javascripts/protocols/steps.js index 8508b7809..f3576db37 100644 --- a/app/assets/javascripts/protocols/steps.js +++ b/app/assets/javascripts/protocols/steps.js @@ -588,37 +588,93 @@ $("[data-action='new-step']").on("ajax:success", function(e, data) { }); }); -// Needed because Paperclip deletes files on server-side validation fail (after trying to save the model) -function stepValidator( event ) { +function nameValidator(event) { + var form = $(event.target.form); var nameTooShort = $( "#step_name" ).val().length === 0; var nameTooLong = $( "#step_name" ).val().length > 50; var errMsg; if (nameTooShort) { - errMsg = I18n.t("devise.names.length_too_short"); - animateSpinner(null,false); + errMsg = I18n.t("devise.names.not_blank"); } else if (nameTooLong) { errMsg = I18n.t("devise.names.length_too_long", { max_length: 50 }); animateSpinner(null,false); } - if (!_.isUndefined(errMsg)) { - if(!$("#step_name_error_msg").length) { - $("" + errMsg + "").insertAfter("#step_name"); - $(".form-group:has(> #step_name)").addClass("has-error"); - $("#new-step-main-tab").addClass("has-error"); - } else { - $("#step_name_error_msg").html(errMsg); - } - animateSpinner(null,false); - $("#new-step-main-tab a").click(); - event.preventDefault(); + var hasErrors = !_.isUndefined(errMsg); + if (hasErrors) { + renderError($("#step_name"), errMsg, form); } + return !hasErrors; +} - clearBlankElements(event.target.form); +function checklistsValidator(event, editMode) { + var form = event.target.form; + $(form).clear_form_errors(); + var noErrors = true; + + // For every visible (i.e. not removed) checklist + $(form).find(".nested_step_checklists[style!='display: none']").each(function() { + var checklistNameInput = $(this).find(".checklist_name"); + var checklistNameEmpty = !checklistNameInput.val(); + anyChecklistItemFilled = false; + + // For every ckecklist item input + $(" .checklist-item-text", $(this)).each(function() { + if($(this).val()) { + anyChecklistItemFilled = true; + } else { + // Remove empty checklist item + $(this).closest("fieldset").remove(); + } + }) + + if(checklistNameEmpty) { + if(anyChecklistItemFilled || editMode) { + // In edit mode, name can't be blank + var errMsg = I18n.t("devise.names.not_blank"); + renderError(checklistNameInput, errMsg, $(form)); + noErrors = false; + } else { + // Hide empty checklist + $(this).hide(); + } + } + }); + + $(event.target).blur(); + return noErrors; +} + +// Needed because server-side validation failure clears locations of +// files to be uploaded and checklist's items etc. Also user +// experience is improved +function localStepValidator(event, editMode) { + if(!editMode) { + // Most td's disappear when editing step and not pressing on + // edit tab, so we can't use this function + clearBlankTables(event.target.form) + } + clearBlankFileForms(event.target.form); + var checklistsValid = checklistsValidator(event, editMode); + var nameValid = nameValidator(event); + + var noErrors = checklistsValid && nameValid; + if(noErrors) { + // Validations passed, so animate spinner for possible file + // uploading + animateSpinner(); + } + return noErrors; +} + +function S3StepValidator(event, editMode) { + if(localStepValidator(event, editMode)) { + startFileUpload(event, event.target); + } } // Expand all steps -function expandAllSteps () { +function expandAllSteps() { $('.step .panel-collapse').collapse('show'); $(document).find("[data-role='step-hot-table']").each(function() { renderTable($(this)); @@ -675,8 +731,9 @@ function startFileUpload(ev, btn) { var inputPos = 0; var inputPointer = 0; + animateSpinner(); $form.clear_form_errors(); - clearBlankElements(form); + clearBlankFileForms(form); function processFile () { var fileInput = fileInputs.get(inputPos); @@ -723,18 +780,19 @@ function startFileUpload(ev, btn) { ev.preventDefault(); } -// Clear empty fields in steps -function clearBlankElements(form) { - // Remove empty checklist items - $(form).find(".checklist-item-text").each(function () { - if ($(this).val() === ""){ +// Remove empty file forms in step +function clearBlankFileForms(form) { + $(form).find("input[type='file']").each(function () { + if (!this.files[0]) { $(this).closest("fieldset").remove(); } }); +} - // Remove empty file forms - $(form).find("input[type='file']").each(function () { - if (!this.files[0]) { +// Remove empty tables in step +function clearBlankTables(form) { + $(form).find(".nested_step_tables").each(function () { + if (!$(this).find("td:not(:empty)").length) { $(this).closest("fieldset").remove(); } }); diff --git a/app/assets/javascripts/sitewide/form_errors.js b/app/assets/javascripts/sitewide/form_errors.js index c84e9b57a..88095a07c 100644 --- a/app/assets/javascripts/sitewide/form_errors.js +++ b/app/assets/javascripts/sitewide/form_errors.js @@ -14,6 +14,7 @@ $.fn.render_form_errors_input_group = function(model_name, errors) { $.fn.render_form_errors_no_clear = function(model_name, errors, input_group) { var form = $(this); + var firstErr = true; $.each(errors, function(field, messages) { input = $(_.filter(form.find('input, select, textarea'), function(el) { var name = $(el).attr('name'); @@ -33,10 +34,22 @@ $.fn.render_form_errors_no_clear = function(model_name, errors, input_group) { } else { input.parent().append(error_text); } + + if(firstErr) { + // Focus and scroll to the first error + input.focus(); + firstErr = false; + $('html, body').animate({ + scrollTop: input.closest(".form-group").offset().top + - ($(".navbar-fixed-top").outerHeight(true) + + $(".navbar-secondary").outerHeight(true)) + }, 2000); + } }); }; $.fn.clear_form_errors = function() { + $(this).find('.nav.nav-tabs li').removeClass('has-error'); $(this).find('.form-group').removeClass('has-error'); $(this).find('span.help-block').remove(); }; @@ -79,20 +92,50 @@ $.fn.add_upload_file_size_check = function(callback) { } }; + // Show error message and mark error element and, if present, mark + // and show the tab where the error occured. + // NOTE: Similar to $.fn.render_form_errors, except here we process + // one error at a time, which is not read from the form but is + // specified manually. +function renderError(nameInput, errMsg, form) { + var errMsgSpan = nameInput.next(".help-block"); + if(!errMsgSpan.length) { + nameInput.after("" + errMsg + ""); + nameInput.closest(".form-group").addClass("has-error"); + } else { + errMsgSpan.html(errMsg); + } + tabsPropagateErrorClass($(form)); + + // Focus and scroll to the error if it is the first (most upper) one + if($(form).find(".form-group.has-error").length === 1) { + nameInput.focus(); + $('html, body').animate({ + scrollTop: nameInput.closest(".form-group").offset().top + - ($(".navbar-fixed-top").outerHeight(true) + + $(".navbar-secondary").outerHeight(true)) + }, 2000); + } + + event.preventDefault(); +} + // If any of tabs has errors, add has-error class to // parent tab navigation link function tabsPropagateErrorClass(parent) { var contents = parent.find("div.tab-pane"); - _.each(contents, function(tab) { - var $tab = $(tab); - var errorFields = $tab.find(".has-error"); - if (errorFields.length > 0) { - var id = $tab.attr("id"); - var navLink = parent.find("a[href='#" + id + "'][data-toggle='tab']"); - if (navLink.parent().length > 0) { - navLink.parent().addClass("has-error"); + if(contents.length) { + _.each(contents, function(tab) { + var $tab = $(tab); + var errorFields = $tab.find(".has-error"); + if (errorFields.length > 0) { + var id = $tab.attr("id"); + var navLink = parent.find("a[href='#" + id + "'][data-toggle='tab']"); + if (navLink.parent().length > 0) { + navLink.parent().addClass("has-error"); + } } - } - }); - $(".nav-tabs .has-error:first > a", parent).tab("show"); + }); + $(".nav-tabs .has-error:first > a", parent).tab("show"); + } } diff --git a/app/assets/stylesheets/extend/bootstrap.scss b/app/assets/stylesheets/extend/bootstrap.scss index 643480294..485c408d0 100644 --- a/app/assets/stylesheets/extend/bootstrap.scss +++ b/app/assets/stylesheets/extend/bootstrap.scss @@ -1,6 +1,7 @@ - /* Extending Bootstrap */ +@import "colors"; + /* navbar avatar image */ .navbar-nav .avatar { border-radius: 30px; @@ -11,6 +12,7 @@ top: 5px; } -.bootstrap-tagsinput > .label { - line-height: 2.3; +/* Active tab with error should retain error color if clicked on again */ +.nav-tabs > li.active.has-error > a { + color: $color-apple-blossom; } diff --git a/app/views/result_assets/_edit.html.erb b/app/views/result_assets/_edit.html.erb index d62665650..2e85146f5 100644 --- a/app/views/result_assets/_edit.html.erb +++ b/app/views/result_assets/_edit.html.erb @@ -8,7 +8,7 @@ <% end %>
<% if direct_upload %> - <%= f.submit t("result_assets.edit.update"), class: 'btn btn-primary', onclick: 'animateSpinner(); startFileUpload(event, this);' %> + <%= f.submit t("result_assets.edit.update"), class: 'btn btn-primary', onclick: 'startFileUpload(event, this);' %> <% else %> <%= f.submit t("result_assets.edit.update"), class: 'btn btn-primary', onclick: 'animateSpinner();' %> <% end %> diff --git a/app/views/result_assets/_new.html.erb b/app/views/result_assets/_new.html.erb index f9ebd1999..8b617eb9d 100644 --- a/app/views/result_assets/_new.html.erb +++ b/app/views/result_assets/_new.html.erb @@ -5,7 +5,7 @@ <%= ff.file_field :file %> <% end %> <% if direct_upload %> - <%= f.submit t("result_assets.new.create"), class: 'btn btn-primary', onclick: 'animateSpinner(); startFileUpload(event, this);' %> + <%= f.submit t("result_assets.new.create"), class: 'btn btn-primary', onclick: 'startFileUpload(event, this);' %> <% else %> <%= f.submit t("result_assets.new.create"), class: 'btn btn-primary', onclick: 'animateSpinner();' %> <% end %> diff --git a/app/views/steps/_edit.html.erb b/app/views/steps/_edit.html.erb index d4e2d1124..95bdd860e 100644 --- a/app/views/steps/_edit.html.erb +++ b/app/views/steps/_edit.html.erb @@ -5,9 +5,9 @@ <%= render partial: "empty_step.html.erb", locals: {step: @step, f: f} %>
<% if direct_upload %> - <%= f.submit t("protocols.steps.edit.edit_step"), class: 'btn btn-primary', onclick: 'animateSpinner(); startFileUpload(event, this);' %> + <%= f.submit t("protocols.steps.edit.edit_step"), class: 'btn btn-primary', onclick: 'S3StepValidator(event, true);' %> <% else %> - <%= f.submit t("protocols.steps.edit.edit_step"), class: 'btn btn-primary', onclick: 'animateSpinner(); stepValidator(event);' %> + <%= f.submit t("protocols.steps.edit.edit_step"), class: 'btn btn-primary', onclick: 'localStepValidator(event, true);' %> <% end %> <%= t("general.cancel")%> diff --git a/app/views/steps/_form_assets.html.erb b/app/views/steps/_form_assets.html.erb index 5420f0ae9..8d1568b1f 100644 --- a/app/views/steps/_form_assets.html.erb +++ b/app/views/steps/_form_assets.html.erb @@ -12,6 +12,8 @@ <% if ff.object.file.exists? %> <% if !(ff.object.file.content_type =~ /^image/).nil? %> <%= image_tag ff.object.file.url(:medium) %> +
+ <%= ff.object.file_file_name %> <% else %> <%= ff.object.file_file_name %> <% end %> diff --git a/app/views/steps/_form_checklists.html.erb b/app/views/steps/_form_checklists.html.erb index f7dba1d90..dcba5f9ee 100644 --- a/app/views/steps/_form_checklists.html.erb +++ b/app/views/steps/_form_checklists.html.erb @@ -9,7 +9,7 @@
- <%= ff.text_field :name, label: t("protocols.steps.new.checklist_name"), autofocus: true, placeholder: t("protocols.steps.new.checklist_name_placeholder") %> + <%= ff.text_field :name, label: t("protocols.steps.new.checklist_name"), class: "checklist_name", autofocus: true, placeholder: t("protocols.steps.new.checklist_name_placeholder") %> <%= ff.label t("protocols.steps.new.checklist_items") %>
    <%= ff.nested_fields_for :checklist_items, ordered_checklist_items(ff.object) do |chkItems| %> diff --git a/app/views/steps/_new.html.erb b/app/views/steps/_new.html.erb index 2df86eb46..3e1bdde7d 100644 --- a/app/views/steps/_new.html.erb +++ b/app/views/steps/_new.html.erb @@ -5,9 +5,9 @@ <%= render partial: "empty_step.html.erb", locals: {step: @step, f: f} %>
    <% if direct_upload %> - <%= f.submit t("protocols.steps.new.add_step"), class: 'btn btn-primary', onclick: 'animateSpinner(); startFileUpload(event, this);' %> + <%= f.submit t("protocols.steps.new.add_step"), class: 'btn btn-primary', onclick: 'S3StepValidator(event, false);' %> <% else %> - <%= f.submit t("protocols.steps.new.add_step"), id: "create-step", class: 'btn btn-primary', onclick: 'animateSpinner(); stepValidator(event);' %> + <%= f.submit t("protocols.steps.new.add_step"), id: "create-step", class: 'btn btn-primary', onclick: 'localStepValidator(event, false);' %> <% end %>