patternjavascriptModerate
Gmail Mouse Gesture
Viewed 0 times
gesturegmailmouse
Problem
Gmail had this awesome feature and it was discontinued for some reason. Nowadays I have installed a nice Google Chrome plugin for this job.
A couple of months ago I had to develop a similar feature for a project and I think that could be improved but I don't know how.
I'm here asking for a code review or suggestions.
```
(function($, undefined) {
$.fn.mousePointer = function(options) {
var settings = $.extend({}, {
size : 95,
bgColor : "#000",
arrColor : "#FFF",
btnLeft : null,
btnRight : null,
btnUp : null,
btnDown : null,
className : "p_c_arr_" + new Date().getTime()
}, options);
var fncCreateButton = function(target, degrees, left, top) {
var r = settings.size, w = r / 5;
var c = document.createElement("canvas");
c.className = settings.className;
c.height = r, c.width = r;
var ctx = c.getContext("2d");
// CREATE THE BACKGROUND BUTTON
ctx.beginPath();
ctx.arc(r, r, r, Math.PI, 1.5 * Math.PI);
ctx.arc(r, r, w 2, 1.5 Math.PI, Math.PI, true);
ctx.closePath();
ctx.strokeStyle = settings.bgColor;
ctx.stroke();
ctx.fillStyle = settings.bgColor;
ctx.fill();
// CREATE THE ARROW INSIDE THE BUTTON
ctx.beginPath();
ctx.moveTo(r / 2 - w / 2, r / 2 - w / 2);
ctx.lineTo(r / 2, r / 2 + w);
ctx.lineTo(r / 2 + w, r / 2);
ctx.closePath();
ctx.strokeStyle = settings.arrColor;
ctx.stroke();
ctx.fillStyle = settings.arrColor;
ctx.fill();
// SET THE ANGLE AND POSITION
var canvas = $(c);
canvas.css({
"-webkit-transform" : 'rotate(' + degrees + 'deg)',
"-moz-transform" : 'rotate(' + degrees + 'de
A couple of months ago I had to develop a similar feature for a project and I think that could be improved but I don't know how.
I'm here asking for a code review or suggestions.
```
(function($, undefined) {
$.fn.mousePointer = function(options) {
var settings = $.extend({}, {
size : 95,
bgColor : "#000",
arrColor : "#FFF",
btnLeft : null,
btnRight : null,
btnUp : null,
btnDown : null,
className : "p_c_arr_" + new Date().getTime()
}, options);
var fncCreateButton = function(target, degrees, left, top) {
var r = settings.size, w = r / 5;
var c = document.createElement("canvas");
c.className = settings.className;
c.height = r, c.width = r;
var ctx = c.getContext("2d");
// CREATE THE BACKGROUND BUTTON
ctx.beginPath();
ctx.arc(r, r, r, Math.PI, 1.5 * Math.PI);
ctx.arc(r, r, w 2, 1.5 Math.PI, Math.PI, true);
ctx.closePath();
ctx.strokeStyle = settings.bgColor;
ctx.stroke();
ctx.fillStyle = settings.bgColor;
ctx.fill();
// CREATE THE ARROW INSIDE THE BUTTON
ctx.beginPath();
ctx.moveTo(r / 2 - w / 2, r / 2 - w / 2);
ctx.lineTo(r / 2, r / 2 + w);
ctx.lineTo(r / 2 + w, r / 2);
ctx.closePath();
ctx.strokeStyle = settings.arrColor;
ctx.stroke();
ctx.fillStyle = settings.arrColor;
ctx.fill();
// SET THE ANGLE AND POSITION
var canvas = $(c);
canvas.css({
"-webkit-transform" : 'rotate(' + degrees + 'deg)',
"-moz-transform" : 'rotate(' + degrees + 'de
Solution
First, a word about usage:
In line 5 you do
Use
In lines 16-68 you create a canvas, paint a static image to it, then rotate it using CSS and return the canvas. Instead, consider using SVG:
The downside of external SVG is that javascript cannot easily tweak the view. Even then, an inline SVG still has the benefit of higher graphics quality. Even if you don't want to learn SVG or use external editors, I recommend rotating the button in code and rendering an already rotated version onto the canvas.
Also, out of desktop browsers, only Safari needs a prefix for CSS transformations.
Also, not sure what's the point of stroking and filling the same shape with the same color, except maybe to provide a more solid outline if the color used happens to be semitransparent.
L64: Use
L72-73: It's great that you namespace your event handlers. Note that you can supply multiple events at once:
L78:
L84-91: What is that? The absence of an
L101:
L124:
Finally, the math can be simplified:
Concerning the signature of
$().mousePointer is sub-optimal. Why instantiate a jQuery object that you never even touch? Instead, you can attach your plugin as $.mousePointer = ... and call it as $.mousePointer(...).In line 5 you do
$.extend({}, {...}, options). The point of passing an empty object as the first argument is to prevent modifying an existing object. Since you're passing an object literal (i.e. an object that isn't used anywhere else) as the second argument, this is not needed. $.extend({...}, options) will do. If you decide to move the defaults definition to the top of the file (a good idea), $.extend({}, defaults, options) does make sense.Use
Date.now() instead of new Date().getTime() to avoid an extra allocation and also to make the code more readable. Or rather, don't rely on classes to identify your buttons.In lines 16-68 you create a canvas, paint a static image to it, then rotate it using CSS and return the canvas. Instead, consider using SVG:
- Smaller size and easier to change (graphics designers are more likely to be savvy in Inkscape than in canvas javascript). It can also be stored in an external file and not clutter up code. Also note that the client might want to use a PNG image instead. You don't give him that option.
- It has about the same support as canvas does (IE8 supports neither, other desktop browsers support both).
- SVG is a vector format. Canvas renders onto a bitmap. This means higher quality when doing transformations (scaling, rotating by anything else than a multiple of 90 degrees, displaying on a retina display ...)
- SVG works with javascript disabled (not a benefit in this case, I admit)
- As far as I can tell, browsers will prefer to allocate CSS-transformed canvas buffers on the GPU. I don't think these buttons need, or can benefit from, GPU acceleration. Browsers are getting better, but there's only so much they can guess.
The downside of external SVG is that javascript cannot easily tweak the view. Even then, an inline SVG still has the benefit of higher graphics quality. Even if you don't want to learn SVG or use external editors, I recommend rotating the button in code and rendering an already rotated version onto the canvas.
Also, out of desktop browsers, only Safari needs a prefix for CSS transformations.
Also, not sure what's the point of stroking and filling the same shape with the same color, except maybe to provide a more solid outline if the color used happens to be semitransparent.
L64: Use
$(document.body) instead of $("body"). It looks better and it's slightly faster.L72-73: It's great that you namespace your event handlers. Note that you can supply multiple events at once:
$(document).off("mousemove.mousePointer mouseup.mousePointer"). Similarly for $().on. Alternatively, you can pass the event handlers themselves to off.L78:
if ( target = button.data("target")) -- This looks like a typo. If you do want to do an assignment inside a condition while testing for truthiness, you can denote that using an extra pair of parentheses: if((target = button.data("target"))). Better yet, assign first, then test. var target = button.data("target"); if(target) .... Also, your target variable was never declared an is accidentally global. This mistake is harder to make when assigning first and testing then.L84-91: What is that? The absence of an
onclick property doesn't mean the DOM node doesn't have any event handlers. Also, don't use $._data. It's a private function and can be removed at any time. If you want to provide a default event handler, accept an alternate event handler, attach it yourself and if one isn't provided, attach your own instead. This also prevents an awkward situation when someone wants to detect click events on your buttons without disrupting their functionality. In short, what event handlers are attached to a DOM node is none of your business.L101:
if(e.which == 3) surely deserves a comment: // right mouse button. Also, === is generally preferred over ==.L124:
x and _x aren't very descriptive variable names. menuCenterX and menuClickX? Even firstClickX and secondClickX are easier to understand.Finally, the math can be simplified:
1 - (_x - (x - r)) / r
= 1 - (_x - x + r) / r
= 1 - (_x - x)/r - r/r
= 0 - (_x - x) / r
= (x - _x) / rConcerning the signature of
fncCreateButton: the left and top arguments do not respect the button symmetry. I would probably choose the arc center as the reference point. It would simplify the caller code and possibly also the callee code. The amount of responsibilities of this function seems acceptable. The function can be split further, although the comments do serve their purpose and code duplication is not an issue here. Not sure if the same function should both create an element and append it to the document - even if the creation is separated into a different function. The single responsibility principle Code Snippets
1 - (_x - (x - r)) / r
= 1 - (_x - x + r) / r
= 1 - (_x - x)/r - r/r
= 0 - (_x - x) / r
= (x - _x) / rContext
StackExchange Code Review Q#61538, answer score: 13
Revisions (0)
No revisions yet.