JS understanding form validation return value

Hello,
To avoid submitting a form the JavaScript-function (see below) must return false.
Why doesn't it work like that? (see below) The return value must be put into the anonymous function. Thanks!

 document.getElementById("myform").onsubmit = function() {
                  validate();
              };
         
             function validate () {
          
                 var element = document.getElementById("error_div");
                    element.classList.toggle("error_message");
                   
                return false;
              }

Code (JavaScript):

 

Hi Iago111.

you need to pass the return value of validate() to anonymous function I guess. Try the following:

document.getElementById("myform").onsubmit = function() {
     return validate();
}

Code (JavaScript):

or:

document.getElementById("myform").onsubmit = validate();

Code (JavaScript):

 

The return value approach only works when you say "onsubmit" in the MARKUP like it's still 1997.

When you use the equally outdated Element.onsubmit it behaves more akin to the modern Element.addEventListener where you have to say "event.precentDefault();" to stop it.

You should be using addEventListener not onsubmit should other scripts wish to hook the element. As a rule of thumb, saying onsubmit in the markup or the JavaScript is outdated outmoded practices we were SUPPOSED to stop using over two decades ago.

You're also making an anonymous function for NOTHING.

document.getElementById('myform').addEventListener('submit', validate, false);

function validate(event) {
	var element = document.getElementById('error_div');
	element.classList.toggle('error_message');
	event.preventDefault();
}

Code (markup):

Note the above code is written for IE9/newer and all modern browsers. If you require IE8/earlier support... well.

NOT that JavaScript should be used for validation anymore now that we have HTML 5 attributes. Such nonsense should have gone the way of the dodo about seven or eight years ago.

Also remember that client side validation CANNOT be trusted, so be sure your server-side code is checking everything too.

 

Same problem, no return value:

  document.getElementById('myform').addEventListener('submit', validate, false);

            function validate(event) {
           
                /* var mailformat = /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/; */
            
                 var rootEmail = document.getElementById("email").value;
                 var rootSubject = document.getElementById("subject").value;
                 var rootMessage = document.getElementById("message").value;
               
                  var errorDiv = document.getElementById("error_div");
                
                  var fieldsMissing = "";
                
                  console.log(errorDiv);
                
                 if(rootEmail == "") {             
                    fieldsMissing += "Please enter an email address<br/>";
                 }
               
                 if(rootSubject == "") {
                    fieldsMissing += "Please enter a Subject<br/>";            
                 }
               
                 if(rootMessage == "") {           
                    fieldsMissing += "Please enter your message<br/>";
                 }             
                if(fieldsMissing != ""){         
                    console.log(errorDiv.style.display);  
                      
                      
                    errorDiv.style.display = "block";
                      
                    errorDiv.innerHTML = fieldsMissing;              
                    return false;  
                      
                    }
                                   
                    else {       
                    return true;                  
                    }              
              
              }
          

Code (JavaScript):

 

Again, you don't return true/false when attaching from JavaScript.

	if(fieldsMissing != ""){         
		console.log(errorDiv.style.display);  
			
		errorDiv.style.display = "block"; // you probably shouldn't be doing that
			
		errorDiv.innerHTML = fieldsMissing; // you REALLY shouldn't be using innerHTML
		event.preventDefault();
			
	}

Code (markup):

You don't even need the else / return true. That's the default behavior.

/*
	use SIF/IIFE so that we can shortcut the word "document" and to isolate
	scope so that we're invisible to other scripts. Also gives us a namespace
	in which we can pre-store values speeding up execution.
*/

(function(d) {

	/*
		static store variables outside the loop so the "longer" getBy
		operations aren't called at check time
	*/


	/*
		if you've got a bunch of var, comma delimit don't waste time saying
		"var" on each and every blasted one!
	*/
	var
		rootEmail = d.getElementById("email"),
		rootSubject = d.getElementById("subject"),
		rootMessage = d.getElementById("message"),
		errorDiv = d.getElementById("error_div"),
		errorCount ;

	// DOM flush is faster in most cases than innerHTML=''
	function flush(element) {
		while (element.firstChild) element.removeChild(element.firstChild);
	}

	// do this on the DOM! WAY faster than innerHTML and no security risks.
	function writeError(txt) {
		errorCount++;
		errorDiv.appendChild(d.createTextNode(txt));
		errorDiv.appendChild(d.createElement('br'));
	} // writeError

	function validate(event) {

		flush(errorDiv);
		errorCount = 0;

		if (rootEmail.value == '') writeError('Please enter an email address');
		if(rootSubject.value == '') writeError('Please enter a Subject');
		if(rootMessage == '') writeError('Please enter your message');
		
		if (errorCount) {
			console.log(errorDiv.style.display);
			errorDiv.style.display = "block";
			event.preventDefault();
		}
		
	} // validate

	// hook our check LAST
	d.getElementById('myform').addEventListener('submit', validate, false);

})(document);

