keypress help please

[cloudll]

Hi guys, I have a couple of questions about my javascript code. When a user presses w,a,s,d it loads a certain page. My first question is, is the code okay? I've seen a few different ways to do it, but this is the only one i could get working, so I just wanted to check if it is good code.

 <script>
    document.onkeydown = function(e) {
    e = e || window.event;
    key = e.keyCode || e.charCode;
    var keys = { 
      87: '?move=up', 
      68: '?move=right', 
      83: ?move=down', 
      65: '?move=left'
    };
    if (keys[key]) window.location.href = keys[key];
    };
    </script>

And my second question was, is there any way to make it fire only once if the user holds down a key?

 

Thanks guys.

[requinix]

You shouldn't assign to an on* event handler directly as it only allows you to use one handler per element per event (using that method).

 

Use event registration instead. There are browser differences so it's easier to do with a Javascript framework, but if you want to do it manually then you use element.addEventListener or element.attachEvent.

// a quick and simple helper function
function registerEventListener(element, event, handler) {
	// IE9+ and other browsers
	if (element.addEventListener) {
		element.addEventListener(event, handler);
	}
	// IE<9
	else if (element.attachEvent) {
		element.attachEvent("on" + event, function() {
			// in here, this != element
			handler.call(element, window.event);
		});
	}
}

registerEventListener(document, "keydown", function(e) {
	// e is already normalized to the event object
	var key = e.keyCode || e.charCode;
	// ...
});

Note that key has the var keyword too. Without it you create/assign properties on the global (ie, window) object, and that creates clutter, and clutter is bad.

 

For the second question, for printable characters, keydown will repeat as long as the key is held down. Which you've discovered. If you want to do something once then you want to do it the first time the keydown event happens, and then reset when the keyup happens.

(function() {
	// separate scope for this variable
	var once = false;

	registerEventListener(document, "keydown", function(e) {
		if (once) return;
		once = true;

		// ...
	});
	registerEventListener(document, "keyup", function() {
		// reset
		once = false;
	});
})();

That may be a little finicky if you try multiple keys at once.