Search code examples
javascripteventsloopslistenerarrow-keys

JS ArrowDown addEventListener for several elements in loop (demo)

Have following listener for keyboard ArrowDown event(it's key code is 40):

window.onload = function() {    
var itemsContainer = document.getElementById('cities-drop');
document.addEventListener('keyup',function(event){
if (event.keyCode == 40 && itemsContainer.style.display=='block') {
event.preventDefault();
    for (var i=0;i<itemsContainer.children.length-1;i++){
        if (itemsContainer.getAttribute('class').substr('hovered')!=-1){
            itemsContainer.children[i].setAttribute('class','');
            itemsContainer.children[i].nextSibling.setAttribute('class','hovered');
                //break;
                }
            }
        }
    });

in this case hovering jumps to last element in list, after ArrowDown pressed.

In case break is uncommented,it jumps to the second element and doesn't jumping any more.

Can't get the principle,how to do, that listener always listens...

EDIT live demo
perhaps,it's a matter of closure,but i'm not certain

Solution

  • After looking at your code and realizing what you're trying to do, I think your error is using substr where you should be using indexOf. Here is the updated line:

    if (itemsContainer.getAttribute('class').indexOf('hovered') != -1)


    More detail: What you were actually doing was using a substr with a string value for the start index. Not sure what the result of that would be, but apparently it's not -1, since the conditional was returning true every time, causing the next element to be hovered EVERY time, all the way down to the bottom of the list. With the break statement in there, it was executing the if-statement at the first element (causing the second element to be 'hovered'), and then quitting.

    I would leave the break statement in there after correcting your code, so that the loop stops after it finds its match, and doesn't loop through the rest of the items unnecessarily.

    EDIT:

    I found a couple other issues in your code as well. Here is an example that works for me in IE and FF, at least (haven't tested in Safari, Opera or Chrome):

    <html>
<head>
    <style type="text/css">
        .hovered
        {
            color:red;
        }
    </style>
    <script type="text/JavaScript">
        function move(event)
        {
            var itemsContainer = document.getElementById('cities-drop');
            if (event.keyCode == 40 && itemsContainer.style.display == 'block')
            {
                if (event.preventDefault)
                    event.preventDefault();
                if (event.cancelBubble)
                    event.cancelBubble();
                if (event.stopImmediatePropagation)
                    event.stopImmediatePropagation();

                for (var i=0; i<itemsContainer.children.length-1; i++)
                {
                    if (itemsContainer.children[i].className.indexOf('hovered') != -1)
                    {
                        itemsContainer.children[i].className = "";
                        itemsContainer.children[i+1].className = "hovered";
                        break;
                    }
                }
            }
        };
    </script>
</head>
<body onkeydown="move(event)">
    <div id="cities-drop" style="display:block;">
        <p class="hovered">Item 1</p>
        <p>Item 2</p>
        <p>Item 3</p>
        <p>Item 4</p>
        <p>Item 5</p>
        <p>Item 6</p>
        <p>Item 7</p>
    </div>
</body>
</html>

    Detail: For me, in IE9, the function was never being called. Instead, I just made it a regular function and added an onkeydown event to the body tag.

    Next, for cross-browser compatibility, you should check to make sure that event.preventDefault exists before using it. I was getting a JS error in IE.

    In your if-statement, you had itemsContainer.getAttribute('class'). Firstly, you needed to use itemsContainer.children[i]. Secondly, .getAttribute('class') didn't work for me in IE, so I switched it to just .className.

    Lastly, itemsContainer.children[i].nextSibling didn't work for me, but it's simple enough to just change it to itemsContainer.children[i+1] to get the next sibling.