0

I have written some simple jquery for a few elements that are close together. Each one works just fine, but I was hoping to get some help making my jquery more efficient, and smaller in order to place in a seperate JS file. I am newer to jQuery and do not really know how to combine what I have written. Any help is greatly appreciated! My code is below:

$('.fade-carousel').on('init', function(event, slick, currentSlide, nextSlide) {
  if ($('.slick-active .slide-container div').hasClass('intro-bg')) {
    $('#header').addClass('transparent-header')
  } else {
    $('#header').removeClass('transparent-header');
  }
  $('.fade-carousel').on('beforeChange', function(event, slick, currentSlide, nextSlide) {
    if ($('.slide-container:eq(' + nextSlide + ') > div').hasClass('intro-bg')) {
      $('#header').addClass('transparent-header')
    } else {
      $('#header').removeClass('transparent-header');
    }

  });
});
$('.fade-carousel').on('init', function(event, slick, currentSlide, nextSlide) {
  if ($('.slick-active .slide-container div').hasClass('intro-bg')) {
    $('.slick-arrow').removeClass('black-arrow')
  } else {
    $('.slick-arrow').addClass('black-arrow')
  }
  $('.fade-carousel').on('beforeChange', function(event, slick, currentSlide, nextSlide) {
    if ($('.slide-container:eq(' + nextSlide + ') > div').hasClass('intro-bg')) {
      $('.slick-arrow').removeClass('black-arrow')
    } else {
      $('.slick-arrow').addClass('black-arrow');
    }
  });
});
$('.fade-carousel').on('init', function(event, slick, currentSlide, nextSlide) {
  if ($('.slick-active .slide-container div').hasClass('intro-bg')) {
    $('.scroll-arrow i').removeClass('black-arrow')
  } else {
    $('.scroll-arrow i').addClass('black-arrow')
  }
  $('.fade-carousel').on('beforeChange', function(event, slick, currentSlide, nextSlide) {
    if ($('.slide-container:eq(' + nextSlide + ') > div').hasClass('intro-bg')) {
      $('.scroll-arrow i').removeClass('black-arrow')
    } else {
      $('.scroll-arrow i').addClass('black-arrow');
    }
  });
});
$('.fade-carousel').on('init', function(event, slick, currentSlide, nextSlide) {
  if ($('.slick-active .slide-container div').hasClass('video-intro')) {
    $('#header').addClass('transparent-header')
  }
  $('.fade-carousel').on('beforeChange', function(event, slick, currentSlide, nextSlide) {
    if ($('.slide-container:eq(' + nextSlide + ') > div').hasClass('video-intro')) {
      $('#header').addClass('transparent-header')
    }
  });
});
user9263373
  • 1,054
  • 8
  • 15
AaronS
  • 131
  • 13
  • It looks like you are repeating bindings, with different internal conditionals and changes. Nothing says you can't perform all the conditionals and logic within one single binding. – Taplar Feb 14 '18 at 20:20
  • Could you post a snippet of the relevant HTML? We may be able to use some sibling selectors or `.closest()` in some cases to keep the parser from re-traversing the DOM each time. However for this to work, we'll need to know the structure of your carousel... – War10ck Feb 14 '18 at 21:15

1 Answers1

0

You could precompute the jquery objects at the top of your work:

var $scrollArrowIcon = $('.scroll-arrow i');

And then use that for things like

$scrollArrowIcon.addClass('whatever');
$scrollArrowIcon.removeClass('something_else');

One risk here though is that by the time your click handlers run, the actual set of elements that match that selector might have changed, and your jQuery object will only contain the DOM elements that were present at the time you created it.

You can improve your existing code by taking advantage of jquery chaining ( http://www.jquery-tutorial.net/introduction/method-chaining/ )... for example

$('.fade-carousel').on('init', function() {
  ... do something
}).on('click', function() {
  ... something else
}).on('beforeChange', function() {
  ... something else again
});

When you have the "if x, add class Y, else remove class Y", you can reduce repetition with:

$('#header').toggleClass(
  'transparent-header',
  $('.slick-active .slide-container div').hasClass('intro-bg')
);
Mike K
  • 601
  • 5
  • 8
  • It won't be "cached", the DOM will have already rendered so it will not update until something cause it to be rendered again. – Drewness Feb 14 '18 at 21:29
  • i'm not sure i understand your comment Drewness but I have removed my reference to the word 'cached' to make my intent more clear – Mike K Feb 14 '18 at 21:31
  • Read [this](https://stackoverflow.com/questions/24053838/store-jquery-selector-in-variable) over, it should clear things up. – Drewness Feb 14 '18 at 21:39
  • I saw what you mean and I tried a couple other things, and i get no console errors, but it does not work. – AaronS Feb 15 '18 at 17:15
  • Yep, that page says the same thing as what I've described above: if you create the jQuery object early, then it will contain elements matching your selector at the time the jQuery object is created, and not contain items that were added later. – Mike K Feb 16 '18 at 02:35