Menu
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
617
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)
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.
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>
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.
80
You can try a simpler approach instead of using a loop:
window.onload = function() {
var itemsContainer = document.getElementById('cities-drop');
document.addEventListener('keyup',function(event) {
if (event.keyCode == 40 && itemsContainer.style.display=='block') {
event.preventDefault();
var previousHoveredChoice = itemsContainer.querySelector('.hovered');
previousHoveredChoice.className = '';
var currentHoveredChoice = previousHoveredChoice.nextSibling;
if (currentHoveredChoice) {
currentHoveredChoice.className = 'hovered';
}
}
});
//following code is copy-pasted from the live example
//just to close the onload function handler in this solution
document.addEventListener('keyup',function(event){
if (event.keyCode == 27) {
if (document.getElementById('cities-drop').style.display=='block'){
document.getElementById('cities-drop').style.display='none';
}
}
});
//end of copy-pasted code
};
There are a few things I can see that might be a problem. First of all, you update
itemsContainer.children[i].nextSibling which is itemsContainer.children[i+1]. That's why always the last element it selected if you skip the break. itemsComtainer[i+1] will always be hovered if there is on item matching the class.
Second problem is what Travesty3 is pointing out in his answer.
I also changed the if condition to check if the hovered class is on one of the children and not the container itself.
if (itemsContainer.children[i].getAttribute('class').match('hovered'))
I've modified the event handler with the following lines of code, and this seems to work fine:
document.addEventListener('keyup',function(event){
if (event.keyCode === 40 && itemsContainer.style.display==='block') {
event.preventDefault();
for (var i=0,l=itemsContainer.children.length;i<l;++i){
if (itemsContainer.children[i].getAttribute('class').match('hovered')){
itemsContainer.children[i].setAttribute('class','');
itemsContainer.children[i+1].setAttribute('class','hovered');
break;
}
}
}
});
Keep in mind that making such a drop down control is quite a lot of work. The user expects to navigate it using the keyboard. To make a great user experience, you should handle a number of keys, such as the arrow keys, tab to focus the control, space to open and close it, alpha-numeric input to focus on first matched element, etc.
If user experience is important, I'd recommend using a framework and a plugin for this. I personally prefer jquery and jquery ui, and there are a number of plugins for select drop downs. Another advantage is that if javascript is disabled by the client, or your javascript would err for some reason, most plugins fall back to a regular native select element, which would still work fine functionally.
I have used this plugin for a selectbox myself for a simple drop down: http://www.abeautifulsite.net/blog/2011/01/jquery-selectbox-plugin/
Edit: I'm revoking this recommendation, as it doesn't work if multiple elements have the same name. If this is important, you should check out the Filament Group's selectmenu plugin instead: http://filamentgroup.com/lab/jquery_ui_selectmenu_an_aria_accessible_plugin_for_styling_a_html_select/ //Edit
...and the jquery autocomplete plugin for a combobox also supporting written input: http://jqueryui.com/demos/autocomplete/
36
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With
