I created my first plugin in jQuery and just want some inputs on how I can improve the code and features.
$.fn.fruitZoom = function (options) {
var defaults = {
ele: "p,span",
};
var settings = $.extend(defaults, options);
$("#fruitZoom :first-child").click(function () {
fontSize = $(defaults.ele).css("font-size")
$(defaults.ele).css("font-size", parseInt(fontSize) + 2);
console.log(fontSize)
});
$("#fruitZoom :last-child").click(function () {
fontSize = $(defaults.ele).css("font-size")
$(defaults.ele).css("font-size", parseInt(fontSize) - 2);
console.log(fontSize)
});
};
})(jQuery);
options = {
ele: "p,h1",
}
$("#fruitZoom").fruitZoom(options);
1 Answer 1
Flexibility
Your plugin relies on very specific markup; an element with an ID of fruitZoom
having a first and last child element meant to be used as zoom in and zoom out buttons, respectively.
This is not flexible enough to be generally useful. What if you want multiple zoom controls on the page at once? What if you want to use a slider instead of buttons, or keyboard controls?
Naming and style
The identifier ele
is not appropriate for a user-facing option name. The name elements
would be an improvement. Better yet, use something more descriptive of what the value should to be, such as selector
. After all, the user does not assign elements to this property, they assign a selector to it.
Your use of semicolons is inconsistent. Don't rely on automatic semicolon insertion in some places and not in others. Either terminate every statement with a semicolon, or only use them as separators in places where they are necessary. Pick a style and stick to it.
Behavior
This line here...
fontSize = $(defaults.ele).css("font-size")
...only gets the size of the first element matched by the ele
selector. That means as soon as you zoom in or out, all elements matching your selector are resized to the size of the first element. You can see this in your own example code on JSFiddle; as soon as you zoom, the heading changes its size to match the first paragraph. This is probably not desirable behavior.
Redundancy
This block of code...
$("#fruitZoom :first-child").click(function () {
fontSize = $(defaults.ele).css("font-size")
$(defaults.ele).css("font-size", parseInt(fontSize) + 2);
console.log(fontSize)
});
...is nearly identical to the next:
$("#fruitZoom :last-child").click(function () {
fontSize = $(defaults.ele).css("font-size")
$(defaults.ele).css("font-size", parseInt(fontSize) - 2);
console.log(fontSize)
});
This is a sure sign that you should refactor your code to eliminate redundancy. Consider writing a named function, replacing the + 2
/ - 2
part with + amount
, where amount
is a variable you pass in, having a value of 2 or negative 2.
-
-
\$\begingroup\$ @user3534656 did you see my comments on your post? They remove the redundancy. Also,
.css
has a special syntax for incrementing/decrementing values. If you use that, it fixes the problem of everything getting scaled to the same size. jsfiddle.net/zpYb4/6 \$\endgroup\$Dagg– Dagg2014年07月11日 11:08:30 +00:00Commented Jul 11, 2014 at 11:08