Code (markup):

Is roughly how I'd approach that IF I were to use JavaScript for this... BUT

THESE CHECKS ARE NONE OF JAVASCRIPT'S FLIPPING BUSINESS!!!

Is there a reason you're not just adding the "required" attribute in the markup, and are instead throwing scripting at this like it's still 2009? Seriously, NOTHING you've shown or are doing so far warrants the use or even presence of JavaScript. I'd have to see the form in question, but for example if you want the e-mail field to be a valid e-mail and required, just:

<input type="email" name="email" id="contact_email" required>

Code (markup):

Since the e-mail type will only let the user type in valid e-mails, and the required field means it can't be empty. In any "modern" browser and/or IE9/newer it will implement those checks for you. NO SCRIPTING REQUIRED. Form validation has been none of JavaScript's business for a decade unless you're doing some sort of unusual / non-standard check.

 

I get the job that I want, if I'm really good at JavaScript, says the boss of the dev department of my company. I think I have to work a lot with deprecated code.

What if the flush module is not available? Should I create a non-flush workaround then?

But something must prevent the form to submit the data to the server, isn't it?

 

A big part of being good at JavaScript is knowing when NOT to use it. A lot of know-nothing poseurs throw it at everything because they aren't qualified to write a single line of HTML or CSS. Hence why to be really good at JavaScript -- at least client-side -- you NEED a far better command of HTML than the run-of-the-mill jokers who derp together trash with idiocy like frameworks or just blindly go "eh, close enough".

If your HTML is trash, everything else is trash. See why idiocy like bootcrap and jQuery destroy everything they touch.

What module? I gave you the function, or you can just do it manually. Let's say there's a div#test in the markup you want to flush.

var test = document.getElementById('div');
while (test.firstChild) test.removeChild(test.firstChild);

That is a node flush. There's no "module' or any of that nonsense involved. It's just iterating through all the children of an element and detaching them.

Yes, event.preventDefault() prevents the submit. That's what that method of the EVENT does. "event.preventDefault();" stops the default behavior of the event from... eventing. So if the event was a "submit', preventing the defaults submit behavior prevents... the submit. Doesn't get simpler than that.

It's only if you don't prevent it that it submits.

 

Ok, I understand!

 

I would do it this way:

var doc = document
  , myForm = doc.getElementById('myform')
  , email = doc.getElementById("email")
  , subject = doc.getElementById("subject")
  , message = doc.getElementById("message")
  , errDiv = doc.getElementById("error_div")
  , mailformat = /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,3})+$/;

 myForm.addEventListener('submit', function(event) {
    event.preventDefault();
   
    // client side validation
    if (email.value) {
        if(email.value.match(mailformat)) {
            return true;
        } else {
            errDiv.insertAdjacentHTML('beforeend', 'Please enter a valid email address<br/>');
            return false;
        }
    }
   
    if (!subject.value) {
        errDiv.insertAdjacentHTML('beforeend', 'Please enter a Subject<br/>');
        return false;
    }
   
    if (!message.value) {
        errDiv.insertAdjacentHTML('beforeend', 'Please enter your message<br/>');
        return false;
    }
   
    submitData();
   
 }, false);

Code (markup):

cleaner, easier to understand the code, easier to maintain by the person working after you

 

I wouldn't...

So... global scope? That's not good.

type="email" in the markup. Not JavaScript's not. Not that ANY of this is JavaScript's job, but that's just making it worse.

ALWAYS prevent? Why? Since you've got it at the start you'd have to Form.submit when there are no errors... sadly though your code is incapable of that.

