Reputation: 101
Well I'm kind of new with jQuery and javascript and I have written 'some' code for a image slider but it is enormous. I am convinced it can be shorter, but I don't know how. Could somebody show me how it sould be done so I can learn from it ? Thanks in advance :D
PS I don't want to look lazy, I really want to get better in Javascript and Jquery
Here is my code
jQuery(document).ready(function(){
//Variables for image slider
var imgWrapper = $('.carousel-wrapper'),
img = $('.carousel-box'),
imgWidth = $('.carousel-box').width(),
imgLength = $('.carousel-box').length, // 4
currentMargin = imgWrapper.css('margin-left').replace('px',''), //0px
responsiveLength = 3,
maxMargin = -((imgLength - responsiveLength) * imgWidth),
minMargin = 0; //0px
$(window).resize(function(){
if ($(window).width() <= 1080) {
responsiveLength = 2;
maxMargin = -((imgLength - responsiveLength) * imgWidth);
} else if ($(window).width() <= 700) {
responsiveLength = 1;
maxMargin = -((imgLength - responsiveLength) * imgWidth);
} else {
responsiveLength = 3;
maxMargin = -((imgLength - responsiveLength) * imgWidth);
}
});
//Transition animation on click
$('.portbutton').click(function(){
//Get the direction
var dir = $(this).data('dir');
if (dir === 'next' && currentMargin != maxMargin) {
currentMargin -= imgWidth;
imgWrapper.animate({marginLeft: currentMargin},300);
} else if (dir === 'next' && currentMargin === maxMargin){
currentMargin = minMargin;
imgWrapper.animate({marginLeft: currentMargin},300);
} else if (dir === 'prev' && currentMargin != minMargin){
currentMargin += imgWidth;
imgWrapper.animate({marginLeft: currentMargin},300);
} else {
currentMargin = maxMargin;
imgWrapper.animate({marginLeft: currentMargin},300);
}
});
});
Upvotes: 0
Views: 44
Reputation: 1736
You code isn't really that bad. One thing to know about javascript, and especially using jquery, it does get / look messy and verbose. There have been attempts to clean up javascript syntax with CoffeeScript, but it hasn't caught on in mainstream development as much as some had hoped (myself included).
Also, making things as abbreviated as possible is not always in your's and other's best interest. You might end up making your code a whole lot less readable and therefore less maintainable when you come back to change the code a year later.
That being said, you could perhaps make the code cleaner and more readable by splitting some of your logic into separate functions. Anonymous functions are nice (and more or less standard), but you could perhaps break those out into named functions.
Upvotes: 1
Reputation: 226
You can use some functions for:
function animate(currentMargin){
imgWrapper.animate({marginLeft: currentMargin},300);
}
function getMaxMargin(imgLength,responsiveLength,imgWidth){
maxMargin = -((imgLength - responsiveLength) * imgWidth);
return maxMargin
}
And maybe avoid create all time the value responsiveLength, if it just a integer that you always set before use , then just punt the integer instead, you will have more memory space.
Upvotes: 2