patternjavascriptMinor
Handling a dragging gesture using jQuery
Viewed 0 times
handlinggesturejqueryusingdragging
Problem
I'm not sure if this is the right way to do this, but I want to simply this script and hopefully use CSS3 animations. The code is really large, and parts are very irrelevant, so I'm wondering if I should first get rid of all the irrelevant parts, then go from there?
Here's the code:
```
var _target = null,
_dragx = null,
_dragy = null,
_rotate = null,
_resort = null;
var _dragging = false,
_sizing = false,
_animate = false;
var _rotating = 0,
_width = 0,
_height = 0,
_left = 0,
_top = 0,
_xspeed = 0,
_yspeed = 0;
var _zindex = 1000;
jQuery.fn.touch = function (settings) {
// DEFINE DEFAULT TOUCH SETTINGS
settings = jQuery.extend({
animate: false,
sticky: false,
dragx: true,
dragy: true,
rotate: false,
resort: false,
scale: false
}, settings);
// BUILD SETTINGS OBJECT
var opts = [];
opts = $.extend({}, $.fn.touch.defaults, settings);
// ADD METHODS TO OBJECT
this.each(function () {
this.opts = opts;
this.ontouchstart = touchstart;
this.ontouchend = touchend;
this.ontouchmove = touchmove;
this.ongesturestart = gesturestart;
this.ongesturechange = gesturechange;
this.ongestureend = gestureend;
});
};
function touchstart(e) {
_target = this.id;
_dragx = this.opts.dragx;
_dragy = this.opts.dragy;
_resort = this.opts.resort;
_animate = this.opts.animate;
_xspeed = 0;
_yspeed = 0;
$(e.changedTouches).each(function () {
var curLeft = ($('#' + _target).css("left") == 'auto') ? this.pageX : parseInt($('#' + _target).css("left"));
var curTop = ($('#' + _target).css("top") == 'auto') ? this.pageY : parseInt($('#' + _target).css("top"));
if (!_dragging && !_sizing) {
_left = (e.pageX - curLeft);
_top = (e.pageY - curTop);
_dragging = [_left, _top];
if (_resort) {
_z
Here's the code:
```
var _target = null,
_dragx = null,
_dragy = null,
_rotate = null,
_resort = null;
var _dragging = false,
_sizing = false,
_animate = false;
var _rotating = 0,
_width = 0,
_height = 0,
_left = 0,
_top = 0,
_xspeed = 0,
_yspeed = 0;
var _zindex = 1000;
jQuery.fn.touch = function (settings) {
// DEFINE DEFAULT TOUCH SETTINGS
settings = jQuery.extend({
animate: false,
sticky: false,
dragx: true,
dragy: true,
rotate: false,
resort: false,
scale: false
}, settings);
// BUILD SETTINGS OBJECT
var opts = [];
opts = $.extend({}, $.fn.touch.defaults, settings);
// ADD METHODS TO OBJECT
this.each(function () {
this.opts = opts;
this.ontouchstart = touchstart;
this.ontouchend = touchend;
this.ontouchmove = touchmove;
this.ongesturestart = gesturestart;
this.ongesturechange = gesturechange;
this.ongestureend = gestureend;
});
};
function touchstart(e) {
_target = this.id;
_dragx = this.opts.dragx;
_dragy = this.opts.dragy;
_resort = this.opts.resort;
_animate = this.opts.animate;
_xspeed = 0;
_yspeed = 0;
$(e.changedTouches).each(function () {
var curLeft = ($('#' + _target).css("left") == 'auto') ? this.pageX : parseInt($('#' + _target).css("left"));
var curTop = ($('#' + _target).css("top") == 'auto') ? this.pageY : parseInt($('#' + _target).css("top"));
if (!_dragging && !_sizing) {
_left = (e.pageX - curLeft);
_top = (e.pageY - curTop);
_dragging = [_left, _top];
if (_resort) {
_z
Solution
I'm wondering if I should first get rid of all the irrelevant parts
Yes. Especially since source control will keep any code you may want in the future. If it is not being used it is just taking up resources to maintain it. (e.g. everytime you read this section of code you have to remember what is and isn't relevant.)
If these are Constants I would suggest capitalising them. then your code will read a little easier. If they are default settings put them in an object. (If they are global variables...)
your style uses
statements like:
can be chained like:
When you use the same selector more than once its best to put it in a variable so you don't make jQuery work harder.
can become:
the if form you use here is unusual.
Might just be a matter of taste but I find it puts greater cognitive load (make me think too hard) when there are two statements per line:
Achieves the same and is much easier to read through.
Yes. Especially since source control will keep any code you may want in the future. If it is not being used it is just taking up resources to maintain it. (e.g. everytime you read this section of code you have to remember what is and isn't relevant.)
If these are Constants I would suggest capitalising them. then your code will read a little easier. If they are default settings put them in an object. (If they are global variables...)
var _target = null,
_dragx = null,
_dragy = null,
_rotate = null,
_resort = null;
var _dragging = false,
_sizing = false,
_animate = false;
var _rotating = 0,
_width = 0,
_height = 0,
_left = 0,
_top = 0,
_xspeed = 0,
_yspeed = 0;
var _zindex = 1000;your style uses
_variablename but javascript normally has the camelCase style for variables, CamelCase for classes and previously mentioned ALLCAPS for constants. Consistency is good but unless there is a reason not to i would suggest sticking with standards. (esp if others maintain it after you)_left = (e.pageX - curLeft);
_top = (e.pageY - curTop);
_dragging = [_left, _top];statements like:
$('#' + _target).css({
backgroundColor: "#4B880B"
});
$('#' + _target + ' b').text('WEEEEEEEE!!!!');can be chained like:
$('#' + _target)
.css({
backgroundColor: "#4B880B"
})
.find('b')
.text('WEEEEEEEE!!!!');When you use the same selector more than once its best to put it in a variable so you don't make jQuery work harder.
if (_dragx || _dragy) $('#' + _target).css({
position: "absolute"
});
if (_dragx) $('#' + _target).css({
left: _left + "px"
});
if (_dragy) $('#' + _target).css({
top: _top + "px"
});can become:
var $target = $('#' + _target);
if (_dragx || _dragy) $target.css({
position: "absolute"
});
if (_dragx) $target.css({
left: _left + "px"
});
if (_dragy) $target.css({
top: _top + "px"
});the if form you use here is unusual.
if (_dragy) $target.css({
top: _top + "px"
});Might just be a matter of taste but I find it puts greater cognitive load (make me think too hard) when there are two statements per line:
if(_dragy)
{
$target.css({
top: _top + "px"
});
}Achieves the same and is much easier to read through.
Code Snippets
var _target = null,
_dragx = null,
_dragy = null,
_rotate = null,
_resort = null;
var _dragging = false,
_sizing = false,
_animate = false;
var _rotating = 0,
_width = 0,
_height = 0,
_left = 0,
_top = 0,
_xspeed = 0,
_yspeed = 0;
var _zindex = 1000;_left = (e.pageX - curLeft);
_top = (e.pageY - curTop);
_dragging = [_left, _top];$('#' + _target).css({
backgroundColor: "#4B880B"
});
$('#' + _target + ' b').text('WEEEEEEEE!!!!');$('#' + _target)
.css({
backgroundColor: "#4B880B"
})
.find('b')
.text('WEEEEEEEE!!!!');if (_dragx || _dragy) $('#' + _target).css({
position: "absolute"
});
if (_dragx) $('#' + _target).css({
left: _left + "px"
});
if (_dragy) $('#' + _target).css({
top: _top + "px"
});Context
StackExchange Code Review Q#15441, answer score: 8
Revisions (0)
No revisions yet.