set the Id of an element

[garethhall]

I am working on a little job and I need the set the id on a li tag. At this stage it not working.

 

The first problem is liNodes.length is returning 0?? I should be 4!

 

Here is the JS

var liNodes;
liNodes = document.getElementsByTagName("li");
for(i = 0; i < liNodes.length; i++){	
if(liNodes[i].className == "subCat"){
	liNodes[i].onclick = setSubCatActive;
}
}
function setSubCatActive(){
this.setAttribute("id"."selected")	
}

 

Here is the HTML

<ul>
  <li class="subCat">item1</li>
  <li class="subCat">item2</li>
  <li>item3</li>
  <li>item4</li>
</ul>

[roopurt18]

Is your JavaScript in the head section at the top of the page?  If yes, then the browser is running it right when it encounters it and before the markup has been loaded!  Your JavaScript should really run after the page has finished loading.

 

This might work:

window.onload = function() {
  var liNodes;
  liNodes = document.getElementsByTagName("li");
  for(i = 0; i < liNodes.length; i++){	
  	if(liNodes[i].className == "subCat"){
  		liNodes[i].onclick = setSubCatActive;
  	}
  }
}
function setSubCatActive(){
this.setAttribute("id","selected")	
}

 

While that may work, it's very bad JavaScript.  I highly recommend using a JavaScript library.  I use Dojo quite frequently at work; assuming you were using Dojo, you might accomplish the same thing like so:

 

dojo.addOnLoad( function() {
  // Keep track of dojo.connects()
  window.appDojoConnects = [];

  // Add an event handler to the window onunload event to disconnect all event handlers
  window.appDojoConnects.push( 
    dojo.connect( 
      window, 
      "unload", 
      function( e ) {
        // Disconnect all event handlers
        while( window.appDojoConnects.length > 0 ) {
          dojo.disconnect( window.appDojoConnects.pop() );
        }
      } 
    )
  );

  // Add your li event handler
  dojo.query( 'li.subCat' ).forEach( 
    function( n, idx, all ) {
      window.appDojoConnects.push(
        dojo.connect(
          n,
          "click",
          "setSubCatActive"
        )
      );
    }
  )
} );

/**
* @param Event object
*/
function setSubCatActive( e ) {
  dojo.attr( e.target, "id", "selected" );
}

[garethhall]

My js in the head

[haku]

While that may work, it's very bad JavaScript.  I highly recommend using a JavaScript library.

 

