1
\$\begingroup\$

I have the following js script for my logon page. It feels like a long run on sentence. It works but wondering if there is a better way to structure this.

window.onload = function(){    
//define form elements
const remBox = document.querySelector('input[type="checkbox"]');
const alerts = document.getElementsByClassName('alerts');
const form   = document.querySelector('form');
const formEmail  = document.querySelector('input[type="email"]');
const formPassword = document.querySelector('input[type="password"]');  

//define local storage for remember me
const localStorageEmail = localStorage.getItem("lsEmail");
const localStoragePassword = localStorage.getItem("lsPassword");

if(localStorageEmail === null)formEmail.value = '';
else formEmail.value = localStorageEmail;  

// create listeners
document.querySelector('input[type="submit"]').addEventListener('click',submit);
document.querySelector('input[type="email"]').addEventListener("focus", focus);
document.querySelector('input[type="password"]').addEventListener("focus", focus);
//window.addEventListener("pageshow", clearBack);

//localStorage has valid email - fill form with email and dummy password
if(localStorageEmail !== null){
    formEmail.value  = localStorageEmail;
    formPassword.value = localStoragePassword;
    //check remember me box
    document.querySelector('input[type="checkbox"]').checked=true;
}

function submit(){
    event.preventDefault();

    const emailPass = {};
    let responseData = {};

    formEmail.value = formEmail.value.toLowerCase();
    const eflag = emailCheck(formEmail.value);
    const pflag = passwordCheck(formPassword.value);

    if(eflag && pflag){
        emailPass.formEmail = formEmail.value;
        emailPass.formPassword = formPassword.value;

        //responseData = sendData2('http://d78d-96-230-240-153.ngrok.io/lagin/public/login.php',emailPass);

        responseData = sendData2('login.php',emailPass);
        responseData.then(function(response){

            if(response.email == 'FOUND' && response.password == 'MATCH'){

                // Remember me box
                if(remBox.checked === false){
                    localStorage.removeItem('lsEmail');
                }else{
                    localStorage.setItem("lsEmail", formEmail.value);
                    localStorage.setItem("lsPassword", formPassword.value);
                }
                form.submit();
                
            }else if(response.email == 'NOT FOUND'){
                alerts[0].innerHTML = "Email address not found.  Register or enter a valid email address";
                alerts[0].classList.remove("hide");
                alerts[0].classList.add("show");
                formEmail.value = '';

            }else if(response.password  == 'NO MATCH'){
                alerts[1].innerHTML = 'Password does not match.  Please re-enter';
                alerts[1].classList.remove("hide");
                alerts[1].classList.add("show");
                formPassword.value = '';

            }else if(response.email == 'DATABASE CORRUPT'){
                document.querySelector('body').innerHTML = '<div><h1>FATAL ERROR: Database corrupt</h1></div>';
            }else if(response.email == 'NOT GET OR POST'){
                document.querySelector('body').innerHTML = '<div><h1>FATAL ERROR: Network error. Invalid request method</h1></div>';
            }else{
                document.querySelector('body').innerHTML = '<div><h1>FATAL ERROR: Unknown error login.js</h1></div>';
            }
        });
    }else{
        console.log('client side eflag = ' + eflag + ' pflag = ' + pflag);
    }
}

function emailCheck(email){
    // function checks for a valid email address  [email protected]
    const parts = [String.raw`^(?:[a-z0-9!#$%&'*+/=?^_\`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_\`{|}~-]+)*|`,
    String.raw`"(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21\x23-\x5b\x5d-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])*")`,
    String.raw`@(?:(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?|`,
    String.raw`\[(?:(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9]))\.){3}(?:(2(5[0-5]|[0-4][0-9])|1[0-9][0-9]|[1-9]?[0-9])|`,
    String.raw`[a-z0-9-]*[a-z0-9]:(?:[\x01-\x08\x0b\x0c\x0e-\x1f\x21-\x5a\x53-\x7f]|\\[\x01-\x09\x0b\x0c\x0e-\x7f])+)\])$`];

    const mailformat = parts.join('');
    if(!email.match(mailformat)){
        alerts[0].innerHTML = "Please enter a valid email address";
        alerts[0].classList.remove("hide");
        alerts[0].classList.add("show");
        return false;
    }else{
        return true;
    }
}

function passwordCheck(password){
    if(password.length < 1){
        alerts[1].innerHTML = '1 or more characters required';
        alerts[1].classList.remove("hide");
        alerts[1].classList.add("show");
        return false;
    }
    return true;
}

function focus(event){
    if(event.target.type == 'email'){
        alerts[0].classList.remove("show");
        alerts[0].classList.add("hide");
        formEmail.value = '';
    }else{
        alerts[1].classList.remove("show");
        alerts[1].classList.add("hide");
        formPassword.value='';
    }
}

function clearBack(){
    if(remBox.checked === false)formEmail.value = '';
}


/* using axios for async request
function getAjax(url, success) {
    var xhr = window.XMLHttpRequest ? new XMLHttpRequest() : new ActiveXObject('Microsoft.XMLHTTP');
    xhr.open('GET', url);
    xhr.onreadystatechange = function() {
        if (xhr.readyState>3 && xhr.status==200) success(xhr.responseText);
    };
    xhr.setRequestHeader('X-Requested-With', 'XMLHttpRequest');
    xhr.send();
    return xhr;
} */


};
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

There looks to be a lot of

alerts[0].innerHTML = "Please enter a valid email address";
alerts[0].classList.remove("hide");
alerts[0].classList.add("show");

or similar variations. You could create a function, taking in the innerHTML and array index to reduce this vastly.

You can create a second function just taking in the array index when you only need to toggle the class.

Ontop of this you may want to look at having the hide class properties already applied to your element and then you only have the show class to add and remove.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Thanks. These are all good suggestions. I revise and ask again \$\endgroup\$
    – DCR
    Commented Aug 22, 2022 at 14:55
  • 1
    \$\begingroup\$ Why would someone downvote this question? Isn’t this approach for code review? \$\endgroup\$
    – DCR
    Commented Aug 22, 2022 at 14:59
  • \$\begingroup\$ Nice to see you actively taking the suggestions in, not sure on the downvote I added an upvote for it tho \$\endgroup\$
    – Nox-iv
    Commented Aug 22, 2022 at 17:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.