Slideshow on key press not working properly

Question

So I have this code. I'm doing something wrong. I want the left arrow to go back (show the previous image) but it isn't working. It shows the next image (like the right arrow). Whatever arrow I click, left or right it shows the next image. I've tried millions of different things and I can't seem to find the problem. Can anyone help me?

I also like the sources to loop. When the last source from the array has been reached, I want to loop back to the first source again (and to the last source when you reach the first).

Btw, I also have a code for the image to change on click. I'm pretty sure it has nothing to do with the problem, but I decided to keep it, just in case it's messing something up.
Thank you :)

---

HTML:

<div class="container">
    <div id="slideshow"> 
        <img alt="slideshow" src="1.jpg" id="imgClickAndChange" onclick="changeImage()"/> 
    </div>
</div>

JS:

<script>
    var imgs = ["2.jpg", "3.jpg", "4.jpg", "5.jpg"];

    function changeImage(dir) {
        var img = document.getElementById("imgClickAndChange");
        img.src = imgs[imgs.indexOf(img.src) + (dir || 1)] || imgs[dir ? imgs.length - 1 : 0];
    }

    document.onkeydown = function(e) {
        e = e || window.event;
        if (e.keyCode == '37') {
            changeImage(-1) //left <- show Prev image
        } else if (e.keyCode == '39') {
            changeImage() // right -> show next image
        }
    }
</script>

<script language="javascript">
    var imgs = ["2.jpg", "3.jpg", "4.jpg", "5.jpg"];
    function changeImage() {
        document.getElementById("imgClickAndChange").src = imgs[0];
        imgs.push(imgs.shift())
    }
</script>

Answer 1

You were on the right track, but you got a few things wrong:

• You have two variables with the same name.
• You have two functions with the same name.
• If a function expects an argument, you must always send an argument (or null), you can not only send -1 for left, and nothing for right. (And in your case, you're actually making it more difficult for yourself by not sending a 1 for right).
• || imgs[dir ? imgs.length - 1 : 0], doesn't do what you think it does. You're checking dir as if it's a boolean. JavaScript can interpret 1 and -1 as booleans (true and false respectively), but because you only send along -1 it will not work correctly.

Below is your code with these issues fixed:

---

JS, HTML:

var imgs = ["https://i.sstatic.net/NjC6V.jpg",
            "https://i.sstatic.net/0eBBQ.gif",
            "https://i.sstatic.net/uhjjB.png",
            "https://i.sstatic.net/cNhjf.jpg",
            "https://i.sstatic.net/Dn175.png"];

document.onkeydown = function(event) {
  var e = event || window.event;
  if (e.keyCode == '37') { //LEFT
    changeImage(-1);
  } else if (e.keyCode == '39') { //RIGHT
    changeImage(1);
  }
}

function changeImage(dir) {
  var img = document.getElementById("imgClickAndChange");
  img.src = imgs[imgs.indexOf(img.src)+dir] || imgs[(dir==1) ? 0 : imgs.length-1];
}

#imgClickAndChange {
  width: 150px;
  height: 150px;
}

<div class="container">
  <div id="slideshow"> 
    <img id="imgClickAndChange" src="https://i.sstatic.net/NjC6V.jpg" alt="" onclick="changeImage(1)" /> 
  </div>
</div>

(fiddle: http://jsfiddle.net/k3wxhc58/8/)

• In the ternary operator, instead of (dir==1) you could just use dir, since JavaScript translates a 1 to true. But using (dir==1) is safer because that will create a real boolean.
• I replaced the images in the imgs-array with links to images that actually exist, so that the script works without any errors.
• I added some CSS, this is only for the answer's sake, so that all images have the same size.

---

If you don't want the images to loop, change the changeImage() function to this:

function changeImage(dir) {
    var img = document.getElementById("imgClickAndChange");
    var newSrc = imgs[imgs.indexOf(img.src)+dir];

    if (imgs.indexOf(newSrc) != -1) { //only sets new src if it's in the imgs-array (i.e. if it exists)
        img.src = newSrc;
    }
}

(fiddle: http://jsfiddle.net/k3wxhc58/9/)