There is nothing wrong with using a javascript library (I am a jquery guy myself, and use it all the time), but by no means is the javascript he has written bad (other than the fact that it doesn't work ). Especially with a case like this, where the code is lightweight, a javascript library is overkill for what is essentially one function.

 

As for the original problem, Roopert mostly got it right in saying it needs to be run onload, and not just directly. But there is one more problem that needs to be fixed:

 

window.onload = function() {
  var liNodes;
  liNodes = document.getElementsByTagName("li");
  for(i = 0; i < liNodes.length; i++){   
     if(liNodes[i].className == "subCat"){
        liNodes[i].onclick = setSubCatActive(this);
     }
  }
}
function setSubCatActive(target){
   target.setAttribute("id","selected")   
}

[roopurt18]

        liNodes.onclick = setSubCatActive(this);

I believe that line is in error because in that context this refers to the onload function.

 

I didn't mean my comment on "bad JavaScript" to come off how it did.  What I meant when I said, "bad JavaScript" was using code like:

window.onload = function() { ... }

 

That is very bad practice and there's no way around it.  Similarly, liNodes.onclick = setSubCatActive() is also bad JavaScript practice.

 

What's bad about it?

 

Really what's bad is setting event handlers in that manner.  Consider what happens if you write a site and set all of your JavaScript event handlers that way.  You later go and install a third party script that uses the same convention.  What will happen depends on which order the JavaScript runs in, but for the sake of argument let's say your JavaScript runs first and the third party runs second.

 

What happens is your event handlers all get set.  Then the third party code runs and replaces your event handlers with its own.  This is why we have the more evolved (and complicated) event model in current browsers: Multiple scripts need to listen to the same events on the same objects and you can not accomplish that if you set events in this manner.

 

My recommendation:  Write "raw" JavaScript until you're comfortable with the concepts and ideas, but then learn and use a library because it's loads easier.

[KevinM1]

Eh, window.onload isn't that bad.  Yes, if you're going to be using a lot of 3rd party code, then using a framework is the way to go (I'm also a jQuery man), but for small-medium projects that don't rely on outside resources, window.onload is fine.  And, if written properly, it's not hard to port it to a more flexible, framework-supported version down the line anyway, as the structure would essentially be the same (replace window.onload with $(document).ready, all getElementByX functions with CSS selectors, etc).

[haku]

You can run into potential conflicts with ANY 3rd party script, whether you are using a library or not. The library will use an alias of sorts to run their onload script, but in the end, if you are setting event handlers, and another script is also setting conflicting event handlers for the same element, there will be a conflict regardless of whether you are using a library or not. All the library does is change how the event handler is set - it won't prevent a conflict.

[corbin]

"        liNodes.onclick = setSubCatActive(this);

I believe that line is in error because in that context this refers to the onload function."

 

 

 

Technically the error is not only because of the misplaced this, but because of the syntax.

 

 

Function definitions in JS are are essentially like in mathematics:

 

var somevar = someFunction;

 

var someval = someFunction();

 

somevar holds a pointer to the function someFunction, and someval holds the value the function someFunction() returns.

 

 

Sort of like:

 

f(x) = 2x;

g = f;

h = f(3);

 

Then:

 

g(x) = f(x)

 

h = 6;

[garethhall]

You can run into potential conflicts with ANY 3rd party script, whether you are using a library or not. The library will use an alias of sorts to run their onload script, but in the end, if you are setting event handlers, and another script is also setting conflicting event handlers for the same element, there will be a conflict regardless of whether you are using a library or not. All the library does is change how the event handler is set - it won't prevent a conflict.

 

As it turns out I am using jquery in this project can you recommend a plug in? I found this one but I am not sure on how to implement it?

[haku]

You don't even need a plugin. The jquery for this is really easy. Show us the HTML for the li tags

 

[RichardRotterdam]

Just wanted to add a little thing.

 

The following code could result into invalid HTML  since you are able to set the id "selected" twice.

window.onload = function() {
  var liNodes;
  liNodes = document.getElementsByTagName("li");
  for(i = 0; i < liNodes.length; i++){   
     if(liNodes[i].className == "subCat"){
        liNodes[i].onclick = setSubCatActive(this);
     }
  }
}
function setSubCatActive(target){
   target.setAttribute("id","selected")   
}

 

In HTML this could result into the following:

<ul >
  <li class="subCat" id="selected">item1</li>
  <li class="subCat" id="selected">item2</li>
  <li>item3</li>
  <li>item4</li>
</ul>

This is not something you would want since the ids should always be unique.

 

Instead why not add a second class? You can add multiple classes to an element. that would look like

<li class="subCat selected">item1</li><!-- two classes here-->

 

This is how a jQuery translation would look like(read the comments for explanation):

 

<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.3/jquery.min.js"></script>
<script type="text/javascript">
// wait till the dom is loaded
$(document).ready(function(){

// fetch all list elements with class subCat
var liElements = $('#jquery_list li.subCat');

// set click event for all li elements with class subCat
liElements.click(function(){
	// add extra class "selected" to the li element 
	$(this).addClass( "selected");
});

});
</script>
<!-- 
I added the id jquery_list here so possible other li elements will be uneffected. It's just a little prevention of posible conflicting code.
-->
<ul  id="jquery_list">
  <li class="subCat">item1</li>
  <li class="subCat">item2</li>
  <li>item3</li>
  <li>item4</li>
</ul>

[trq]

You really are doing to much work if your already using jQuery here. I'm not exactly sure what your after but a simple example of setting an id to all li elements within a ul list with jQuery would be something like...

 

$(document).ready(function() {
  $('ul').each(function(i)) {
    $(this).attr({'id': 'foo'+i});
  });
});

 

this would give all the li's there own id. foo1, foo2, foo3 etc etc etc.

[haku]

I think that will give all the <ul> tags an ID.

 

$("ul li").each(function(i)) {

 

should get all the <li>s.