patternjavascriptMinor
Disable button when required inputs are not filled in
Viewed 0 times
inputsaredisablefilledbuttonwhennotrequired
Problem
Currently I'm having a big form, with steps and a lot of complex fields. To make sure the user fills the required inputs and don't make them crazy pushing down the button submit when one required field on step 3 of 6 is no filled in, I decided to disable the button so they can see fast something is not right and check the form back.
My version is working but I'm not satisfied with the result because I'm sure it can be much better.
I simplify my form and I make a fiddle with my code to show the functionality:
You can check my code there.
HTML
Javascript + jQuery
```
//This is the first load of is valid, just in case the user gets back again with old inputs
//if all the required inputs are filled enable the button
var isValid = true;
$('input,textarea,select').filter('[required]:visible').each(function() {
if ( $(this).val() === '' )
isValid = false;
});
if( isValid ) {
$('#submitBtn').prop('disabled', false);
} else {
$('#submitBtn').prop('disabled', true);
};
$('#submitBtn').click(function() {
var isValid = true;
$('input,textarea,select').filter('[required]:visible').each(function() {
if ( $(this).val() === '' )
isValid = false;
});
if( isValid ) {
$("#myForm")[0].submit();
};
});
//When a input is changed check all the inputs and if all are filled, enable the button
$('input,textarea,select').change(function() {
var isValid = true;
$('input,textarea,select').filter('[required]:visible').each(function() {
if ( $(this).val() === '' )
isValid = false;
});
if( isValid ) {
$('#submitBtn').prop('disabled', false);
} else {
$('#submitBtn').prop('disabled', true);
};
});
//In case the user edits the button with firebug and removes d
My version is working but I'm not satisfied with the result because I'm sure it can be much better.
I simplify my form and I make a fiddle with my code to show the functionality:
You can check my code there.
HTML
Name *
Surname *
Age
Submit
Javascript + jQuery
```
//This is the first load of is valid, just in case the user gets back again with old inputs
//if all the required inputs are filled enable the button
var isValid = true;
$('input,textarea,select').filter('[required]:visible').each(function() {
if ( $(this).val() === '' )
isValid = false;
});
if( isValid ) {
$('#submitBtn').prop('disabled', false);
} else {
$('#submitBtn').prop('disabled', true);
};
$('#submitBtn').click(function() {
var isValid = true;
$('input,textarea,select').filter('[required]:visible').each(function() {
if ( $(this).val() === '' )
isValid = false;
});
if( isValid ) {
$("#myForm")[0].submit();
};
});
//When a input is changed check all the inputs and if all are filled, enable the button
$('input,textarea,select').change(function() {
var isValid = true;
$('input,textarea,select').filter('[required]:visible').each(function() {
if ( $(this).val() === '' )
isValid = false;
});
if( isValid ) {
$('#submitBtn').prop('disabled', false);
} else {
$('#submitBtn').prop('disabled', true);
};
});
//In case the user edits the button with firebug and removes d
Solution
Sure you can DRYfy you code in several ways.
The most obvious point is that you're using 3 times the almost identical sequence of statements, so it should be factorized.
Having isolated the sequence, we can already both simplify and improve its "core":
on the other hand, yes we might stop looping as soon as
Now it's time to look at how to really factorize, i.e. make this sequence usable each time we need it to work.
But at the same time we may also think about how to make it of more general use. The idea here is to be able to work independently (while automatically) upon all or some of several forms.
It may be achieved by wrapping the sequence in a handler:
So the whole sequence becomes a function like this:
At this step we must decide how to make this handler be launched.
It worth mention that
So for example, based on a full valid form, if user empties a required input the submit button remains enabled till navigated to another element!
It's why I suggest using
For the sake of simple usage, I suggest affecting a distinguishing class to the submit button of each form you want to be involved, say
So the way to make monitoring active will be:
This way, we know that our handler will be fired each time an element value changes.
So it's time to add what we want the handler to do depending on the form state, so it becomes:
Now, each time an element value changes the involved submit button is enabled or not depending on the whole form state.
But when page was just loaded, the buttons are always all enabled regardless of anything else.
It's why we must immediately fire the handler as soon as it has been bound:
Last point, regarding your care of user-hacking with Firebug (a bit strange, from my point of view, since if user can suppress the
A simple way to also monitor that is to fire the same handler on form
It actually results in this final whole version:
Here is a working example where I used two forms with distinct "required" exigences:
`const inputSelector = ':input[req
The most obvious point is that you're using 3 times the almost identical sequence of statements, so it should be factorized.
var isValid = true;
$('input,textarea,select').filter('[required]:visible').each(function() {
if ( $(this).val() === '' )
isValid = false;
});
// then do something with "isValid"Having isolated the sequence, we can already both simplify and improve its "core":
$(this).val()can be simpler and faster:this.value
- but as pointed by @Tushar, it should be secured:
this.value.trim();
on the other hand, yes we might stop looping as soon as
false (returning false rather than with break;, since each() is not a loop), but there can never be a significative lot of inputs so personally I'll give up, for the sake of readability- and finally the whole condition
$(this).val() === ''can be yet simplified:!this.value.trim()
Now it's time to look at how to really factorize, i.e. make this sequence usable each time we need it to work.
But at the same time we may also think about how to make it of more general use. The idea here is to be able to work independently (while automatically) upon all or some of several forms.
It may be achieved by wrapping the sequence in a handler:
- fired on any input element
- looking for valid state of all input elements in its own form
So the whole sequence becomes a function like this:
function checkForm() {
// here, "this" is an input element
var isValidForm = true;
$(this.form).find(':input[required]:visible').each(function() {
if (!this.value.trim()) {
isValidForm = false;
}
});
// then do something with "isValidForm"... not stated yet
}At this step we must decide how to make this handler be launched.
It worth mention that
change is not a good choice, because it fires only when the element value had changed AND the element lost focus.So for example, based on a full valid form, if user empties a required input the submit button remains enabled till navigated to another element!
It's why I suggest using
keyup event instead (nevertheless if checkboxes or radios should also be involved, you'll have to also use change event).For the sake of simple usage, I suggest affecting a distinguishing class to the submit button of each form you want to be involved, say
monitored-btn.So the way to make monitoring active will be:
$('.monitored-btn').closest('form').find(':input[required]:visible').keyup(checkForm);This way, we know that our handler will be fired each time an element value changes.
So it's time to add what we want the handler to do depending on the form state, so it becomes:
function checkForm() {
// here, "this" is an input element
var isValidForm = true;
$(this.form).find(':input[required]:visible').each(function() {
if (!this.value.trim()) {
isValidForm = false;
}
});
$(this.form).find('.monitored-btn').prop('disabled', !isValidForm);
}Now, each time an element value changes the involved submit button is enabled or not depending on the whole form state.
But when page was just loaded, the buttons are always all enabled regardless of anything else.
It's why we must immediately fire the handler as soon as it has been bound:
$('.monitored-btn').closest('form').find(':input[required]:visible')
// bind the handler
.keyup(checkForm)
// immediately fire it to initialize buttons state
.keyup();Last point, regarding your care of user-hacking with Firebug (a bit strange, from my point of view, since if user can suppress the
disabled attribute he can also do almost anything he wants, including directly submit through the console).A simple way to also monitor that is to fire the same handler on form
submit event, with two slight changes:- the handler expects
thisto be an input element, so we must call it like if it was a simple function (not a handler):checkForm.apply($(this).find(':input')[0]);
- apart from enabling/desabling button, the handler should now return
isValidForm, so we can use it to prevent defaut
It actually results in this final whole version:
function checkForm() {
// here, "this" is an input element
var isValidForm = true;
$(this.form).find(':input[required]:visible').each(function() {
if (!this.value.trim()) {
isValidForm = false;
}
});
$(this.form).find('.monitored-btn').prop('disabled', !isValidForm);
return isValidForm;
}
$('.monitored-btn').closest('form')
// indirectly bind the handler to form
.submit(function() {
return checkForm.apply($(this).find(':input')[0]);
})
// look for input elements
.find(':input[required]:visible')
// bind the handler to input elements
.keyup(checkForm)
// immediately fire it to initialize buttons state
.keyup();Here is a working example where I used two forms with distinct "required" exigences:
`const inputSelector = ':input[req
Code Snippets
var isValid = true;
$('input,textarea,select').filter('[required]:visible').each(function() {
if ( $(this).val() === '' )
isValid = false;
});
// then do something with "isValid"function checkForm() {
// here, "this" is an input element
var isValidForm = true;
$(this.form).find(':input[required]:visible').each(function() {
if (!this.value.trim()) {
isValidForm = false;
}
});
// then do something with "isValidForm"... not stated yet
}$('.monitored-btn').closest('form').find(':input[required]:visible').keyup(checkForm);function checkForm() {
// here, "this" is an input element
var isValidForm = true;
$(this.form).find(':input[required]:visible').each(function() {
if (!this.value.trim()) {
isValidForm = false;
}
});
$(this.form).find('.monitored-btn').prop('disabled', !isValidForm);
}$('.monitored-btn').closest('form').find(':input[required]:visible')
// bind the handler
.keyup(checkForm)
// immediately fire it to initialize buttons state
.keyup();Context
StackExchange Code Review Q#148072, answer score: 4
Revisions (0)
No revisions yet.