Ghoyos
Ghoyos

Reputation: 622

Issues with using JavaScript to increment the position of an element

A previous QUESTION I had has led me to construct the code below. What Im trying to do is increment the element up/down/left/right based on keyboard arrow selection

"<div id="test"></div>"

by 20 pixels (or however many I chose) at a time. I have the "left" and "right" direction working for the most part but something interesting keeps happening when I try and change directions. First off whichever direction I initially chose to go first does not work on the first key press, I have to hit the arrow key of either left or right twice to start transitioning. Then once the transitions do start working; if im traveling left and then want to change direction to the right; it goes left even though im hitting the right key at least once before it goes the correct direction corresponding to the key I chose. This is a bit difficult to explain by text so here is the code pen so you can see what I mean. CODEPEN (hit the left and right arrow keys a couple times so clearly see the issue)

Also the functions moveDown() and moveUp() dont even move the box positioning at all. To try and trouble shoot the problem I added console.log('moveUp') and console.log('moveDown') in order to see whether the console was recognizing the key code when I struck it, and it clearly does. It even recognizes when I strike the left or right key but still runs the incorrect direction at-least once before it starts going the correct direction on the left and right keys.

I have a feeling it make have to do with transitions not being cleared or maybe using an IF statement might not be the base way to read the key code?

Also thought about using switch case for JavaScript key code recognition but I am pretty sure the issue would still be the same and im not skilled enough to pull off switchcase yet.

ALL I WANT is the box to go the direction I tell it to go using the arrows on my keypad, I am close but as you can see I am missing some key concepts

document.addEventListener("keydown", keyBoardInput);
var left = 0;
var top = 0;


function moveUp() {
  console.log('moveUp');
  document.getElementById("test").style.top = top + "px";
  top -= 20;
  document.getElementById("test").style.transition = "all 1s";
};

function moveDown() {
  console.log('moveDown');
  document.getElementById("test").style.top = top + "px";
  top += 20;
  document.getElementById("test").style.transition = "all 1s";
};

function moveLeft() {
  console.log('moveLeft');
  document.getElementById("test").style.left = left + "px";
  left -= 20;
  document.getElementById("test").style.transition = "all 1s";
};

function moveRight() {
  console.log('moveRight');
  document.getElementById("test").style.left = left + "px";
  left += 20;
  document.getElementById("test").style.transition = "all 1s";
};

function keyBoardInput() {
  var i = event.keyCode;

  if (i == 38) {
    moveUp()
  };

  if (i == 40) {
    moveDown()
  };

  if (i == 37) {
    moveLeft()
  };

  if (i == 39) {
    moveRight()
  };
};
#test {
  position: relative;
  width: 30px;
  border: 1px solid blue;
  height: 10px;
  top: 0px;
  left: 0px;
}
<div id="test"></div>

Upvotes: 1

Views: 221

Answers (2)

andreas
andreas

Reputation: 16936

2 issues: You updated your globals too late, just do it before you use it on the element. And the variable name "top" is reserved, just use another one (see this blog post). Here the code:

document.addEventListener("keydown", keyBoardInput);
var left = 0;
var topx = 0;


function moveUp() {
  console.log('moveUp');
  topx -= 20;
  document.getElementById("test").style.top = topx + "px";
  document.getElementById("test").style.transition = "all 1s";
};

function moveDown() {
  console.log('moveDown');
  topx += 20;
  document.getElementById("test").style.top = topx + "px";
  document.getElementById("test").style.transition = "all 1s";
};

function moveLeft() {
  console.log('moveLeft');
  left -= 20;
  document.getElementById("test").style.left = left + "px";
  document.getElementById("test").style.transition = "all 1s";
};

function moveRight() {
  console.log('moveRight');
  left += 20;
  document.getElementById("test").style.left = left + "px";
  document.getElementById("test").style.transition = "all 1s";
};

function keyBoardInput() {
  var i = event.keyCode;

  if (i == 38) {
    moveUp()
  };

  if (i == 40) {
    moveDown()
  };

  if (i == 37) {
    moveLeft()
  };

  if (i == 39) {
    moveRight()
  };
};
#test {
  position: relative;
  width: 30px;
  border: 1px solid blue;
  height: 10px;
  top: 0px;
  left: 0px;
}
<div id="test"></div>

Upvotes: 1

Karin
Karin

Reputation: 8600

You need to do the arithmetic for top and left before setting it in the CSS :)

function moveUp() {
  console.log('moveUp');
  top -= 20;
  document.getElementById("test").style.top = top + "px";
  document.getElementById("test").style.transition = "all 1s";
};

function moveDown() {
  console.log('moveDown');
  top += 20;
  document.getElementById("test").style.top = top + "px";
  document.getElementById("test").style.transition = "all 1s";
};

function moveLeft() {
  console.log('moveLeft');
  left -= 20;
  document.getElementById("test").style.left = left + "px";
  document.getElementById("test").style.transition = "all 1s";
};

function moveRight() {
  console.log('moveRight');
  left += 20;
  document.getElementById("test").style.left = left + "px";
  document.getElementById("test").style.transition = "all 1s";
};

Here's a working fiddle: https://jsfiddle.net/x2yxexs9/

Upvotes: 1

Related Questions