You've added it as an event listener, so there's NOTHING to return. It doesn't work that way. Also doing a return true means none of the other conditions will EVER run. He looks to be trying to check all of them, not just the first on failure. Because BOTH conditions are set to return, valid OR invalid NOTHING past your first check if an e-mail is provided -- valid or invalid -- would EVER be able to even execute!

Don't even get me STARTED about the mental huffing midgetry that is dicking around with HTML as strings in the JS. insertAdjacentHTML being just as big a steaming pile of trash as innerHTML!

The what now? Unfamiliar with that function and it sure as shine-ola has nothing to do with vanilla scripting. Did you mean myForm.subnit() which would just trigger this function again in an endless loop?

Sorry my friend, but /fail/ hard. It's bad enough having JS for nothing, without diving headlong into even worse practices than his original code. In NO circumstances would the code you presented ever actually submit!

 

O.k. I'm more in favor of the "Donald Rumsfeld" version.

The purpose of flush() is to guarantee that there is nothing attached to errorDiv, right? If yes, why should there be anything attached to it?

I attached the following line to deathshadows' script, I hope that makes sense (checkEmail contains the regex):

if((rootEmail.value != '') && (checkEmail.test(rootEmail.value) == false)) writeError('Please enter a correct email address');

Code (JavaScript):

 

Yup, it's funcitonally similar to saying Element.innerHTML = '', just without the issues innerHTML can create such as longer re-render time.

Try to submit incorrectly twice in a row. You'll get the errors listed twice if you don't erase the contents of the DIV first... cause the old ones will still be there.

Hence why your original was dumping to a (pointless) string and then doing innerHTML =. You could also have done innerHTML = ''; at the start before your conditions, and then done innerHTML += 'message<br>'; which is more akin to how doing it directly on the DOM works... just in a more derpy fashion since innerHTML and its kin/kine force a reparse of the ENTIRE document every time you use it.

Rewriting mine to innerHTML -- and again, DON'T DO THIS IN PRODUCTION CODE! -- do NOT use innerHTML in real world development if it can be avoided!

/*
	use SIF/IIFE so that we can shortcut the word "document" and to isolate
	scope so that we're invisible to other scripts. Also gives us a namespace
	in which we can pre-store values speeding up execution.
*/

(function(d) {

	/*
		static store variables outside the loop so the "longer" getBy
		operations aren't called at check time
	*/


	/*
		if you've got a bunch of var, comma delimit don't waste time saying
		"var" on each and every blasted one!
	*/
	var
		rootEmail = d.getElementById("email"),
		rootSubject = d.getElementById("subject"),
		rootMessage = d.getElementById("message"),
		errorDiv = d.getElementById("error_div"),
		errorCount ;

	// do this on the DOM! WAY faster than innerHTML and no security risks.
	function writeError(txt) {
		errorCount++;
		errorDiv.innerHTML += txt + '<br>';
	} // writeError

	function validate(event) {
	
		errorCount = 0;
		errorDIV.innerHTML = '';

		if (rootEmail.value == '') writeError('Please enter an email address');
		if (rootSubject.value == '') writeError('Please enter a Subject');
		if (rootMessage == '') writeError('Please enter your message');
		
		if (errorCount) {
			console.log(errorDiv.style.display);
			errorDiv.style.display = "block";
			event.preventDefault();
		}
		
	} // validate

	// hook our check LAST
	d.getElementById('myform').addEventListener('submit', validate, false);

})(document);

Code (markup):

It would if ANY of this was ANYTHING that was ANY of JavaScript's business. Which again it isn't.

Oh, and something else... if that DIV#error_div only exists for scripting use, it shouldn't even be in the markup in the first place. Create it on the DOM from the scripting so that scripting off there's not a pointless / possibly confusing element in the way. If you have something that only works when scripting is present, it does NOT belong in the markup!

 

Yep, my bad. I just skimmed through the thread and see you suggested a code that people doesn't understand, even in the intermediate level I guess. I just want to give a simpler idea, don't expect OP to pick it up without further research. The OP's code caught my eye and I got an Eureka moment. Meanwhile... my customer is chasing me so I've only five minutes to finish it all.

I did the same thing as the OP recently.

Yep, in production, you put that in a self-invoked function.

That's what I picked from the OP's code, I have no idea if it worked.

