JS Prototype - Moving items between two containers - what's wrong here?

Question

I am trying to move items between two lists. Items when clicked should go to another list. JS code prototype:

document.observe('dom:loaded', function() { $$('li.available-item').each(function(element){ element.observe('click', function(){ element.removeClassName('available-item'); element.addClassName('selected-item'); $('selected-list').insert(element); }); }); $$('li.selected-item').each(function(element){ element.observe('click', function(){ element.removeClassName('selected-item'); element.addClassName('available-item'); $('available-list').insert(element); }); }); }); 

Sample Elements:

 <ul id="available-list"> <li class="available-item">Apple</li> <li class="available-item">Orange</li> <li class="available-item">Grapes</li> </ul> <ul id="selected-list"> <li class="selected-item">Pineapple</li> <li class="selected-item">Papaya</li> </ul> 

While it is working for the first time, I click on the items, as soon as the item moves to another list, they do not move to another (original) list when clicked.

What am I doing wrong here?

Answer 1

The reason is that when you move an element, it still gets the attached old event handler, which will just move it again.

This is the classic place to use event delegation. Instead of tracking clicks on items, find the clicks on the lists (because the clicks are bubbling), and then move the corresponding item. Something like that:

 $('available-list').observe('click', function(event) { var li; li = event.findElement('li'); if (li) { event.stop(); li.addClassName('selected-item').removeClassName('available-item'); $('selected-list').insert(li); } }); 

... and the opposite for selected-list .

You can use only one handler for both lists:

 $('available-list').observe('click', listClick); $('selected-list').observe('click', listClick); function listClick(event) { var fromType, toType, li; // Get the clicked list item li = event.findElement('li'); if (li) { // We're handling it event.stop(); // Determine whether moving to the selected or available list if (this.id.startsWith("selected")) { fromType = "selected"; totype = "available"; } else { fromType = "available"; totype = "selected"; } // Update class names li.addClassName(toType + '-item').removeClassName(fromType + '-item'); $(toType + '-list').insert(li); } }); 

It becomes even easier if you throw classes on the elements (see below):

 $('available-list').observe('click', listClick); $('selected-list').observe('click', listClick); function listClick(event) { var targetList, li; // Get the clicked list item li = event.findElement('li'); if (li) { event.stop(); targetList = this.id.startsWith("selected") ? "available-list" : "selected-list"; $(targetList).insert(li); } }); 

---

Partly OT, but you may not need these selected-item and available-item classes. You don’t need them to find them anymore, and in your CSS you can use the descendant selector to style the elements:

 #selected-list li { /* ...styles for the `li` elements in the `selected-list` ... */ } #available-list li { /* ...styles for the `li` elements in the `available-list` ... */ } 

If you want to affect only li , which are direct children of lists, use the child selector instead of the child selectors (note > ):

 #selected-list > li { /* ...styles for the `li` elements in the `selected-list` ... */ } #available-list > li { /* ...styles for the `li` elements in the `available-list` ... */ }