5
\$\begingroup\$

I'm new to JavaScript and jQuery and need input about how to optimize my script:

(document).ready(function() {

    var inputs_1 = $("form#myForm .case1 input, form#myForm .case1 select");
    var inputs_2 = $("form#myForm .case2 input, form#myForm .case2 select");
    var inputs_3 = $("form#myForm .case3 input");

    $("form#myForm .case2").addClass("disabled");
    $("form#myForm .case3").addClass("disabled");
    $("form#myForm .case2 input, form#myForm .case2 select").prop('disabled', true);
    $("form#myForm .case3 input").prop('disabled', true);

    var validateInputs = function validateInputs(inputs) {
      var validForm = true;
      inputs.each(function(index) {
        var input = $(this);
        if (!input.val()) {
            $("#subnewtide").prop('disabled', true);
            validForm = false;
        }
      });
      return validForm;
    }


    inputs_1.change(function() {
      if (validateInputs(inputs_1)) {
          $("form#myForm .case2").removeClass('disabled');
          $("form#myForm .case2 input, form#myForm .case2 select").prop('disabled', false);
      }
    });

    inputs_2.change(function() {
      if (validateInputs(inputs_2)) {
          $("form#myForm .case3").removeClass('disabled');
          $("form#myForm .case3 input").prop('disabled', false);
      }
    });

    inputs_3.change(function() {
      if (validateInputs(inputs_3)) {
          $("#subnewtide").prop('disabled', false);
      }
    });

});

It is a registration form. After a user has inserted all inputs from step1 the next step should be shown. so far so good. My solution works, but I guess it is not the best way to solve my issue.

How can I abstract this more to increase maintainability?

I need a way to use one XY.change(function) handler, not for everyone a single solution.

\$\endgroup\$
  • \$\begingroup\$ I think it would be enough for form validation use some jsplugin for example this or you can find some inspiration in it. \$\endgroup\$ – Martin Jinda Nov 20 '16 at 12:53
  • 2
    \$\begingroup\$ it is not a validation. yes i check is it empty or not, but for this i dont want to use a plugin with 100mb. it should be simple, thats all. \$\endgroup\$ – JohnDoo Nov 20 '16 at 12:57
  • \$\begingroup\$ I assume there is a missing $ at the start of the code? \$\endgroup\$ – 200_success Nov 20 '16 at 16:03
  • \$\begingroup\$ Find some improvements here. Edit the question to add corresponding HTML. \$\endgroup\$ – Tushar Nov 21 '16 at 4:57
1
\$\begingroup\$

Code is pretty neat, though you might consider a couple of additional objectives :

  • ensuring that elements are re-disabled if the user revisits an earlier form field and makes it invalid.
  • allowing the validate/disable/enable system to work with as many cases as might appear in the form, without needing to amend the javascript.

I guess there are many ways these objectives could be met.

Here's one way, in which :

  • the key actions ('validate' and 'disable') are written as custom event handlers. That proves to be very convenient as it allows elements to be given behaviours that can be triggered en bloc without needing to write jQuery plugins or loop manually in the high level application code.
  • several assumptions are made about the HTML/DOM_tree, in particular that .case1, .case3, .case3 select one element each.

HTML

Add class name case to all "case" containers.

JavaScript

jQuery(function($) {
    var $cases = $('.case'),
        $finalElement = $('#subnewtide'),
        $casesPlus = $cases.add($finalElement)
        fieldSelector = 'input, select'; // a reusable selector string

    // attach event handlers to $cases
    $cases.on('validate', function(event) { // custom event
        var $this = $(this),
            n = $this.index($cases), // index of this within $cases
            valid = true;
        $casesPlus.slice(n+1).trigger('disable', [true]); // unconditionally disable everything below `this`
        $(this).find(fieldSelector).each(function() {
            if (!$(this).val()) {
                valid = false;
                return false; // break out of .each() iteration
            }
        });
        if (valid) {
            // conditionally enable next case and trigger 'validate' on it ... and so on in cascade ...
            $casesPlus.slice(n+1, n+2).trigger('disable', [false]).trigger('validate');
        }
    }).on('disable', function(event, bool) { // custom event
        if (bool) {
            $(this).addClass('disabled').find(fieldSelector).prop('disabled', true);
        } else {
            $(this).removeClass('disabled').find(fieldSelector).prop('disabled', false);
        }
    }).on('change', fieldSelector, function(event) {
        $(this).closest('.case').trigger('validate');
    });

    // attach event handlers to $finalElement
    // (slightly different from $cases)
    $finalElement.on('disable', function(event, bool) {
        $(this).prop('disabled', bool);
    });

    // initially, enable the first case and trigger 'validate' to disable everything else.
    $cases.eq(0).trigger('disable', [false]).trigger('validate');
});

