I have a problem with form validation in JS DOM

Question

When i click a field and pass another, span tag is getting red color. Then i press the submit button it is showing alert message. But when i turn to red span and fill in the field and press submit button it is showing success even if other fields are blank.

const regForm = document.getElementById('regForm');
var validObjects = document.querySelectorAll('[customValidate]');
validObjects.forEach(function(element) {
    element.addEventListener('blur', function() {
        var emoji = element.previousElementSibling;
        var label = emoji.previousElementSibling;

        if (!element.value) {
            emoji.className = "far fa-frown float-right  text-danger";
            var span = document.createElement("span");
            span.innerHTML = "&emsp;* Required";
            span.style.color = "red";
            if (!label.getElementsByTagName("span")[0])
                label.appendChild(span);
            isValid = false;
        } else {
            emoji.className = "far fa-smile float-right  text-success";
            var span = label.getElementsByTagName("span")[0];
            if (span)
                label.removeChild(span);
            isValid = true;
        }
    });
});
regForm.addEventListener('submit', function(event) {
    event.preventDefault();
    var isValid = true;

    validObjects.forEach(function(element) {
        isValid = element.value ? true : false;
    })
    if (!isValid) {
        alert("empty!");
    } else {
        alert("success!");
    }
});

JSFiddle :https://jsfiddle.net/roop06/cjmdabrf/

epascarello · Answer 1 · 2019-12-16T18:49:10.967

2

because isValid is only going to be equal to the last item in the forEach

validObjects.forEach(function(element) {
    isValid = element.value ? true : false; // replaces false with true on last iteration
})

If you want to use a forEach you would want to code it like this so it does not overwrite isValid. It uses its previous state.

var isValid = true;
validObjects.forEach(function(element) {
    isValid = element.value ? isValid : false;
})

But if you are not doing anything else in the forEach loop, there is a better option. That option is to use every which will exit when it gets to false.

var isValid = validObjects.every(function (element) {
  return element.value.length
})

var form = document.querySelector('form');
var validObjects = Array.from(document.querySelectorAll('[customValidate]'));
form.addEventListener("submit", function (e) {
  var isValid = validObjects.every(function (element) {
    return element.value.length
  })
  return isValid
})

<form>
  <input customValidate>
  <input customValidate>
  <input customValidate>
  <button>submit</button>
</form>

Or you can just use the built in HTML5 validation using required and let the browser do it for you.

<form>
  <input customValidate required>
  <input customValidate required>
  <input customValidate required>
  <button>submit</button>
</form>

edited Dec 16 '19 at 18:49

answered Dec 16 '19 at 14:36

epascarello

204,599
20
195
236

i have to do pure js. I can't use HTML5 required. – Roop Dec 16 '19 at 14:59
1

It is HTML. Nothing magical about HTML. You should be using `every()`, not each. Guessing you did not actually read the answer. Every is a better solution because it exits when it encounters a wrong value. So you are not going to every element when you know it is already failed validation. – epascarello Dec 16 '19 at 15:00
@epascarello FWIW, if your concern is with speed, a classic for loop with a break statement is still over 5x faster. Tested via Benchmark.js. But I fully agree that `every` is better than `forEach` here. – Quangdao Nguyen Dec 16 '19 at 17:49
@Roop not that I would suggest this, but you technically _could_ use HTML5 required by setting it in JS and it would work just the same. – Quangdao Nguyen Dec 16 '19 at 17:50
Thanks for advice but they want to me use to JS. – Roop Dec 16 '19 at 17:57
@Roop my solution with `every()` has nothing to do with HTML5. The HTML5 comment you are hung up on was a one line suggestion at the end that it would be better to use. – epascarello Dec 16 '19 at 18:42

score 0 · Answer 2 · answered Dec 16 '19 at 14:42

The problem you have is that if the last field is valid then the isValid flag will always be true. One way to get around this is to stop setting the flag once you have determined that there is an invalid field:

validObjects.forEach(function (element) {
    if (isValid) {
        isValid = element.value ? true : false;
    }
});

BrOsCoRe · Accepted Answer · 2019-12-16T14:55:16.057

0

Try this

JSFiddle

validObjects.forEach(function(element) {
       if(!(element.value)){
        isValid = false;
       }
    })

edited Dec 16 '19 at 14:55

answered Dec 16 '19 at 14:43

BrOsCoRe

73
1
10

[`return;` does nothing](https://stackoverflow.com/questions/2641347/short-circuit-array-foreach-like-calling-break) – epascarello Dec 16 '19 at 14:44
@epascarello edited. he set isValid true as standard. Now it iterates over every Element and if it is false then it sets isValid to false and exits. Only if all are true isValid still is true. – BrOsCoRe Dec 16 '19 at 14:45
You can not exit a forEach – epascarello Dec 16 '19 at 14:46
@epascarello Ok I see what you mean. I just can leave out the return. Sorry ^^ – BrOsCoRe Dec 16 '19 at 14:55
1

This does work, but forEach is the wrong solution. It should be `every()`. Enjoy the extra loops. – epascarello Dec 16 '19 at 15:02

I have a problem with form validation in JS DOM

3 Answers3