So, that's too bad.

In reality, I listen to two separated events this way:

email.addEventListener('blur', function(event) {
    // Do the pre-screen validation here
    // Give them error messages if anything wrong
});
myForm.addEventListener('submit', function(event) {
    event.preventDefault();
    submitData(); //=> don't trust the user, so do the validation again on the server
});

Code (markup):

Do you think it is bad to do it this way?
It is the method I use currently. It works as expected.

It's just my preference. I know that appendChild is faster but it's prone to error if not handle correctly and it isn't worked with CSS.
With insertAdjacentHTML, you can style things you attached to the DOM in the stylesheet. That's marvelous.
On my defense, it executed faster than innerHTML on jsperf test.

See above.

Now, let's me told you what caught my eye. See the code below:

I'm planning to have a dark mode of my current development.
Do you think it is a good idea to change background, classes of related elements this way or switching to the entire new stylesheet. I don't know how people out there, say Google, do it. And this is the only point I want to participate on this thread.

Thank you,

 

Happens.

You used JavaScript to do things that haven't been JS' job in nearly a decade?

So that it wouldn't work is "just too bad"?!?

Conmsidering this isn't any of JavaScript's bloody business anymore? Yeah, it's bad. Particularly since if you use things like the new type="" or "required" you can use the new CSS :valid selector to do custom things to it like "warn as it's invalid".

If you're using proper semantics or add proper holders, you can go ahead and style it from where it belongs -- IN THE STYLESHEET -- doing ANY sort of style from the JavaScript is another violation of concerns and certainly NOT what I would consider "marvelous". Quite the opposite in fact.

That and if you were to use a container instead of BR there's nothing stopping you from using element.style so... Why would you even do ANY of what you just said?

If nothing else you don't set style from JavaScript unless it's for positioning or effects CSS can't handle via a simple class swap... otherwise you're just making junk code and possibly sending style to UA's where it's broken or doesn't apply. See those "media targets" I'm always talking about. Just because you have style for screen doesn't mean it's for print, or aural/speech, or TTY, or dozens of possible future UA media we might not even have thought of yet!

RE: strange function

Presentation that you don't need JavaSCript to do.

If you have an extra wrapping DIV, which these days I do as a "modal fix" so I don't end up with two scrollbars, you can put a checkbox before it and use a label to toggle the checkbox on and off just like one might with scriptless modals.

HTML:

<body>
	<input
		 type="checkbox"
		 id="toggle_nightDay"
		 class="toggle storageTrack"
		 hidden
	 >
	<div class="modalFix">
		<h1>Night Day Sample Site</h1>
		<label for="toggle_nightDay" class="nightDay"></label>
		<p>Put rest of your page content here</p>
	</div>

Code (markup):

CSS:

label {
	cursor:pointer;
}

.modalFix {
	position:absolute;
	top:0;
	left:0;
	width:100%;
	height:100%;
	overflow:auto;
	background:#EEE;
	color:#222;
}

#toggle_nightDay:checked + .modalFix {
	background:#333;
	color:#EEE;
}

h1 {
	color:#000;
}

#toggle_nightDay:checked + .modalFix h1 {
	color:#FFF;
}

.nightDay:before {
	content:"Switch to Night Mode";
}

#toggle_nightDay:checked + .modalFix .nightDay:before {
	content:"Switch to Day Mode";
}

Code (markup):

That gives you 90%+ of your day/night switch functionality. The only thing it doesn't do is remember the user choice across page-loads.

Use localstorage.THIS is where scripting gets involved.

function toggle(e) {
	localStorage.setItem(
		e.currentTarget.id,
		e.currentTarget.checked ? '1' : ''
	);
}

var toggles = document.getElementsByClassName('storageTrack');
for (var i = 0, e, value; e = toggles[i]; i++) {
	if (value = localStorage.getItem(e.id)) e.checked = value == '1';
	e.addEventListener('change', toggle, false);
}

Code (markup):

It's style, it goes in your stylesheet. If you have enough of it all-together that it becomes an issue on size, you've got too blasted much style in the first place.

though we're treading into threadjack territory with this... but still the OP might be able to learn from these methodologies. Simply put, 80%+ of what people are still doing client-side with JavaScript isn't JS' job anymore.