CODEWITHSUNDEEP
javascripthtml
Alvaro Gomez
Alvaro Gomez

Reputation: 360

onclick event on submit button

I'm trying to make a verification that an input text has an email format after clicking submit buttom and calling a js function. The first problem i encounter is that after some tests, i've seen that it doesnt enter the called function. After this, i think everything should be ok, but just in case in order to not post 2 almost equal questions within minutes, ill include the most important part. Summarizing, it should check: -The email field is not null -The email field has an @ (without taking into account order etc)

Then it should tell if it found any problem, if not leave everything unchanged

I hope i made my point, if not i can try to explain it again..

 <input type="text" name="email" id="email">
    <input type="submit"  onclick=" proceed()"/>
<script>
    proceed(){
        var email= document.getElementById('email').value;
        var problems;
        if (email == ""){
            problems = "Empty variable \n";

        }

        var noat = true;
        for (int i=0; email.length; i++){
            if (email.charAt(i) == "@"){  //Compare each character
                noat=false;
                break;
            }
        }

        if (email=="" || noat=true){
            problems += "No @ \n"
            alert(problems);                        
        }
    }

</script>

Upvotes: 3

Views: 32810

Answers (4)

Marty Cortez
Marty Cortez

Reputation: 2343

This question has already been marked as answered, but I feel that there a few key takeaways / learning points from the question, so i've added my input below, fwiw.

  1. Right now the code in your question starts off with proceed() { ... }. You should change this to function proceed() { ... } to make it a proper function declaration
  2. It's a good habit to define your variables at the top of your scope in which they are being used. As of now you have the email and problems vars declared next to each other, and that's good. However, var noat is all by itself a few lines down. Moving it up to the other declarations helps the reader of your code understand which variables are to be used in this function
  3. On the fourth line of your function you check to see if email is an empty string. If it is, you set the problems variable. But if it's empty, we can stop where we are. You can just return problems; and be done with the function.
  4. It's already been pointed out, but email.indexOf("@") uses the native JS method to do what you're doing in the string iterator.
  5. for(int i=0; ...) I thought the int i =0 looked funny, so I tried typing it into the javascript console on my browser. I got an error. You can change this to for(var i = 0; ...) for the correct syntax.
  6. Later on in the function we ask if email is empty again: if(email == ""). But we've already asked this question, and would have exited the function if it was true. We can't have an @ symbol in the string if the string is empty.
  7. This has been mentioned before but you probably want to use a regular expression to check the email.
  8. Next we have noat = true. This will actually always evaluate to true because the result of the assignment is truthy. You're setting the value of noat to true, and javascript is like "Cool awesome looks good to me". It doesn't check if noat was originally set to true or false.

I've created and tested a jsfiddle that follows most of these recommendations. It also shows a way to make the code more concise and achieve a similar goal. Sorry if this comes off as too preachy/know-it-all, I definitely don't. This question piqued my interest and I had to respond!

window.proceed = function () {
    var email = document.getElementById('email').value;
    if (email == "") {
        alert("Empty variable \n");
    } else if (email.indexOf("@") == -1) {
        alert("No @ \n");
    } else return true; 
}

jsfiddle

Upvotes: 1

Deryck
Deryck

Reputation: 7658

Well I was about to write this myself but I happened to have found it like 4 hours ago. This will help you.

<script>
function proceed()
{
var email = document.getElementById("email").value;
var atpos = email.indexOf("@");
var dotpos = email.lastIndexOf(".");
if (atpos < 1 || dotpos < atpos+2 || dotpos+2 >= email.length)
  {
  alert("Not a valid e-mail address");
  return false;
  }
};
</script>

<form onsubmit="proceed();">
    <input type="text" name="email" id="email" />
    <input type="submit" />
</form>

Your code had a lot of typos, just take your time when writing. This is from w3schools

Upvotes: 1

Tony
Tony

Reputation: 17637

<form onsubmit="return proceed();">
    <input type="text" name="email" id="email">
    <input type="submit" />
</form>

<script>
    function proceed() {
        var email = document.getElementById('email').value;
        var problems = "";

        if (email == "") {
            problems = "Empty variable \n";
        }

        var noat = false;
        if (email.indexOf("@") == -1) noat = true;

        //    for (int i=0; i< email.length; i++){
        //    if (email.charAt(i) == "@"){  //Compare each character
        //       noat=false;
        //       break;
        //}

        if (email == "" || noat == true) {
            problems += "No @ \n";
            alert(problems);
        }

        if (problems == "")
            return true;
        else
            return false;
    }
</script>

Upvotes: 5

Rainer Plumer
Rainer Plumer

Reputation: 3753

Instead of using onclick, i think you should use onsubmit and instead of comparing each character for @ symbol, you should use email test regular expressions.

there are plenty of those online, just google.

sample regular expressions

Upvotes: 2

Related Questions