CODEWITHSUNDEEP
javascript
user976813
user976813

Reputation: 77

what's wrong with my simple script?

I am trying to write a very simple script and can't figure out the issue there. the function of the script should be displaying the inner html of each list element but it keeps showing only the last one which is 'Orange'

here is what i have in my html:

<ul id='mylist'>
  <li>Red</li>
  <li>Green</li>
  <li>Black</li>
  <li>Orange</li>
</ul>

and below is that script:

var x = document.getElementById('mylist');
var z = x.getElementsByTagName('li');

for (i = 0; i < z.length; i++) {    
    var res = z[i].innerHTML;
    z[i].setAttribute('onclick','alert(res)');
}

I probably need to add a closure here but i am not sure if i really need to and how to add it

Upvotes: 0

Views: 97

Answers (5)

3ocene
3ocene

Reputation: 2220

First off, make sure you have a closing </ul> tag:

<ul id="test-list">
  <li>One</li>
  <li>Two</li>
  <li>Three</li>
</ul>

Second, use either the onclick property of the DOM object or addEventListener():

var x = document.getElementById('mylist');
var z = x.getElementsByTagName('li');

for(i=0;i<z.length;i++){
  z[i].onclick = function() {
    alert(this.innerHTML); // In this scope, "this"
  };                      // means the element that was clicked
}

or

var x = document.getElementById('mylist');
var z = x.getElementsByTagName('li');

for(i=0;i<z.length;i++){
  z[i].addEventListener('click', function() {
    alert(this.innerHTML); // In this scope, "this"
  });                      // means the element that was clicked
}

If you must use setAttribute(), you need to be very careful of scoping. The last value of res was orange, and that's what the browser remembers. When you click the element, the browser checks the current value of res and alerts it. If you use 

setAttribute('onclick', 'alert"' + res '")');

It will take the value of res at the current iteration of the for loop and inject it into the string.

Upvotes: 0

epignosisx
epignosisx

Reputation: 6192

Try this:

var x = document.getElementById('mylist');
var z = x.getElementsByTagName('li');

for (var i = 0; i < z.length; i++) {
    (function(i) {
        var res = z[i].innerHTML;
        z[i].onclick = function() {
            alert(res)
        };
    })(i);
}​

http://jsfiddle.net/sssonline2/UYEa9/

Upvotes: 2

meouw
meouw

Reputation: 42140

A simple solution

HTML

<ul id="test-list">
    <li>One</li>
    <li>Two</li>
    <li>Three</li>
</ul>

JavaScript

var x = document.getElementById( 'test-list' );
var li = x.getElementsByTagName( 'li' );

for( var i = 0; i < li.length; i++ ) {
    li[i].onclick = function() {
        alert( this.innerHTML );
    }
}

Upvotes: 0

Adil
Adil

Reputation: 148160

Try this by making res as a identifier instead of string string constant Demo on JsFiddle

var x = document.getElementById('mylist');
var z = x.getElementsByTagName('li');

for(i=0;i<z.length;i++){
      var res = z[i].innerHTML;  
      z[i].setAttribute('onclick',"alert('"+res+"');");

}​

Upvotes: 1

A123321
A123321

Reputation: 837

This could be done in another way

var lis = document.getElementsByTagName('li');
for (var i = 0; i < lis.length; i++) {  
        lis[i].addEventListener('click', 
            function() {
                console.log('asd');
                alert(this.innerHTML);
            }
            , false);
}

Try it on JSFiddle

Using addEventListener is the current recommended way to do this according to MDN.

Upvotes: 0

Related Questions