Jump to content

Best way to add onclick events to elements in a for loop


Day
 Share

Recommended Posts

I'm adding onclick events to an array of elements and each one should get the .active class after being clicked. I've got 2 ways to make it work, but can someone help me to understand the difference between the 2? And also let me know if there is a better option?

for (i=0; i<_fsMenuItems.length; i++) {	// Option 1	(function(index) {		_fsMenuItems[index].onclick = function() {			_fsMenuItems[index].className += ' active';		}	})(i);	// Option 2	_fsMenuItems[i].onclick = (function(index) {				return  function(){			_fsMenuItems[index].className += ' active';		};				})(i);}

Thank you!

Link to comment
Share on other sites

The closures aren't necessary. There's the this keyword and also the target property of the event object to refer to the element. It would also save memory to have all of the elements refer to the same function.

 

Have you thought about the need to remove the active class from any elements?.

 

This is slightly better:

for (i=0; i<_fsMenuItems.length; i++) {        _fsMenuItems[i].onclick = makeActive;}function makeActive() {    this.className += ' active';}
Link to comment
Share on other sites

Awesome! Thank you. I didn't realize I could use 'this' here. I already had some other functions, activate() and deactivate() for another part that I am reusing.

 

This is what I ended up with:

for (i=0; i<_fsMenuItems.length; i++) {	_fsMenuItems[i].onclick = activateMenuItem;}function activateMenuItem() {	for (i=0; i<_fsMenuItems.length; i++) {		deactivate(_fsMenuItems[i]);	}	activate(this);}
Edited by Day
Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...