Custom slider issue

Question

I have written a custom slider with object-oriented javascript as seen below. I have included the module in a fiddle here https://jsfiddle.net/5z29xhzg/7/. After sliding left or right, the slides are cloned and appended accordingly so that the user can loop through the slider as much as wanted. There is a separate function for controlling the active tab. When used separately, the tabs and slider work perfectly, but I am having an issue when the two are used in conjuction. For instance, clicking 'blue apron' and then clicking the left slide button (which should take you to the 'dave & busters' slide) takes you to the bliss slide. Or clicking the last slide using the tabs and then clicking the next button does not show anything. Can someone point out the flaw in the object that I have written. Any help is much appreciated!

    GiftSlider = {
    prev: '.slider-container .prev',
    next: '.slider-container .next',
    slide: '.slide',
    slidesContainer: '#slides',
    navLink: '.gift-nav li a',
    init: function() {
        // Initialize nextSlide function when clicking right arrow
        $(this.next).click(this.nextSlide.bind(this));
        $(this.next).click(this.activeTabs.bind(this));
        // Initialize prevSlide function when clicking left arrow
        $(this.prev).click(this.prevSlide.bind(this));
        $(this.prev).click(this.activeTabs.bind(this));
        // Initialize toggleSlides and activeTab functions when clicking nav links
        $(this.navLink).click(this.toggleSlides.bind(this));
        $(this.navLink).click(this.activeTabs.bind(this));
    },
    nextSlide: function(e) {
        // Prevent default anchor click
        e.preventDefault();
        // Set the current slide
        var currentSlide = $('.slide.current');
        // Set the next slide
        var nextSlide = $('.slide.current').next();
        // remove the current class from the current slide
        currentSlide.removeClass("current");
        // Move the current slide to the end so we can cycle through
        currentSlide.clone().appendTo(this.slidesContainer);
        // remove the last slide so there is not two instances
        currentSlide.remove();
        // Add current class to next slide to display it
        nextSlide.addClass("current");
    },
    prevSlide: function(e) {
        // Prevent defaulct anchor click
        e.preventDefault();
        // Set the current slide
        var currentSlide = $('.slide.current');
        // Set the last slide
        var lastSlide = $('.slide').last();
        // remove the current class from the current slide
        currentSlide.removeClass("current");
        // Move the last slide to the beginning so we can cycle through
        lastSlide.clone().prependTo(this.slidesContainer);
        // remove the last slide so there is not two instances
        lastSlide.remove();
        // Add current class to new first slide
        $('.slide').first().addClass("current");
    },
    toggleSlides: function(e) {
        // Prevent defaulct anchor click
        e.preventDefault();
        var itemData = e.currentTarget.dataset.index;
        var currentSlide = $('.slide.current');
        currentSlide.removeClass("current");
        newSlide = $('#slide-' + itemData);
        // currentSlide.clone().appendTo(this.slidesContainer);
        newSlide.addClass("current");
        // console.log(newSlide);
    },
    activeTabs: function() {
        // *** This could be much simpler if we didnt need to match the slider buttons
        // *** up with nav tabs.  Because I need to track the slider as well, I have
        // *** made this its own function to be used in both instances
        // get the active slide
        var activeSlide = $('.slide').filter(':visible');
        // get the active slide's id
        var currentId = activeSlide.attr('id');
        // grab just the number from the active slide's id
        var currentNum = currentId.slice(-1);
        // remove any active gift-nav links
        $(this.navLink).removeClass("active");
        // get the current tab by matching it to the current slide's id
        var currentTab = $('.gift-nav li a[data-index="' + currentNum + '"]');
        // make that active
        currentTab.addClass("active");
    }
}

$(document).ready(function(){

    // Init our objects
    GiftSlider.init();

});

Answer 1

The bug seems to be in toggleSlides.

EDIT: The following doesn't work

When the page loads, currentSlide is slide 1. Now say you click the 3rd tab. At this point, you need to move the slide before the 3rd tab i.e. the 2nd tab to the end. When you say currentSlide.clone().appendTo(this.slidesContainer);, you are instead moving the first slide to the end. Therefore no matter which tab you clicked, when you click the previous button it was going to the first slide.

If instead you do newSlide.prev().clone().appendTo(this.slidesContainer); the code seems to work fine.

toggleSlides: function(e) {
    // Prevent defaulct anchor click
    e.preventDefault();
    var itemData = e.currentTarget.dataset.index;
    var currentSlide = $('.slide.current');
    currentSlide.removeClass("current");
    newSlide = $('#slide-' + itemData);
    //currentSlide.clone().appendTo(this.slidesContainer);
    newSlide.prev().clone().appendTo(this.slidesContainer);
    newSlide.addClass("current");
    //console.log("In toggle slide: "+newSlide.next().attr("id"));
    //console.log("In toggle slide: "+newSlide.prev().attr("id"));
    //console.log("In toggle slide: "+$('.slide.current').next().attr("id"));
},

