Sven Sven -3 years ago 141
jQuery Question

Getting undefined error in jquery gallery

i am still a beginner so bear with me. I am trying to write an image gallery with jquery. I am getting an undefined error at :
slides[slideIndex-1].style.display = "block";

Here is my js code:

$(function(){
var slideIndex = 1;
$('.demo').on("click", function(e) {
e.preventDefault();
var o = $(this).attr("data-slide");
showSlides(slideIndex = o);
console.log(o);
});

$('.arrow').on('click', function (e){
e.preventDefault();
var g = $(this).attr('data-move');
showSlides(slideIndex += g);
});

function plusSlides(n) {
showSlides(slideIndex += n);
}
showSlides(slideIndex);

function showSlides(n) {
var i;
var slides = $(".mySlides");
var dots = $(".demo");
var captionText = $("#caption");
if (n > slides.length) {slideIndex = 1}
if (n < 1) {slideIndex = slides.length}
for (i = 0; i < slides.length; i++) {
slides[i].style.display = "none";
}
for (i = 0; i < dots.length; i++) {
dots[i].className = dots[i].className.replace(" active", "");
}
console.log(slides[slideIndex-1]);
console.log(slideIndex);
slides[slideIndex-1].style.display = "block";

dots[slideIndex-1].className += " active";
captionText.innerHTML = dots[slideIndex-1].alt;
}
});


Here is a codepen of the whole code with placeholder, somehow the big picture doesnt work, it does work on my localhost:
https://codepen.io/anon/pen/prdRXM

Answer Source

Ok... Since you use jQuery to get the images collection, better keep on using jQuery to select one.

You are using the pure JavaScript bracket [] array syntax when .eq() method is more efficient.

The index to use with .eq() is zero based... Fits perfectly with the element collection you have. So I removed all -1 from slideIndex.

Then, instead of looping the collections to hide them, just apply .hide() on the whole thing (more efficient again... And actually working).

Another issue was about the values from the data attributes being text instead of integer. I used parseInt() a couple places.

So here is you code debugged... I left all console.log() I used:

console.clear();

$(function(){
  var slideIndex = 0;
  $('.demo').on("click", function(e) {
    e.preventDefault();
    var o = parseInt($(this).attr("data-slide"));
    slideIndex = o;
    showSlides(slideIndex);
    console.log("o: "+o+" type: "+typeof(o));
  });

  $('.arrow').on('click', function (e){
    e.preventDefault();
    var g = parseInt($(this).attr('data-move'));
    console.log($(this).attr('data-move'));
    console.log("g in arrow: "+g+" Type: "+typeof(g));
    slideIndex += g;
    showSlides(slideIndex);
  });

  /*        // Moved lower... "showSlides" is not defined yet here.
  function plusSlides(n) {
    slideIndex += parseInt(n);
    showSlides(slideIndex);
  }
  showSlides(slideIndex);
  */

  function showSlides(n) {
    console.log("n in showslide: "+n+" Type: "+typeof(n));

    //var i;
    var slides = $(".mySlides");
    var dots = $(".demo");
    var captionText = $("#caption");

    if (n > slides.length-1) {slideIndex = 0}
    if (n < 0) {slideIndex = slides.length-1}

    // Why  loops?
    /*
    for (i = 0; i < slides.length; i++) {
      slides[i].style.display = "none";
    }
    for (i = 0; i < dots.length; i++) {
      dots[i].className = dots[i].className.replace(" active", "");
    }
    */

    // This works great instead of loops
    slides.hide();
    dots.hide();

    console.log(slides.eq(slideIndex));
    console.log(slideIndex);


    //slides[slideIndex-1].style.display = "block";
    //dots[slideIndex-1].className += " active";
    slides.eq(slideIndex).show().addClass("active");

    //captionText.innerHTML = dots[slideIndex-1].alt;
    captionText.html(dots.eq(slideIndex).attr("alt"));
  }

  function plusSlides(n) {
  console.log("n in plusslide: "+n+" Type: "+typeof(n));
    slideIndex += parseInt(n);
    showSlides(slideIndex);
  }
  showSlides(slideIndex);
});

CodePen

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download