2
\$\begingroup\$

I created my first plugin in jQuery and just want some inputs on how I can improve the code and features.

Here is the fiddle

 $.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);
Quill
12k5 gold badges41 silver badges93 bronze badges
asked Jul 11, 2014 at 7:09
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

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.

answered Jul 11, 2014 at 8:42
\$\endgroup\$
2
  • \$\begingroup\$ I tried to fix few issues but unable to remove redundancy part. link \$\endgroup\$ Commented Jul 11, 2014 at 10:59
  • \$\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\$ Commented Jul 11, 2014 at 11:08

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.