jQuery / JS Slider

[Mad_Griffith]

Hi everybody! I just made this slider in an object oriented fashion. I would like to know how I can improve the script. Thank you!

  var Slideshow = function(element) {   var totalWidth = 0;   this.slider_wrap = element.find('.slider_wrap');   this.slider = element.find('.slider');   this.slide = element.find('.slide');   this.first_slide = element.find('.slide:first');   this.slider_controls_prev = element.find('.slider_controls_prev');   this.slider_controls_next = element.find('.slider_controls_next');   this.all_slides = this.slide.each(function(){    totalWidth = totalWidth + jQuery(this).outerWidth(true);  });   this.maxScrollPosition = totalWidth - this.slider_wrap.outerWidth();   this.toSliderItem = function(jQuerytargetItem){     if(jQuerytargetItem.length){       this.newPosition = jQuerytargetItem.position().left;       if(this.newPosition <= this.maxScrollPosition){                  jQuerytargetItem.addClass('active');         jQuerytargetItem.siblings().removeClass('active');         this.slider.animate({           left : - this.newPosition         });       } else {         this.slider.animate({          left : - this.maxScrollPosition        });       };     };   };   this.slider.width(totalWidth);   this.first_slide.addClass('active');   this.buttons(); } Slideshow.prototype.buttons = function() {  var that = this;  this.slider_controls_prev.click(function() {    var jQuerytargetItem = that.slider.find('.active').prev();    that.toSliderItem(jQuerytargetItem);  });    this.slider_controls_next.click(function() {    var jQuerytargetItem = that.slider.find('.active').next();    that.toSliderItem(jQuerytargetItem);  });}var speakers = new Slideshow(jQuery('.slider-1'));});

The HTML:

<div class="slider_wrap"> <div class="slider clearfix">   <div class="slide">Slide Content</div> </div> <div class="slider_controls clearfix">   <div href="#" class="slider_controls_prev">     ◄   </div>   <div href="#" class="slider_controls_next">     ►   </div> </div></div>

Edited June 10, 2015 by Mad_Griffith

[justsomeguy]

It looks fine, although you might want to consider removing jQuery as a requirement.

[Mad_Griffith]

Thanks. It's on my schedule.

 

Also, I don't get when one should prefer writing methods or writing functions (apart from the case where you are writing functions which are blatantly "helpers" and shouldn't be methods of the objects as such), and when one should be prefer properties over variables.

Edited June 11, 2015 by Mad_Griffith

[justsomeguy]

You can probably tell that Javascript kind of blurs the line between methods and functions (technically, in Javascript, everything is a method). In general though, just keep related things together. If you have a function where you pass an object and the function works on that object, that should probably be a method instead. Variables that describe the object should be properties.

[Mad_Griffith]

Thanks!

 

I am converting it now and there are a couple of things I don't understand.

 

e.g. I have this code:

var Slideshow = function(element) {    var element = document.getElementById(element);    this.total_width = 0;    this.slide = element.getElementsByClassName('slide');}var speakers = new Slideshow('speakers');

• if I call the object "speakers" in the console, I get "undefined". Why so?
• If I call the object "speakers.slide" in the console, I get the uncaught error that property "slide" is undefined. Why so?
• if I try to access the property by logging "this.total_width", I would get "NaN" though it is set to the number 0. How come?
• If total_width was just a variable ("var total_width" instead of "this.total_width"), how would I access it?

Thanks!

Edited June 13, 2015 by Mad_Griffith

[Ingolme]

If the code you're showing is inside a function, then the console will not have access to the speakers variable.

 

If total_width was a variable instead of a property then it would only be accessible from inside the function.

 

The console runs in the global scope.

[Mad_Griffith]

That's the thing, there's nothing surrounding the code. Could it be anything else?

Edited June 13, 2015 by Mad_Griffith

[Mad_Griffith]

Also, why if I try to access the property by logging "this.total_width", I would get "NaN" though it is set to the number 0?

[Ingolme]

There is context missing. If that was all of your code then there wouldn't be a problem.

 

Most likely your code is wrapped in something like

$(function() {});

or

$(document).ready(function() {});

That means that it is inside a function and not accessible from the console.

[Mad_Griffith]

Hi, I am stuck at a point of converting jQuery's next() and prev() to JS.

 

Something like this

var jQuerytargetItem = this.slider.find('.active').next()

could be done like this

var jQuerytargetItem = this.slider.getElementsByClassName('active')[0].nextElementSibling

But then somewhere else I have to perform this:

if(jQuerytargetItem.length) {// some code}

and nextElementSibling doesn't have a length property...

 

How can I workaround this?

[justsomeguy]

Any node can only have 1 next sibling. If you're trying to figure out how many child nodes are under a parent node, then each node has a childNodes array which does have a length property. So you get the parent node and check its childNodes array.

[Mad_Griffith]

cool thanks!

 

I was trying (and hopefully it will work):

if(jQuerytargetItem !== null) {// some code}

But if that ends up not working I will follow your advice!

[Mad_Griffith]

Hi, one issue with returning values... can you explain how to workaround this scoping problem?

var Slideshow = function(element) {  element = document.querySelectorAll(element)[0];  var total_width = 0; // let's initialize it to 0  this.allSlides(element, total_width);   this.slider.width = total_width;  // It should be 550, no? It's still 0 instead. Why?};Slideshow.prototype.allSlides = function(element, total_width) {  var that = this;  that.slide = element.querySelectorAll('.slide');  for (i = 0; i < that.slide.length; i++) {    total_width = total_width + outerWidth(that.slide[i]);    return total_width; // it's 550 now  }};

Edited June 18, 2015 by Mad_Griffith

[justsomeguy]

When you call the allSlides function you're not doing anything with the return value. Also, if you have that return statement inside the for loop it's going to return after the first time through the loop.

[Mad_Griffith]

When you call the allSlides function you're not doing anything with the return value.

 

what do you mean? I am assigning it to this.slider.width. What should it be like?

 

Thanks for the other tip, didn't really notice! I took it out of the loop.

Edited June 18, 2015 by Mad_Griffith

[justsomeguy]

You're telling the allSlides function to return a value. Where do you get that value that was returned? How is the code supposed to know that you want that returned value to be saved in total_width? Naming all of the variables and parameters the same thing isn't going to do it.

[Mad_Griffith]

Uhm, so what should the code rather be like? Nothing comes to my mind.

Edited June 18, 2015 by Mad_Griffith

[justsomeguy]

You need to study this:http://www.w3schools.com/js/js_functions.asp

total_width = this.allSlides(element, total_width);

[Mad_Griffith]

You need to study this:http://www.w3schools.com/js/js_functions.asp

total_width = this.allSlides(element, total_width);

 

ok, didn't think of assigning it. Thanks, I am replacing the "this.allSlides(element, total_width)" with it.

Edited June 18, 2015 by Mad_Griffith