javascript oo programming syntax help for a noob

Question

I'm making my first stab at writing some oo style js. When my page loads, it's executing my login() function twice and the event listener that I want to fire the login() isn't working at all. Any help would be greatly appreciated. Here is my code:

(function( window, undefined ) {
var document = window.document,
    navigator = window.navigator,
    location = window.location;
//build the login button
var DA = {
    width : '70px',
    height : '20px',

    createLoginButton : function(){
        var a = document.createElement('a');
        a.style.width = this.width;
        a.style.height = this.height;
        a.style.cursor = 'pointer';
        var div = document.createElement('div');
        div.id = 'da_login_button';
        div.onclick = DA.login();
        div.innerHTML = "client login";
        div.style.width = this.width;
        div.style.height = this.height;
        div.style.backgroundColor = '#59724A';
        div.style.color = 'white';
        div.style.border = '4px solid #6B5D49';
        div.style.padding = '3px';
        div.style.textAlign = 'center';
        a.appendChild(div);
        return a;
    },

    //write the button to the dom
    writeLoginButton : function(){
        var dest = document.getElementById('digital_arborist_login');
        dest.style.width = this.width;
        dest.style.height = this.height;
        var theButton = DA.createLoginButton();
        dest.appendChild( theButton );
        theButton.addEventListener("click", DA.login(), false);
    },

    //detect ssl
    isSecure : function(){
        if (location.protocol === 'https:') {
            return true;
        } else {
            return false;
        }
    },

    // the login function
    login : function(){ 
        alert('href: '+location.href+'<br />protocol: '+location.protocol);
    },

};  
window.DA = DA; 
})( window )

// start the magic
DA.writeLoginButton();

Here is the web page where I'm testing my code.

When i check that page i'm getting an alert, what should happen? — Nicola Peluchetti
– Nicola Peluchetti, Commented Apr 18, 2012 at 15:25

gen_Eric · Accepted Answer · 2012-04-18 15:25:44Z

2

theButton.addEventListener("click", DA.login(), false);

In this line change DA.login() to just DA.login. When you do DA.login(), you're passing the result of the login() function (which is undefined) to the click handler, instead of the function itself.

It should be:

theButton.addEventListener("click", DA.login, false);

EDIT: div.onclick = DA.login(); should be div.onclick = DA.login;, also.

answered Apr 18, 2012 at 15:25

gen_Eric

228k42 gold badges304 silver badges343 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Prestaul · Accepted Answer · 2012-04-18 15:26:53Z

1

Change this:

theButton.addEventListener("click", DA.login(), false);

to:

theButton.addEventListener("click", DA.login, false);

and this:

div.onclick = DA.login();

to:

div.onclick = DA.login;

You want to assign the function itself as the callback in each case. You are actually executing the function and assigning the result of the function call as the callback... That is why your "login" method is running twice at startup and why your click events are not functioning properly.

You should also remove the undefined argument from the first line. undefined is a keyword in javascript and I'm honestly surprised that you didn't get a syntax error from that. It should look like:

(function(window) {

answered Apr 18, 2012 at 15:26

Prestaul

85.4k10 gold badges89 silver badges85 bronze badges

9 Comments

gen_Eric Over a year ago

undefined may be a keyword, but most browsers let you assign a value to it. Adding undefined to the parameter list makes sure it's really set to undefined. Demo: jsfiddle.net/9Lv7Y

ShadeTreeDeveloper Over a year ago

This did the trick! Thanks! Also, I removed the div.onclick = because I don't need two click handlers for the same button.

ShadeTreeDeveloper Over a year ago

I used the undefined keyword simply as a process of copying jQuery. I didn't know why I was doing it, now I do. Thanks for the comment.

gen_Eric Over a year ago

@Keith: Yup, that's done to make sure global variables are what they should be.

gen_Eric Over a year ago

@Keith: That's called a "Self-Executing Anonymous Function". Basically, you make a function, and then immediately call it. This makes a new scope, so that stuff isn't making variables in the global scope. I Googled a bit, and found this: markdalgleish.com/2011/03/self-executing-anonymous-functions

|

Collectives™ on Stack Overflow

javascript oo programming syntax help for a noob

2 Answers 2

Comments

9 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

9 Comments

Your Answer

Sign up or log in

Post as a guest

Related