Sort a list item up or down

Adam picture Adam · Feb 9, 2011 · Viewed 7k times · Source

A very simple example of what i am trying to do below. Link/button on left to move item up, link/button on right to move the item down. But it is not working, and i am getting an error:

Object doesnt support this property or method

at this line:

items[counter-1] = curr;// move
previous item, to current

Example Image:

Here is my code:

Answer

user113716 picture user113716 · Feb 9, 2011

You're using a for-in statement, which means that you don't have any guarantee of the numeric order you may expect.

Use a for statement instead:

for (var i = 0, len = items.length; i < len; i++) {

Also, keep in mind that items is a "live list", so changes you make in the DOM are reflected in the list, and the list itself is immutable since it isn't an Array.

If you want to shift an element back one index, use insertBefore.

Something like this:

items[i].parentNode.insertBefore( items[i],items[i-1] );

Example: http://jsfiddle.net/d25a3/

function MoveItem(id, direction) {
    var ul = document.getElementById('GroupBy');
    var items = ul.getElementsByTagName('li');

    var counter = 0;
    var previousItem = null;
    var moveNextItemUp = false;

    for (var i = 0, len = items.length; i < len; i++) {
        var item = items[i];

        if (item.id == id) {
            if (direction == 1) { 
                moveNextItemUp = true;
            } else if ((direction == -1) || (moveNextItemUp == true)) {

                item.parentNode.insertBefore( item,items[i-1] );
                break;
            }
        }
        previousItem = item;
        counter = counter + 1;
    }
}

Also, not sure what the full intent of the code is, but it would seem that you could simplify something like this:

Example: http://jsfiddle.net/d25a3/1/

   <!-- pass the parent node of the item clicked as the first argument -->

<li id="Two">
    <a href="#" onclick="MoveItem(this.parentNode, -1)"> ^ </a>two<a href="#" onclick="MoveItem('Two', 1)"> V </a>
</li>

And get rid of the loop entirely:

function MoveItem(item, direction) {

    var counter = 0;
    var previousItem = null;
    var moveNextItemUp = false;

    if (direction == 1) {
        moveNextItemUp = true;
    } else if ((direction == -1) || (moveNextItemUp == true)) {

           // get the previous <li> element
        var prev = item.previousSibling
        while( prev && prev.nodeType != 1 && (prev = prev.previousSibling));

        item.parentNode.insertBefore(item, prev);
    }
    previousItem = item;
    counter = counter + 1;
}