0

I have a form that submits using jquery, everythinh works fine. If the there is an error the browser displays the error. But the problem is if the form is resubmitted the error wont change. Either to nothing or a different message.

<script>
$(document).ready(function(e) {
    $('#addNew').submit(function(e) {
        event.preventDefault();
        var err_count = 0;
        var name_err = '';
        var name_err_count = 0;
        if($('#f_name').val() === ''){
            err_count++;
            name_err_count++;
            name_err = "You need a first name";
        }else{
            name_err = '';
        }
        if($('#l_name').val() === ''){
            err_count++;
            name_err_count++;
            if(name_err_count > 0){
                name_err += " and a last name.";
            }else{
                name_err += "You need a last name";
            }
        }else{
            name_err = '';
        }
        $('#name_err').replaceWith(name_err);
    });
});
</script>

<select id="prefix"><option>Mr</option><option>Mrs</option><option>Miss</option><option>Ms</option></select>&nbsp;<input type="text" id="f_name" name="f_name" placeholder="First Name" />&nbsp;<input type="text" id="l_name" name="l_name" placeholder="Family Name" /><br />
<div class="form_err" id="name_err"></div>
Paul Ledger
  • 1,125
  • 4
  • 21
  • 46
  • jQuery's `replaceWith` removes the selected element and adds the new content, so your `
    ` is being removed and replaced with a string. You might want to try `text()` or `html()` instead to retain the HTML structure.
    – showdev Mar 18 '14 at 21:26

3 Answers3

2

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-identified 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!

Community
  • 1
  • 1
GitaarLAB
  • 14,536
  • 11
  • 60
  • 80
  • I thought replaceWith would replace the contents, that's good to know that it replaces the whole div but I tried it because .html() and .text() didn't do anything – Paul Ledger Mar 18 '14 at 21:47
  • That is because your code (logic) had problems you might want to understand (so you don't repeat them). See my updated answer that explains what *really* went wrong (it is *not* because you 'should' use if/*else*; thereby sacrificing your original goal of dynamically building your error-message string (in a repeatable/modular way) *and* increasing the amount of DOM-access your script needs). – GitaarLAB Mar 19 '14 at 02:25
1

Actually, this should be an else if.

$(document).ready(function (e) {
    $('#addNew').submit(function (e) {
        e.preventDefault();    

        if($('#f_name').val() === '' && $('#l_name').val() === ''){
                name_err = " You need a first and a last name.";
        }else if($('#l_name').val() === '' && $('#f_name').val() !== ''){
                name_err = "You need a last name";
        }else if($('#l_name').val() !== '' && $('#f_name').val() === ''){
                name_err = "You need a first name";            
        }else{
                name_err = 'Looks Good';        
        }

        $('#name_err').html(name_err);
    });
});

…now with rock solid else if.

http://jsfiddle.net/VS3Ya/2/

4m1r
  • 12,234
  • 9
  • 46
  • 58
  • 1
    dont work either, nothing displays at all thats why I used replace With – Paul Ledger Mar 18 '14 at 21:30
  • Nope I copied it directly form jsfiddle and still nothing, but the if else statement means it check s either first name or last name not both like its supposed to. – Paul Ledger Mar 18 '14 at 21:43
  • Try that now. The else if was off before. – 4m1r Mar 18 '14 at 21:49
  • The if else statement rocks but still doesn't replace the text or add the error message. if it helps my jquery version is: jquery-1.10.1.min.js – Paul Ledger Mar 18 '14 at 21:50
  • you probably have an issue with your css. This works fine in the fiddle on safari. What browser are you using? – 4m1r Mar 18 '14 at 21:53
  • does it work in the fiddle i posted above, or just not in your own implementation? – 4m1r Mar 18 '14 at 21:55
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/49985/discussion-between-4m1r-and-paul-ledger) – 4m1r Mar 18 '14 at 21:58
  • no need for else ifs.. Or please explain why you need them (out of curiosity). Your current answer also still contains the problem with `event.preventDefault();` (as outlined in my answer) – GitaarLAB Mar 18 '14 at 22:13
  • The if else works nicely and a lot cleaner but vote up for event.preventDefault() didn't even notice that – Paul Ledger Mar 18 '14 at 22:22
  • 1
    you're right. spaced out on e. edited. why no if else? – 4m1r Mar 18 '14 at 23:44
  • Thx for the `sportsmanship` (just returned it on one of your other answers). "why no if else"? ***A*** it is not the problem (we should `teach 'm how to fish`) ***B*** it looses the OP's clear intention of dynamically building the error-message (arguably, think gzip, repeating yourself in complete strings for every scenario and increasing size) leading to the most important problem ***C*** your current (rev 3) code already *triples* DOM-access and ultimately ***D*** now extend your current existing method to validate just 5 fields... would you still recommend the 'spagetti'-result of that? – GitaarLAB Mar 19 '14 at 02:58
  • 1
    @GitaarLAB, your implementation is superior and I would rather use that in production. I could take a stab at making that a bit more elegant with a single query for storing the value of all inputs i.e. $('.check-for-vals') but what I really wanted to demonstrate here was that paul didn't really need the error counting. Also, I wanted to drive home the idea of using the selectors. Albeit inefficient, it probably makes more sense to people who are not experienced with falsy and ternary. It works, but I concede, your answer is closer to what I would want to use myself. Great thread! Peace. – 4m1r Mar 19 '14 at 18:51
  • Thx for the kind words! Just remember your 'exercise' of extending the 'else if' solution to more checks then 2! – GitaarLAB Mar 20 '14 at 19:15
0

Try this,

<script>
var err_count ;
var name_err;
var name_err_count;

$(document).ready(function(e) {
    $('#addNew').submit(function(e) {
        event.preventDefault();
        initializeVariables();
        if($('#f_name').val() === ''){
            err_count++;
            name_err_count++;
            name_err = "You need a first name";
        }
        else{
            name_err = '';
        }
        if($('#l_name').val() === ''){
            err_count++;
            name_err_count++;
            if(name_err_count > 0){
                name_err += " and a last name.";
            }
            else{
                name_err += "You need a last name";
            }
         }
        else{
            name_err = '';
        }
        $('#name_err').replaceWith(name_err);
      });
 });

function initializeVariables(){
    err_count = 0;
    name_err = '';
    name_err_count = 0;
}
</script>

I think the problem is that the variables are still holding onto the values. I am not sure about the .html() or .replaceWith(), but going by the discussion, .html() seems to be the way.

Works On Mine
  • 1,111
  • 1
  • 8
  • 20