This seems to work. Check it out at https://jsfiddle.net/rfgnm992/1/. Your nextSlide and previousSlide seems to keep the current slide at the beginning. toggleSlides wasn't doing that.

toggleSlides: function(e) {
        // Prevent defaulct anchor click
        e.preventDefault();
        var itemData = e.currentTarget.dataset.index;
        var currentSlide = $('.slide.current');
        currentSlide.removeClass("current");
        newSlide = $('#slide-' + itemData);
        //keep new slide at the beginning and move the preceding slides to the end 
        newSlide.nextAll().addBack().prependTo(this.slidesContainer);
    //console.log("NewSlide.next: "+newSlide.next().attr('id') + "NewSlide.next.next: "+newSlide.next().next().attr('id')+"newSlide.next.next.next: "+newSlide.next().next().next().attr('id'));
        //currentSlide.clone().appendTo(this.slidesContainer);
        newSlide.addClass("current");
        // console.log(newSlide);
    },

Answer 2

sorry I got back a bit later than expected. Took a look there. Think you're over-complicating things with the whole append/prepend/cloning thing.

I got it working, but there's a minor bug in it still. It cycles grand, forward and back and the correct links highlight, however when you click on a random link it doesn't immediately highlight, but when you click the next/prev buttons, the relevant links from the chosen image highlight. It's certainly an advance!!! I'm sure I could roll it out with another look, but its 2am here, and I've already been looking at it for an hour and a half!

Here's a fiddle and a snippet (just js cos my msg was too long - I just took out the paragraph at the end of the content, no css changes)

GiftSlider = {
    prev: '.slider-container .prev',
    next: '.slider-container .next',
    slide: '.slide',
    slidesContainer: '#slides',
    navLink: '.gift-nav li a',
    init: function() {
        // Initialize nextSlide function when clicking right arrow
        $(this.next).click(this.nextSlide.bind(this));
        $(this.next).click(this.activeTabs.bind(this));
        // Initialize prevSlide function when clicking left arrow
        $(this.prev).click(this.prevSlide.bind(this));
        $(this.prev).click(this.activeTabs.bind(this));
        // Initialize toggleSlides and activeTab functions when clicking nav links      
        $(this.navLink).click(this.activeTabs.bind(this));
    $(this.navLink).click(this.toggleSlides.bind(this));
    },
    nextSlide: function(e) {
        // Prevent default anchor click
        e.preventDefault();
        // Set the current slide
        var currentSlide = $('.slide.current');
        // Set the next slide
    var currentId = currentSlide.attr('id');
   var currNum = (currentId.slice(-1));
   var nextNum;
   var increment = 1;
   if (currNum == 4){
       nextNum = 1;
   }
   else
   {   
       nextNum = parseInt(currNum) + parseInt(increment) ;
   }
        var nextSlide = $('#slide-' + nextNum); 
        // remove the current class from the current slide
        currentSlide.removeClass("current");
        // Add current class to next slide to display it
        nextSlide.addClass("current");
    // remove the last slide so there is not two instances
    },
    prevSlide: function(e) {
        // Prevent default anchor click
        e.preventDefault();
        // Set the current slide
        var currentSlide = $('.slide.current');
        // Set the last slide
   var currentId = currentSlide.attr('id');
   var currNum = (currentId.slice(-1));
   var prevNum;
   var decrement =1;
   if (currNum == 1){
       prevNum = 4;
   }
   else
   {   
       prevNum = parseInt(currNum) - parseInt(decrement) ;
   }
        var prevSlide = $('#slide-' + prevNum);
    // Move the last slide to the beginning so we can cycle through
        currentSlide.removeClass("current");
    // Add current class to new first slide
        prevSlide.addClass("current");
    },
    toggleSlides: function(e) {
        // Prevent defaulct anchor click
        e.preventDefault();     
    var itemData = e.currentTarget.dataset.index;
        var currentSlide = $('.slide.current');
        currentSlide.removeClass("current");  
        newSlide = $('#slide-' + itemData);
        newSlide.addClass("current");

    },
    activeTabs: function() {
        var activeSlide = $('.slide').filter('.current');
        // get the active slide's id
        var currentId = activeSlide.attr('id');
        // grab just the number from the active slide's id
      var currentNum = currentId.slice(-1);
        // remove any active gift-nav links
        $(this.navLink).removeClass("active");
        // get the current tab by matching it to the current slide's id
        var currentTab = $('.gift-nav li a[data-index="'+  currentNum + '"]');
        // make that active
        currentTab.addClass("active");
    }
}

$(document).ready(function(){

    // Init our objects
    GiftSlider.init();

});