Skip to content

Commit

Permalink
fix(draggable): namespace events with unique ids
Browse files Browse the repository at this point in the history
Until now if you had multiple instances of Draggable in the same page,
when destroying one of them, all of the events binded to other
instances were also destroyed.

So, we are namespacing event names with unique id’s per instance to ensure
only that only the corresponding events are unbinded.

The other solution is to cache references to the event handlers, but most
of them are proxied or throttled which makes more mess...
  • Loading branch information
vieron committed Sep 25, 2014
1 parent 1d06af1 commit 79aff38
Showing 1 changed file with 26 additions and 14 deletions.
40 changes: 26 additions & 14 deletions src/jquery.draggable.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,16 @@
var $window = $(window);
var dir_map = { x : 'left', y : 'top' };
var isTouch = !!('ontouchstart' in window);
var pointer_events = {
start: 'touchstart.gridster-draggable mousedown.gridster-draggable',
move: 'touchmove.gridster-draggable mousemove.gridster-draggable',
end: 'touchend.gridster-draggable mouseup.gridster-draggable'
};

var capitalize = function(str) {
return str.charAt(0).toUpperCase() + str.slice(1);
};

var idCounter = 0;
var uniqId = function() {
return ++idCounter + '';
}

/**
* Basic drag implementation for DOM elements inside a container.
* Provide start/stop/drag callbacks.
Expand Down Expand Up @@ -82,6 +82,8 @@
this.$dragitems = $(this.options.items, this.$container);
this.is_dragging = false;
this.player_min_left = 0 + this.options.offset_left;
this.id = uniqId();
this.ns = '.gridster-draggable-' + this.id;
this.init();
}

Expand All @@ -96,21 +98,31 @@
this.disabled = false;
this.events();

$(window).bind('resize.gridster-draggable',
$(window).bind(this.nsEvent('resize'),
throttle($.proxy(this.calculate_dimensions, this), 200));
};

fn.nsEvent = function(ev) {
return (ev || '') + this.ns;
};

fn.events = function() {
this.$container.on('selectstart.gridster-draggable',
this.pointer_events = {
start: this.nsEvent('touchstart') + ' ' + this.nsEvent('mousedown'),
move: this.nsEvent('touchmove') + ' ' + this.nsEvent('mousemove'),
end: this.nsEvent('touchend') + ' ' + this.nsEvent('mouseup'),
};

this.$container.on(this.nsEvent('selectstart'),
$.proxy(this.on_select_start, this));

this.$container.on(pointer_events.start, this.options.items,
this.$container.on(this.pointer_events.start, this.options.items,
$.proxy(this.drag_handler, this));

this.$document.on(pointer_events.end, $.proxy(function(e) {
this.$document.on(this.pointer_events.end, $.proxy(function(e) {
this.is_dragging = false;
if (this.disabled) { return; }
this.$document.off(pointer_events.move);
this.$document.off(this.pointer_events.move);
if (this.drag_start) {
this.on_dragstop(e);
}
Expand Down Expand Up @@ -265,7 +277,7 @@
this.mouse_init_pos = this.get_mouse_pos(e);
this.offsetY = this.mouse_init_pos.top - this.el_init_pos.top;

this.$document.on(pointer_events.move, function(mme) {
this.$document.on(this.pointer_events.move, function(mme) {
var mouse_actual_pos = self.get_mouse_pos(mme);
var diff_x = Math.abs(
mouse_actual_pos.left - self.mouse_init_pos.left);
Expand Down Expand Up @@ -391,9 +403,9 @@
fn.destroy = function() {
this.disable();

this.$container.off('.gridster-draggable');
this.$document.off('.gridster-draggable');
$(window).off('.gridster-draggable');
this.$container.off(this.ns);
this.$document.off(this.ns);
$(window).off(this.ns);

$.removeData(this.$container, 'drag');
};
Expand Down

0 comments on commit 79aff38

Please sign in to comment.