There are 4 problems with your code, let's tackle them in order of appearance:
You passed the event as variable e
to your submit-function, so variable event
is undefined
:
$('#addNew').submit(function(e) {
event.preventDefault();
undefined.preventDefault();
is obviously going to crash your function upon execution (so that's why it 'does not work': everything after that statement is not executed).
Fix:
$('#addNew').submit(function(event) { // or just `e` instead of `event`
event.preventDefault(); // or just `e` instead of `event`
Then you have:
err_count++;
name_err_count++;
if(name_err_count > 0){
name_err_count
will be always be > 0
since you have just (post-)incremented it..
Solution: increment your error-counter(s) after your check:
if (name_err_count > 0) { //you could omit the `> 0` check and inverse if/else
// logic here
} else {
// logic here
}
err_count++;
name_err_count++;
Next, you end with an else
on your Last Name if
-check:
}else{
name_err = '';
}
Thus, if the value of element#l_name
is set (so not ''
) your name_err
string will be cleared to ''
, even if it held an error-message generated by your previous first name check..
Fix: delete it (since your name_err
was already an empty string upon initialization and not filled by other error-messages if none of them apply).
Also have a look at If vs. else if vs. else statements?
Finally, by using $('#name_err').replaceWith(name_err);
you replace the element #name_err
with an un-id
entified textnode
(containing your error-message). When the function runs the next time, there is no #name_err
element to replace.
To fix this you should use elm.html()
(or less preferably elm.text()
)
Also have a look at this answer: What's the difference between jQuery's replaceWith() and html()?
So (applying above fixes) here is your corrected code (jsfiddle here):
$(document).ready(function (e) {
$('#addNew').submit(function (event) {
event.preventDefault();
var err_count = 0;
var name_err = '';
var name_err_count = 0;
if ($('#f_name').val() === '') {
name_err = 'You need a first name';
err_count++;
name_err_count++;
}
if ($('#l_name').val() === '') {
if (name_err_count > 0) {
name_err += ' and a last name.';
} else {
name_err += 'You need a last name';
}
err_count++;
name_err_count++;
}
$('#name_err').html(name_err);
});
});
PS, you could delete the name_error_count
and just check if your name_err
evaluates to TRUE
or FALSE
(empty strings evaluate to FALSE
in javascript) in conjunction with a ternary (and some more optimizations) for simpler/easier to read code.
$(document).ready(function(e) {
$('#addNew').submit(function(event) {
var err_count = 0,
name_err = '';
event.preventDefault();
if ($('#f_name').val() === '') {
name_err = 'a first name';
err_count++;
}
if ($('#l_name').val() === '') {
name_err += (name_err ? ' and ' : '')
+ 'a last name';
err_count++;
}
// example of simply adding a check (for a hobby)
// if ($('#hobby').val() === '') {
// name_err += (name_err ? ' and ' : '')
// + 'a hobby';
// err_count++;
// }
$('#name_err').html( name_err ? 'You need ' + name_err : '' );
});
});
Example jsfiddle here.
Another approach would be to use an array to hold your partial error-messages (that were added to it using arr.push()
). That way you can keep track/count of the error's (per section if you'd wanted, like name) by using arr.length
and convert them to a string using arr.join(' and ')
even whilst outputting your string: (arr.length ? 'You need ' + arr.join(' and ') : '')
Note: this way you could also simply obtain you total number of errors by doing err_count+=arr_section_err.length;
after each check-block. In other words, err_count++;
in every check could then also be deleted.
That would look like this:
$(document).ready(function(e) {
$('#addNew').submit(function(event) {
var err_count = 0,
name_err = [];
event.preventDefault();
if($('#f_name').val() === '') name_err.push('a first name');
if($('#l_name').val() === '') name_err.push('a last name');
// if($('#hobby').val() === '') name_err.push('a hobby'); // TRIVIAL
err_count+=name_err.length;
$('#name_err').html( name_err.length
? 'You need ' + name_err.join(' and ')
: ''
);
});
});
Example jsfiddle here.
As I have just explained that an empty string is falsy in javascript, then
if($('#f_name').val() === '') name_err.push('a first name');
equals
if(!($('#f_name').val())) name_err.push('a first name');
which equals
$('#f_name').val() || name_err.push('a first name');
$('#l_name').val() || name_err.push('a last name'); // showcasing the beauty
$('#hobby').val() || name_err.push('a hobby'); // note alignment on ||
Example jsfiddle here. Pretty elegant and simple if you ask me.
As you see, you could continue to build your error-message dynamically as you originally intended (contrary to using if/else structures that (repeatedly) check (combinations of) element-values and having complete static strings for every combination (as someone else suggested) resulting in an exponential increase in 'work' (check's and DOM-access) and a rather unmaintainable 'spaghetti'-code when checking for example just 5 elements..).
Hope this helps!