\$\begingroup\$
\$\endgroup\$
I'm looking for OOP tips or advice.
;(function ( ,ドル window, document, undefined ) {
if ( typeof Object.create !== 'function' ) {
Object.create = function (o) {
function F() {}
F.prototype = o;
return new F();
};
}
if (!Function.prototype.bind) { // check if native implementation available
Function.prototype.bind = function(){
var fn = this, args = Array.prototype.slice.call(arguments),
object = args.shift();
return function(){
return fn.apply(object,
args.concat(Array.prototype.slice.call(arguments)));
};
};
}
$.component = function( name, object ) {
$.fn[name] = function( options ) {
return this.each(function() {
if ( !$.data( this, name ) ) {
$.data( this, name, Object.create(object).init(
$.extend(true, {}, $.fn[name].opts, options), this )
);
}
});
};
};
})( jQuery, window , document );
(function ( ,ドル window, document, undefined ) {
var dropdown = {
opts: {},
init: function( options, el ) {
this.$el = $(el).on('click.dropdown',
(function(evt){
this.toggle(evt);
}).bind(this)
);
this.opts = $.extend(true, this.opts, options);
this.isShowing = false;
return this;
},
show: function (evt) {
if(evt) evt.preventDefault(); evt.stopPropagation();
$(this.opts.header).addClass('show-mobile-nav');
this.isShowing = true;
this.bindClose();
},
hide: function (evt) {
if(evt) evt.preventDefault();
$(this.opts.header).removeClass('show-mobile-nav');
this.isShowing = false;
return this;
},
bindClose: function () {
$('html').on('click.dropdown',
(function(evt){
var $targ = $(event.target || window.event.srcElement),
targetIsNotDropdown = !$targ.is(this.opts.menu),
targetsNotAChildOfDropdown = $(this.opts.menu).has( $targ ).length == 0;
if( targetIsNotDropdown && targetsNotAChildOfDropdown) {
$('html').off('.dropdown');
this.hide();
}
}).bind(this)
);
return this;
},
toggle: function (evt) {
if(evt) evt.preventDefault();
if ( !this.isShowing ) {
this.show(evt);
} else {
this.hide(evt);
}
}
};
$.component('dropdown',dropdown);
})( jQuery, window , document );
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
1 Answer 1
\$\begingroup\$
\$\endgroup\$
From a once over:
- Your code is not consistently indented, especially the first dozen lines need some TLC
$.component = function( name, object ) {
needs commenting, it is not clear what this is trying to achieve.evt
is an unfortunate parameter name, I would go for the classice
or the readableevent
$targ
is needlessly short, just use$target
- You could probably use helper functions for
if(evt) evt.preventDefault();
andif(evt) evt.preventDefault(); evt.stopPropagation();
- Note that
if(evt) evt.preventDefault(); evt.stopPropagation();
is exactly the kind of bug that makes people enforce{}
aroundif
blocks! toggle()
could use a ternary condition with!this.isShowing
'show-mobile-nav'
should probably not be hardcoded since you use it more than once, instead you should use a properly named variable
All in all, some pretty cool code.
answered Apr 1, 2014 at 0:58
default