HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Disabling buttons on forms

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
buttonsdisablingforms

Problem

I have 5 different forms where I am disabling the submit button when a valid form is submitted so the form can only be submitted once. I am using a hidden field on each form to get the form Id.

function OnSubmit(buttonId) {
var disableButton = false;

if ($('#formType').val() == 1) {
    if ($('#CreateEntityName').val() != "" && $('#newAccountGroup').val() != "") {
        disableButton = true;
    }
}
if ($('#formType').val() == 2) {
    if ($('#newAccount').val() != "" && $('#lastName').val() != "") {
        disableButton = true;
    }
}
if ($('#formType').val() == 3) {
    if ($('#select2-entityId').val() != 0 && $('#contact').val() != "") {
        disableButton = true;
    }
}
if ($('#formType').val() == 4) {
    if ($('#select2-entityId').val() != 0 && $('#contact').val() != "" && $('#need').val() != "" && $('#candidates').val() != 0) {
        disableButton = true;
    }
}
if ($('#formType').val() == 5) {
    if ($('#newAccount').val() != "" && $('#country').val() != "") {
        disableButton = true;
    }
}
if (disableButton == true) {
    $('#' + buttonId + '').attr("disabled", true);
}
}


@Html.Hidden("formType", 5)

Solution

I see a few things that can be done to tweak this:

  • Cache your selectors instead of re-querying the DOM each time. Anything you use more than once should be cached.



  • Since all of the form fields use ID's you can use the native getElementById for faster performance.



  • Use strict equality when your are evaluating to reduce the chance of a gotcha.



  • Use 'use strict;' if you aren't already.



  • Surround your code by an IIFE, to encapsulate it and create a private scope.



  • Namespace your code



`

(function($){
  'use strict';
  var validateForm = {};

  //shortcut functions to simplify code below
  function isEmpty(str) { return str === ''; }
  function isNotSelected(num) { return num === 0; }
  function id(str) { return document.getElementById(str).value+''; }
  function getNum(str) { return document.getElementById(str).value-0; }

  function onSubmit( buttonId ) {
    var disableButton = false;
    var formType = getNum('formType');

    //cache selectors below
    var contact= id('contact');
    var newAccount =  id('newAccount');
    //assuming this is a dropdown
    var el = document.getElementById('select2-entityId');
    var entityId = el.options[ el.selectedIndex ].value -0;

    switch( formType ) {
       case 1:
        if ( isEmpty(id('CreateEntityName')) && isEmpty(id('newAccountGroup')) ) {
          disableButton = true;
        }
        break;

       case 2:
        if ( isEmpty(newAccount) && isEmpty(id('lastName')) ) {
          disableButton = true;
        }
        break;

       case 3:
        if ( isNotSelected(entityId) &&  isEmpty(contact) ) {
          disableButton = true;
        }
        break;

       case 4:
        if ( isNotSelected(entityId) && isEmpty(contact) && isEmpty(id('need')) && isNotSelected(getNum('candidates')) ) {
          disableButton = true;
        }
        break;

       case 5:
        if ( isEmpty(newAccount) && isEmpty(id('country')) ) {
          disableButton = true;
        }
        break;
    }

    if ( disableButton ) {
      $('#' + buttonId + '').prop('disabled', true);
    }

    return disableButton;
  }

  validateForm.init = onSubmit;
  window.validateForm = validateForm;

  //document.ready if you need to do something else
  $(function(){
    if ( validateForm.init() ) {
      //do something here
    }  
  });

})( jQuery );


I am sure there are other things that can be done. Hope that helps!

Code Snippets

(function($){
  'use strict';
  var validateForm = {};

  //shortcut functions to simplify code below
  function isEmpty(str) { return str === ''; }
  function isNotSelected(num) { return num === 0; }
  function id(str) { return document.getElementById(str).value+''; }
  function getNum(str) { return document.getElementById(str).value-0; }

  function onSubmit( buttonId ) {
    var disableButton = false;
    var formType = getNum('formType');

    //cache selectors below
    var contact= id('contact');
    var newAccount =  id('newAccount');
    //assuming this is a dropdown
    var el = document.getElementById('select2-entityId');
    var entityId = el.options[ el.selectedIndex ].value -0;

    switch( formType ) {
       case 1:
        if ( isEmpty(id('CreateEntityName')) && isEmpty(id('newAccountGroup')) ) {
          disableButton = true;
        }
        break;

       case 2:
        if ( isEmpty(newAccount) && isEmpty(id('lastName')) ) {
          disableButton = true;
        }
        break;

       case 3:
        if ( isNotSelected(entityId) &&  isEmpty(contact) ) {
          disableButton = true;
        }
        break;

       case 4:
        if ( isNotSelected(entityId) && isEmpty(contact) && isEmpty(id('need')) && isNotSelected(getNum('candidates')) ) {
          disableButton = true;
        }
        break;

       case 5:
        if ( isEmpty(newAccount) && isEmpty(id('country')) ) {
          disableButton = true;
        }
        break;
    }

    if ( disableButton ) {
      $('#' + buttonId + '').prop('disabled', true);
    }

    return disableButton;
  }

  validateForm.init = onSubmit;
  window.validateForm = validateForm;

  //document.ready if you need to do something else
  $(function(){
    if ( validateForm.init() ) {
      //do something here
    }  
  });

})( jQuery );

Context

StackExchange Code Review Q#88857, answer score: 2

Revisions (0)

No revisions yet.