untested

This style of programming tends to suit form validation and spreadsheet-like applications.

\$\endgroup\$
  • \$\begingroup\$ In the disable event handler, instead of if ... else, you can use $(this).toggleClass('disabled', bool).find(fieldSelector).prop('disabled', bool). \$\endgroup\$ – Tushar Nov 25 '16 at 3:25
  • \$\begingroup\$ Yes, that would be more concise. \$\endgroup\$ – Roamer-1888 Nov 25 '16 at 4:39
1
\$\begingroup\$

Nice work.

You can make a lot of headway by using data-* attributes. These are attributes you can add to any element to store whatever you want in them. They will be available on the .dataset property of the HTMLElement, and will always have string values. For instance, given the following HTMLELment:

<div id="some-elm" data-hello-world="JohnDoo" />

You could access the value of the data-hello-world attribute via:

const elm = document.querySelector('#some-elm');
console.log(elm.dataset.helloWold);
// -> "JohnDoo"

With that, you can generalize what you're doing with your handlers. On each case, you can store the case number (1, 2, 3, 4) in a data-case="N" attribute, and store the following case in another attribute data-next-case="N+1".

<section class="js-case" data-case="1" data-next-case="2">
  ...
</section>

This allows you to write one change (actually, "input" event) handler.

function onInput() {
  if (!isCaseValid(this)) {
    return;
  }
  const nextId = this.dataset.nextCase;
  const $nextCase = $form.find(`[data-case=${nextId}]`);
  $nextCase
    .removeClass('disabled')
    .find('.js-form-field')
    .prop('disabled', false);
}

You'll notice I'm using classes of the form js-* like js-form-field, that's because it's nice to separate CSS class names that are used purely for JS from those used for styling (more info).

The full JavaScript:

$(document).ready(function() {

  function isCaseValid(caseElm) {
    let isValid = true;
    $(caseElm).find('.js-form-field').each(function() {
        isValid = isValid && Boolean(this.value);
    });
    return isValid;
  }

  function onInput() {
    if (!isCaseValid(this)) {
      return;
    }
    const nextId = this.dataset.nextCase;
    const $nextCase = $form.find(`[data-case=${nextId}]`);
    $nextCase
      .removeClass('disabled')
      .find('.js-form-field')
      .prop('disabled', false);
  }

  const $form = $('#register-form');  
  $form.find('.js-case').on('input', onInput);

});

And, the HTML to go with it:

<form id="register-form">
  <section class="js-case" data-case="1" data-next-case="2">
    <input type="text" class="js-form-field">
    <select class="js-form-field">
      <option value>select an option</option>
      <option value="hello">hello</option>
      <option value="world">world</option>
    </select>
  </section>
  <section class="disabled js-case" data-case="2" data-next-case="3">
    <input type="text" class="js-form-field" disabled>
    <select class="js-form-field" disabled>
      <option value>select an option</option>
      <option value="hello">hello</option>
      <option value="world">world</option>
    </select>
  </section>
  <section class="disabled js-case" data-case="3" data-next-case="4">
    <input type="text" class="js-form-field" disabled>
    <select class="js-form-field" disabled>
      <option value>select an option</option>
      <option value="hello">hello</option>
      <option value="world">world</option>
    </select>
  </section>
  <section class="disabled js-case" data-case="4">
    <button class="js-form-field" disabled>
      submit
    </button>
  </section>
</form>

JSFiddle: https://jsfiddle.net/oerk5deu/

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.