views:

257

answers:

3

There is the following HTML structure

<div>
   <p class="open">some txt</p>
   <p>some text 2</p>
   <p>some text 3</p>
   <a href="javascript:;" onclick="down(this.parentNode)">Down</a>
</div>

and when I press down I want to move class "open" to next p tag, here is code that do this, but I didn't think that it most elegant solution

function down(el){
    el.getElementsBySelector("p.open").each(

        function(s){ 

            s.removeClassName('open');

            if (s.next('p')){                    
                s.next('p').addClassName('open');
            }   
            else{
                el.getElementsBySelector("p:first").each(
                    function(e){
                        e.addClassName('open');                    
                    }    
                );    
            }

        }
    );       
}

how this code can be improved?

A: 

It seems okay to me, but I have to say your question is very argumentative. There isn't a definitive answer for your question.

w35l3y
A: 

I think you could do with:

function down(el) {
  var currentOpenP = el.down('p.open');
  var nextPSibling = currentOpenP.next('p');
  if (nextPSibling) {
    currentOpenP.removeClassName('open');
    nextPSibling.addClassName('open');
  }
}

This code will get the first P tag under your el which has the className "open", check if a next sibling P tag exists, and if it does, set its className to "open" and remove the className from the previous tag.

Boro
A: 

Try this:

function down(el){
   var selected = el.down('p.open');
   el.select('p.open').invoke('removeClassName', 'open');

   var next = selected.next('p'); // select next p
   if(!next) next = el.down('p'); // if next doesn't exists, use the first one

   next.addClassName('open');
}
Bird