4

Been having a bit of a problem for the last couple of days. I'm trying to streamline my code as much as possible and I have now got to the stage where I am trying to add Event Listeners via JavaScript so my HTML looks tidier.

-HTML Segment-

<input type="button" id="googleSearchButton" />
<input type="button" id="youtubeSearchButton" />
<input type="button" id="wikiSearchButton" />
<input type="button" id="facebookSearchButton" />
<input type="button" id="twitterSearchButton" />
<input type="button" id="tumblrSearchButton" />
<input type="button" id="dropboxSearchButton" />

JavaScript Segment

var contIDArray = ["google", "youtube", "wiki", "facebook", "twitter", "tumblr", "dropbox"];

window.load = initAll();
function initAll(){
    applyProperties();
}

function applyProperties(){
        for (var i = 0; i < contIDArray.length; i++){
            addEventListeners(contIDArray[i] + "SearchButton");
        }
}

function addEventListeners(id){
    document.getElementById(id).addEventListener("click", testAlert(id), false);
}

function testAlert(id){
    alert(id + " clicked")
}

The Theory

As, I hope, you can see, the FOR loop will loop until it runs out of values in the container Array. Each time it will output the place in the Array followed by "SearchButton". For example, the first time it loops it will output "googleSearchButton", the second time "youtubeSearchButton" and so forth.

Now, I know that the FOR loop works for applying properties because I use it to apply Button values and text box placeholder text in other segments of my project.

I have made it add a simple test function ("testAlert()") and set it to pass the id of the element that called it. I have set it up so once the event listeners have been added I can simply click on each button and it will alert its id and tell me that it has been clicked.

The Problem

Now, theoretically, I thought this would work. But it seems that the FOR loops fires the "addEventListeners" function, which, in turn, adds the event listener to fire "testAlert" on click. But it just fires the "testAlert" function as soon as it adds the event listener and does not fire when you click.

I apologise if this seems a bit much to take in, I always overdo the length of my explanation. Hopefully you'll be able to see what I'm trying to accomplish from my code, rather than my explanation.

Help would be much appreciated. :)

Improve this question

4 Answers 4

Reset to default
6

You're close here, but there are a few things wrong.

First, you can't just do id.addEventListener. You need to do document.getElementById(id).addEventListener. id is just a string, you need a DOMElement.

Second, when you do testAlert(id), you're running the function, then assigning its return value (undefined) as the event listener. You need to pass a function. Like so:

id.addEventListener("click", function(){
  testAlert(this.id); // this is the DOMElement you clicked on
}, false);

Though I suggest adding a class to all your buttons, and then adding the event like that.

<input type="button" id="googleSearchButton" class="searchButton" />
<input type="button" id="youtubeSearchButton" class="searchButton" />
<input type="button" id="wikiSearchButton" class="searchButton" />
<input type="button" id="facebookSearchButton" class="searchButton" />
<input type="button" id="twitterSearchButton" class="searchButton" />
<input type="button" id="tumblrSearchButton" class="searchButton" />
<input type="button" id="dropboxSearchButton" class="searchButton" />

And then:

var buttons = document.getElementsByClassName('searchButton');
for(b in buttons){
   if(buttons.hasOwnProperty(b)){
     buttons[b].addEventListener("click", function(){
        testAlert(this.id); // this is the DOMElement you clicked on
     }, false);
   }
}

NOTE: addEventListener and getElementsByClassName may not be available in all browsers (by that I mean they might not work in IE). This is why a lot of websites use a JavaScript library, like jQuery. jQuery handles all the cross-browser stuff for you. If you want to use jQuery, you could do this:

$('.searchButton').click(function(){
  testAlert(this.id);
});

NOTE 2: In JavaScript, functions are variables, and can be passed as parameters.

document.getElementById(id).addEventListener('click', testAlert, false);

Notice how there are no () after testAlert, we are passing the function itself, when you do testAlert() you're passing its return value. If you do it this way, testAlert will need to be modified a bit:

function testAlert(){
    alert(this.id + " clicked")
}
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

StackOverflow never fails to amaze me how quickly and effectively it's users can help me. Thanks a lot, man! I don't particularly understand the need for nesting a function call inside a blank function, but if it works, it works. :) Also, I don't really understand what's going on with the whole class thing and it works now, so I'll stick with what I know. Thank you for the added effort, though! :)
@BenHooper: You're welcome! I love helping people out. Anyway, you need to wrap testAlert in a function because addEventListener expects you to pass it a function. When you just do testAlert(id), it executes that immediately, and then assigns testAlert's return value (which is undefined because it didn't return anything) as the event (which should be a function). The class thing was just a suggestion. Instead of making an array of IDs, I suggest using DOM methods to get a list of all the elements you want, and then looping and adding events.
Ah, I see. I understand both of your suggestions now. Should I use the method of function(){functionName()} every time in the future for use with adding event listeners? And yeah, I guess that would work. But I am actually using the Array and FOR loop with multiple other things, which the class system wouldn't work with. But yes, this has provided interesting prospects for future code. So, once again, I thank you. :D
@BenHooper: The function(){functionName()} works fine, but there is a shorter way. addEventListener("click", testAlert, false); Notice how there aren't () after testAlert. Functions in JavaScript are variables, and can be passed to functions. This will set the function testAlert as the event, testAlert() set its return value. If you do it that day, testAlert would need to be changed to function testAlert(){alert(this.id + " clicked");}.
2

Change:


function addEventListeners(id){
    id.addEventListener("click", testAlert(id), false);
}

for:


function addEventListeners(id){
    document.getElementById(id).addEventListener("click", testAlert(id), false);
}

Otherwise you're applying addEventListener on a string. In any case, replace addEventListener with an assignment to the event, like onClick.

Improve this answer

4 Comments

I'm sure I did try this, as I've had similar problems previously. But I will try your advice now. One moment.
Where are you calling this script? On the HEAD tag or before /BODY?
I'm aware that this is probably considered bad practice/convention, but I call it after the body tag closes, so that the script is called after all HTML elements have been created, as I've had many, many problems doing it the conventional way before.
When you do testAlert(id) you're calling it and setting the event to the return value (undefined). It should be function(){testAlert(this.id);}.
1

id looks like a string to me. So instead do something like this:

function addEventListeners(id){
  var obj = document.getElementById(id);
  obj.addEventListener("click", testAlert(id), false);
}

Also, here is the working code:

http://jsfiddle.net/ZRZY9/2/

obj.addEventListener("click", function() { testAlert(id); }, true);

As Rocket mentions above "you're calling it and setting the event to the return value undefined".

The bad news is addEventListener() is currently not supported in Internet Explorer 7.

Improve this answer

1 Comment

I tried this around half an hour ago, and it did not work. :/ I also just tried some advice given prior to you, which would give the same effect and that did not work either.
0

I ran through your code. The initial problem that I came across was that you were trying to find the elements in the document before they were created. window.onLoad fires before the page is complete. I tested this using the body tag's onload attribute and it works that way.

So, it's a combination of the aforementioned issue of your trying to find the element by using the "id" string and the function firing before the page was completely loaded.

Anyway, glad you got it working!

This is the javascript I had at the end:

    <script>
    var contIDArray = ["google", "youtube", "wiki", "facebook", "twitter", "tumblr", "dropbox"];

function initAll(){
    applyProperties();
}

function applyProperties(){
        for (var i = 0; i < contIDArray.length; i++){
    var newString = contIDArray[i] + "SearchButton"
            addEventListeners(newString);
        }
}

function addEventListeners(id){
    document.getElementById(id).addEventListener("click", testAlert, false);
}

function testAlert(){
    alert(this.id + " clicked")
}
</script>
Improve this answer

Comments

Your Answer

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