HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Multi-level image slider

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
levelsliderimagemulti

Problem

Here is my code I'm working with:

$('#slider_1').click( function change_slider(){
  $('#slider_1').addClass('current_tab').siblings('.current_tab').removeClass('current_tab')
  $('#cart_slider_1').show().siblings().hide();
  $('#cart_slider_1').addClass('current').siblings('.current').removeClass('current');
  numItems = $('.current .cart_slide').length;
  sliderwidth = numItems * swidth;
  $('.cart_slides').css({'left': 0, 'width': sliderwidth});
  numloop = numItems-1;
  last_slide = sliderwidth - swidth;
  i=0;
});

$('#slider_2').click( function change_slider(){
  $('#slider_2').addClass('current_tab').siblings('.current_tab').removeClass('current_tab')
  $('#cart_slider_2').show().siblings().hide();
  $('#cart_slider_2').addClass('current').siblings('.current').removeClass('current');
  numItems = $('.current .cart_slide').length;
  sliderwidth = numItems * swidth;
  $('.cart_slides').css({'left': 0, 'width': sliderwidth});
  numloop = numItems-1;
  last_slide = sliderwidth - swidth;
  i=0;
});


It repeats 6 more times, with incrementing #slider numbers and #cart_slider numbers.

I want to make this into one set, and run the variable sets through it. #slider_1, #slider_2, #slider_3, etc... and #cart_slider_1, #cart_slider_2, #cart_slider_3, etc...

The current code works fine; I just want to simplify / condense it.

Solution

$('#slider_1').click( function change_slider(){


You don't have to name the function when you pass it to an event handler.

$('#slider_1').click( function() {


An anonymous function is sufficient.

$('#slider_1').addClass('current_tab').siblings('.current_tab').removeClass('current_tab')


This seems to be missing a ; at the end.

But neither of those is what you asked:

var change_slider = function() {
  $(this).addClass('current_tab').siblings('.current_tab').removeClass('current_tab');
  $('#cart_'+$(this).attr('id')).show().siblings().hide();
  $('#cart_'+$(this).attr('id')).addClass('current').siblings('.current').removeClass('current');
  numItems = $('.current .cart_slide').length;
  sliderwidth = numItems * swidth;
  $('.cart_slides').css({'left': 0, 'width': sliderwidth});
  numloop = numItems-1;
  last_slide = sliderwidth - swidth;
  i=0;
};

$('#slider_1').click( change_slider );
$('#slider_2').click( change_slider );
$('#slider_3').click( change_slider );
$('#slider_4').click( change_slider );
$('#slider_5').click( change_slider );
$('#slider_6').click( change_slider );


The basic idea is that we assign an anonymous function to a variable and then pass that to the various click handlers. In order to make this work, we make use of the fact that when the function is called, it knows what object called it: this.

$(this).addClass('current_tab').siblings('.current_tab').removeClass('current_tab');
  $('#cart_'+$(this).attr('id')).show().siblings().hide();
  $('#cart_'+$(this).attr('id')).addClass('current').siblings('.current').removeClass('current');


In the first line, we can access our slider object directly. The cart_slider is a little more complicated, so we rely on the fact that it just adds a prefix to the id of the slider object. We get the id by calling $(this).attr('id') and use string concatenation to form the id of the cart_slider.

Note: I haven't tried to run this. Apologies in advance if I missed the syntax somewhere.

Code Snippets

$('#slider_1').click( function change_slider(){
$('#slider_1').click( function() {
$('#slider_1').addClass('current_tab').siblings('.current_tab').removeClass('current_tab')
var change_slider = function() {
  $(this).addClass('current_tab').siblings('.current_tab').removeClass('current_tab');
  $('#cart_'+$(this).attr('id')).show().siblings().hide();
  $('#cart_'+$(this).attr('id')).addClass('current').siblings('.current').removeClass('current');
  numItems = $('.current .cart_slide').length;
  sliderwidth = numItems * swidth;
  $('.cart_slides').css({'left': 0, 'width': sliderwidth});
  numloop = numItems-1;
  last_slide = sliderwidth - swidth;
  i=0;
};

$('#slider_1').click( change_slider );
$('#slider_2').click( change_slider );
$('#slider_3').click( change_slider );
$('#slider_4').click( change_slider );
$('#slider_5').click( change_slider );
$('#slider_6').click( change_slider );
$(this).addClass('current_tab').siblings('.current_tab').removeClass('current_tab');
  $('#cart_'+$(this).attr('id')).show().siblings().hide();
  $('#cart_'+$(this).attr('id')).addClass('current').siblings('.current').removeClass('current');

Context

StackExchange Code Review Q#73675, answer score: 5

Revisions (0)

No revisions yet.