I have been using a pattern similar to this basic structure below. Can anyone please let me know if this is a correct way to code an object-oriented widget for reuse? I am looking to make code than can also be extended easily and modular.
$(function(){
// wrap in jquery .fn wrapper so that may be called on a
function $lider(autoplay, velocity, controls){
var $lider = [
// props n methods, accessible through data-slider-* attributes
{
settings : {
envokeBecause : $('[data-widget~="slider"]'),
autoplay : autoplay,
speed : velocity,
showControls : controls,
slideSize : '100%',// 25% will show 4 slides if the parent container is 100% width and so on
},
bindings : {
slideRail : $('[data-function~="slide-rail"]'),
slide : $('[data-function~="slide"]'),
nextButton : $('[data-function~="next"]'),
prevButton : $('[data-function~="prev"]'),
playButton : $('[data-function~="play"]'),
pauseButton : $('[data-function~="pause"]'),
stopButton : $('[data-function~="stop"]')
// attach this functionality to the DOM
},
methods : {
slideNext : function(){ slideRail.animate({left: '-=100%'}, velocity) },
slidePrev : function(){ slideRail.animate({left: '+=100%'}, velocity) },
slideRestart : function(){ slideRail.animate({left: '0%'}, velocity) },
slideStart : function(){ window.$liderTimer = setInterval(slideNext, velocity) },
slidesCount : function(){ slideRail.children().size()
}
}
}
]
$.each($lider, function(){
// iterate through all of the slider objects properties
window.SliderProps = this;
// set slider to be accessible to the global scope
});
// slider props stored as vars
var $liderHook = SliderProps.settings.envokeBecause;
var slideRail = SliderProps.bindings.slideRail;
var slide = SliderProps.bindings.slide;
var play = SliderProps.bindings.playButton;
var next = SliderProps.bindings.nextButton;
var prev = SliderProps.bindings.prevButton;
var pause = SliderProps.bindings.pauseButton;
var stop = SliderProps.bindings.stopButton;
var slideCount = SliderProps.bindings.slideRail.children().size();
var slideCounter = 0;
function slidePrev(){
var slidePrev = SliderProps.methods.slidePrev();
}
function slideStop(){
/*slideRail.stop(); */
window.clearInterval( $liderTimer )
}
function slideRestart(){
var slidePrev = SliderProps.methods.slideRestart();
slideRail.stop();
}
function autoPlay(){
SliderProps.methods.slideStart();
}
function slideNext(){
slideCounter++
var slideNext = SliderProps.methods.slideNext();
console.log(slideCounter)
console.log(slideCount)
if (slideCounter != slideCount){
// offset var-1 for js index compensation
console.log('keep slidin..')
}
else {
console.log('stop!')
slideRestart();
slideCounter = 0;
}
}
// element -> event delegation type -> function()
slideRail.hover( slideStop )
next.click( slideNext )
prev.click( slidePrev )
stop.click( slideRestart )
pause.click( slideStop )
play.click( autoPlay )
} // close function slider()
$lider(true, 750);
});
-
\$\begingroup\$ I am really trying to understand what is happening when my function ($lider) is the same as my object that it contains such as this... -> var $lider = [{...}]; function makeGlobal() {window.$lider = this } $.each($lider, makeGlobal). And also if anyone can get what I am trying to do and help me out with improvements. That would be simply fantastic! \$\endgroup\$NicholasAbrams– NicholasAbrams2014年08月08日 01:42:05 +00:00Commented Aug 8, 2014 at 1:42
3 Answers 3
I'd suggest taking a look at jquery-boilerplate, especially the jqueryUI widget portion.
I started refactoring your code in a true $widget
manner. It's not completely functional but should point you in the right direction.
;(function ( ,ドル window, document, undefined ) {
$.widget("yourCustomNamespace.slider", {
/*
* Options to be used as defaults:
*/
options: {
autoplay: true,
controls: true,
duration: 750,
slideSize: '100%',
selectors: {
slideRail: '[data-function~="slide-rail"]',
slide: '[data-function~="slide"]',
next: '[data-function~="next"]',
prev: '[data-function~="prev"]',
play: '[data-function~="play"]',
pause: '[data-function~="pause"]',
stop: '[data-function~="stop"]'
}
},
/*
* Private Methods:
*/
_create: function() {
this._setOption("current", 0);
this._setOption("total", this.element.children().length);
this._on(this.element, {
"hover " + this.options.selectors.slideRail: $.proxy(this.stop, this),
"click " + this.options.selectors.next: $.proxy(this.next, this),
"click " + this.options.selectors.prev: $.proxy(this.prev, this),
"click " + this.options.selectors.stop: $.proxy(this.stop, this),
"click " + this.options.selectors.pause: $.proxy(this.pause, this),
"click " + this.options.selectors.play: $.proxy(this.play, this)
});
if(this.options.autoplay) {
this.start();
}
},
_destroy: function() {
this._off(this.element, "click hover");
},
_setOption: function(key, value) {
switch (key) {
case "current":
// Update internal object ...
this.options[key] = value;
// Update DOM ...
// this.element.find(this.options.selectors.slideRail.animate({...}, this.options.duration) ...
break;
default:
this.options[key] = value;
break;
}
return this._super("_setOption", key, value);
},
/*
* Public Methods:
*/
next: function() {
// TODO: Account for overflow, this.options.total
this._setOption("current", this.options.current + 1);
},
prev: function() {
// TODO: Account for underflow, this.options.total
this._setOption("current", this.options.current -1);
},
restart: function() {
this._setOption("current", 0);
},
start: function() {
this.interval = window.setInterval($.proxy(this.next, this), this.options.duration);
},
stop: function() {
window.clearInterval(this.interval);
},
getCount: function() {
return this.options.total;
}
});
})( jQuery, window, document );
There are a couple of things regarding reusability going on in here:
options
are your default parameters that can be overwritten on initializing a new widget instance, e.g.$('[data-widget~="slider"]').slider({autoplay: false});
would overwrite the default ofautoplay: true
Event binding is handled internally via
this._on()
Ideally the widget would manage its own application state logic via
this._setOption()
. That way you can distinguish between logic and DOM interaction. Whenever a value changes, you update the DOM accordingly.You can call public methods from other places via the automatic interface the widget provides, e.g.
$('[data-widget~="slider"]').slider("next")
I don't write javascript, even less so jquery, but one thing that strikes me is your naming.
$lider
$liderProps
$liderHook
I find that's a bit of a [perhaps mild] abuse of the $
dollar sign. I would expect these names to be:
$slider
$sliderProps
$sliderHook
Which would be consistent with some of the rest of the identifiers you have:
var stop = SliderProps.bindings.stopButton;
var slideCount = SliderProps.bindings.slideRail.children().size();
var slideCounter = 0;
Instead of
var $top = SliderProps.bindings.stopButton;
var $lideCount = SliderProps.bindings.slideRail.children().size();
var $lideCounter = 0;
I wouldn't treat the $
as [sometimes, sometimes not] part of the identifier.
-
\$\begingroup\$ Yes what your saying is correct, I knew I was gonna here about that part. \$\endgroup\$NicholasAbrams– NicholasAbrams2014年08月08日 01:34:03 +00:00Commented Aug 8, 2014 at 1:34
First, read @MatsMug's answer. Personally, I think that your final exported function is okay as $lider()
(it's cute, almost), but your internal stuff should still be named slider
, sliderProps
, etc.
$lider
(the variable) is a one-element array. Always. Why would you call$.each
on it?jQuery plugins typically will take settings objects and then
$.extend
them. In some cases, they will also take certain parameters for convenience. You could rewrite like the following:function $lider(settings) { var defaults = { envokeBecause: $('[data-widget~="slider"]'), autoplay: true, speed: 400, showControls: true, slideSize: '100%' }; settings = $.extend({}, defaults, settings); ...
What is
envokeBecause
? I assume you meaninvokeBecause
, but I still am not sure what it's supposed to be. It seems like it's the selector for your slider widget, in which case you can just name itselector
orsliderSelector
or something.You've got a setting named
speed
(orvelocity
depending on where in the code you look... that should be made consistent!), but that's misleading.speed
actually refers toduration
, so consider naming it that.$liderTimer
would do better as a local variable instead of being global scope.All of your
methods
should be using$lider.settings.speed
(orSliderProps.settings.speed
). Or rather,.duration
, assuming you've renamed it.slidesCount()
doesn't do anything. It needs toreturn
it. Furthermore,.size()
is deprecated and should be replaced with.length
(NOT a function). Finally, for consistency, the closing brace should be on the same line.Why are you assigning
var slidePrev
inslidePrev()
andslideNext()
...?When binding your events, it's recommended that you use
.on("click", ...
instead of.click( ...
.