This is my first plugin for JavaScript. It originated from a need for a similar feature at work and I decided to make it a bit more generic in case I needed it again. I was just wanting some criticism on my code. I added comments on the defaultOptions
to try and give a better picture of what the plugin does. Do I have JavaScript and jQuery standards and best practices down?
(function ($) {
"use strict";
// Encase this needs changed due to conflict.
var dataKey = "_collapse",
// Plugin instance class
Collapse = function (options, userEvents) {
this.options = options;
this.userEvents = userEvents;
return this;
};
// Plugin's prototype functions
Collapse.prototype = {
enable: function (container) {
var self = this,
addCollapseButtonToThese = container.find("." + this.options.collapser),
collapseThese = container.find("." + this.options.collapsee);
if (collapseThese.length <= 0) {
console.log("Error: The only children in the container are target children. There is nothing to collapse!");
return false;
}
// Run the addCollapseButton function for each target.
$(addCollapseButtonToThese).each(function () { self.options.addCollapseButton($(this)); });
// Add the uncollapsed class to every targetChild.
collapseThese.each(function () {
if (!$(this).hasClass(self.options.collapsed) && !$(this).hasClass(self.options.uncollapsed)) {
$(this).addClass(self.options.uncollapsed);
}
});
// Enable the click event for the toggle button.
$(document).on("click", "." + this.options.collapserButton, this.onToggle);
return true;
},
onToggle: function (e) {
e.preventDefault();
var plugin = $(this).data(dataKey),
toggleSuccess = Collapse.prototype.toggle(this) > -1,
events = plugin.userEvents;
if (plugin === undefined) {
console.log("Error: Plugin data not found!");
return -1;
}
if (toggleSuccess) {
// Run user-defined event.
if (typeof events.onToggle === "function") {
events.onToggle(e, this);
} else {
console.log("Error: User-defined event: onToggle is not a function!");
}
} else {
console.log("Error: There was an issue toggling collapse! User-defined event not executed.");
}
},
toggle: function (button) {
var collapse = $(button).data(dataKey),
collapsees = collapse.getCollapsees(button);
button = $(button);
if (button.hasClass(collapse.options.collapsed)) {
// Change the button image.
collapse.uncollapseElement(button);
button.attr("src", collapse.options.images.uncollapsed);
$(collapsees).each(function () {
collapse.uncollapseElement(this);
});
return 0;
}
if (button.hasClass(collapse.options.uncollapsed)) {
// Change the button image.
collapse.collapseElement(button);
button.show(); // Keep the button visible.
button.attr("src", collapse.options.images.collapsed);
$(collapsees).each(function () {
collapse.collapseElement(this);
});
return 1;
}
return -1;
},
getCollapsees: function (button) {
var row = $(button).closest("." + this.options.collapser),
rows = (this.options.relation === "sibling") ? $(row).siblings() : (this.options.relation === "child") ? $(row).find("." + this.options.collapsee) : $(row),
index = rows.index(row),
collapsees = [],
i = index + 1,
stop = false,
collapsee;
while (!stop && i < rows.length) {
collapsee = $(rows[i]);
stop = rows.filter("." + this.options.collapsee).index(collapsee) < 0;
if (stop) {
break;
}
collapsees.push(collapsee);
i += 1;
}
return collapsees;
},
collapseElement: function (element) {
$(element).removeClass(this.options.uncollapsed).addClass(this.options.collapsed);
},
uncollapseElement: function (element) {
$(element).removeClass(this.options.collapsed).addClass(this.options.uncollapsed);
}
};
// Public function that enables the plugin.
$.fn.collapse = function (userOptions, userEvents) {
var options = $.extend({}, $.fn.collapse.defaultOptions, userOptions || {}),
events = $.extend({}, $.fn.collapse.defaultUserEvents, userEvents || {}),
plugin = new Collapse(options, events),
success = false;
// Initialize the plugin.
success = plugin.enable($(this));
// Save a reference of the plugin object
// Reason: Allow later access.
// How: Use the data method to keep a copy of the plugin instance with the element.
// Where: On the collapserButtons.
$(this).find("." + options.collapserButton).data('_collapse', plugin);
if (success) {
plugin.userEvents.onEnable($(this));
}
return this;
};
// Defaults
$.fn.collapse.defaultOptions = {
collapser: "collapser", // CSS class of the container
collapsee: "collapsee", // CSS class of the items being hidden
relation: "child", // relation between the collapser and collapsee
collapserButton: "collapseButton", // CSS class of the toggle button
collapsed: "collapsed", // CSS class to define the item is collapsed (hidden).
uncollapsed: "uncollapsed", // CSS class to define the item is uncollapsed.
addCollapseButton: function (collapser) { }, // function that adds the button
images: { // image for the button
collapsed: "Images/collapsed.png",
uncollapsed: "Images/uncollapsed.png"
}
};
$.fn.collapse.defaultUserEvents = {
onToggle: function (e, button) { },
onEnable: function (container) { },
onDisable: function (container) { }
};
}(jQuery));
1 Answer 1
I noticed that your options
(削除) force (削除ここまで)strongly push the user to provide a CSS class. This doesn't make much sense to me; it seems like just having a selector instead would be better. As is, if a user wants to get something by ID, then they have to use some shenanigans like settinguserOptions.collapser
to"doesnotexist, #target"
.If you do allow arbitrary selectors, then you could perhaps rename
collapsed
tocollapsedClass
, so as to explicitly state it's a class.In the anonymous function in
Collapse.prototype.enable()
, you're creating$(this)
multiple times. This is not good practice; instead, create it once and store it in a local variable, à lavar $this = $(this);
.With
collapseEach
, you seem to be adding theoptions.uncollapsed
class to anything that does not have it yet, and also does not have theoptions.collapsed
class. There's no need to check if it has theoptions.uncollapsed
class; jQuery will not add the class twice.As a matter of fact, you can simplify this:
collapseEach.not("." + self.options.collapsed).addClass(self.options.uncollapsed);
console.log
should be used carefully. In Internet Explorer, theconsole
object won't exist unless the Developer Tools are open at load time. Thus, this will throw aReferenceError
. Either wrap it in anif (typeof console !== 'undefined')
, or define it withvar console = console || { log: function(){} };
.You've got
true
/false
for success / failure inCollapse.prototype.enable()
, butundefined
/-1
inCollapse.prototype.onToggle()
. This feels inconsistent.Your
stop
variable inCollapse.prototype.getCollapsees()
is redundant. You can write it out of the loop condition and just doif (rows.filter ...) break;
.
-
\$\begingroup\$ ah, I forgot I still had
console
in there. I was planning on switching it to athrow
. Would this be better practice? \$\endgroup\$Shelby115– Shelby1152014年08月08日 17:04:29 +00:00Commented Aug 8, 2014 at 17:04 -
\$\begingroup\$ And as for the CSS Class vs selector, I previously had a use for it but removed it, so thank you for pointing out I can use any selector now. \$\endgroup\$Shelby115– Shelby1152014年08月08日 17:21:26 +00:00Commented Aug 8, 2014 at 17:21