Draggable: Fix double offset bug when scrolling. Fixes #6817 - Dragga…

…ble: auto scroll goes double distance when dragging
jquery · Mar 14, 2013 · 82f588e82b887fdcb2406da2c5dfedc2f13ebdc9 · 82f588e
1 parent 39a2f46
commit 82f588e82b887fdcb2406da2c5dfedc2f13ebdc9
Unified Split

Showing with 51 additions and 4 deletions.

+34 −0 tests/unit/draggable/draggable_options.js

+17 −4 ui/jquery.ui.draggable.js
diff --git a/tests/unit/draggable/draggable_options.js b/tests/unit/draggable/draggable_options.js
@@ -1103,6 +1103,40 @@ test( "scroll, scrollSensitivity, and scrollSpeed", function() {
 	TestHelpers.draggable.restoreScroll( document );
 });

+test( "#6817: auto scroll goes double distance when dragging", function() {
+	expect( 2 );
+
+	var offsetBefore,
+		distance = 10,
+		viewportHeight = $( window ).height(),
+		element = $( "#draggable1" ).draggable({
+			scroll: true,
+			stop: function( e, ui ) {
+				equal( ui.offset.top, newY, "offset of item matches pointer position after scroll" );
+				equal( ui.offset.top - offsetBefore.top, distance, "offset of item only moves expected distance after scroll" );
+			}
+		}),
+		scrollSensitivity = element.draggable( "option", "scrollSensitivity" ),
+		oldY = viewportHeight - scrollSensitivity,
+		newY = oldY + distance;
+
+	element.offset({
+		top: oldY,
+		left: 1
+	});
+
+	offsetBefore = element.offset();
+
+	element.simulate( "drag", {
+		handle: "corner",
+		dx: 1,
+		y: newY,
+		moves: 1
+	});
+
+	TestHelpers.draggable.restoreScroll( document );
+});
+
 test( "snap, snapMode, and snapTolerance", function() {
 	expect( 9 );

diff --git a/ui/jquery.ui.draggable.js b/ui/jquery.ui.draggable.js
@@ -135,6 +135,9 @@ $.widget("ui.draggable", $.ui.mouse, {
 			left: this.offset.left - this.margins.left
 		};

+		//Reset scroll cache
+		this.offset.scroll = false;
+
 		$.extend(this.offset, {
 			click: { //Where the click happened, relative to the element
 				left: event.pageX - this.offset.left,
@@ -433,18 +436,23 @@ $.widget("ui.draggable", $.ui.mouse, {
 		var mod = d === "absolute" ? 1 : -1,
 			scroll = this.cssPosition === "absolute" && !(this.scrollParent[0] !== document && $.contains(this.scrollParent[0], this.offsetParent[0])) ? this.offsetParent : this.scrollParent, scrollIsRootNode = (/(html|body)/i).test(scroll[0].tagName);

+		//Cache the scroll
+		if (!this.offset.scroll) {
+			this.offset.scroll = {top : scroll.scrollTop(), left : scroll.scrollLeft()};
+		}
+
 		return {
 			top: (
 				pos.top	+																// The absolute mouse position
 				this.offset.relative.top * mod +										// Only for relative positioned nodes: Relative offset from element to offset parent
 				this.offset.parent.top * mod -										// The offsetParent's offset without borders (offset + border)
-				( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : scroll.scrollTop() ) ) * mod)
+				( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) ) * mod)
 			),
 			left: (
 				pos.left +																// The absolute mouse position
 				this.offset.relative.left * mod +										// Only for relative positioned nodes: Relative offset from element to offset parent
 				this.offset.parent.left * mod	-										// The offsetParent's offset without borders (offset + border)
-				( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : scroll.scrollLeft() ) * mod)
+				( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : this.offset.scroll.left ) * mod)
 			)
 		};

@@ -459,6 +467,11 @@ $.widget("ui.draggable", $.ui.mouse, {
 			pageX = event.pageX,
 			pageY = event.pageY;

+		//Cache the scroll
+		if (!this.offset.scroll) {
+			this.offset.scroll = {top : scroll.scrollTop(), left : scroll.scrollLeft()};
+		}
+
 		/*
 		 * - Position constraining -
 		 * Constrain the position to a mix of grid, containment.
@@ -508,14 +521,14 @@ $.widget("ui.draggable", $.ui.mouse, {
 				this.offset.click.top	-												// Click offset (relative to the element)
 				this.offset.relative.top -												// Only for relative positioned nodes: Relative offset from element to offset parent
 				this.offset.parent.top +												// The offsetParent's offset without borders (offset + border)
-				( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : scroll.scrollTop() ) ))
+				( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) ))
 			),
 			left: (
 				pageX -																	// The absolute mouse position
 				this.offset.click.left -												// Click offset (relative to the element)
 				this.offset.relative.left -												// Only for relative positioned nodes: Relative offset from element to offset parent
 				this.offset.parent.left +												// The offsetParent's offset without borders (offset + border)
-				( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : scroll.scrollLeft() ))
+				( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : this.offset.scroll.left ))
 			)
 		};