Skip to content

Commit 82f588e

Browse files
Woody Gilkmikesherov
Woody Gilk
authored andcommitted
Draggable: Fix double offset bug when scrolling. Fixes #6817 - Draggable: auto scroll goes double distance when dragging
1 parent 39a2f46 commit 82f588e

File tree

2 files changed

+51
-4
lines changed

2 files changed

+51
-4
lines changed

tests/unit/draggable/draggable_options.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,40 @@ test( "scroll, scrollSensitivity, and scrollSpeed", function() {
11031103
TestHelpers.draggable.restoreScroll( document );
11041104
});
11051105

1106+
test( "#6817: auto scroll goes double distance when dragging", function() {
1107+
expect( 2 );
1108+
1109+
var offsetBefore,
1110+
distance = 10,
1111+
viewportHeight = $( window ).height(),
1112+
element = $( "#draggable1" ).draggable({
1113+
scroll: true,
1114+
stop: function( e, ui ) {
1115+
equal( ui.offset.top, newY, "offset of item matches pointer position after scroll" );
1116+
equal( ui.offset.top - offsetBefore.top, distance, "offset of item only moves expected distance after scroll" );
1117+
}
1118+
}),
1119+
scrollSensitivity = element.draggable( "option", "scrollSensitivity" ),
1120+
oldY = viewportHeight - scrollSensitivity,
1121+
newY = oldY + distance;
1122+
1123+
element.offset({
1124+
top: oldY,
1125+
left: 1
1126+
});
1127+
1128+
offsetBefore = element.offset();
1129+
1130+
element.simulate( "drag", {
1131+
handle: "corner",
1132+
dx: 1,
1133+
y: newY,
1134+
moves: 1
1135+
});
1136+
1137+
TestHelpers.draggable.restoreScroll( document );
1138+
});
1139+
11061140
test( "snap, snapMode, and snapTolerance", function() {
11071141
expect( 9 );
11081142

ui/jquery.ui.draggable.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ $.widget("ui.draggable", $.ui.mouse, {
135135
left: this.offset.left - this.margins.left
136136
};
137137

138+
//Reset scroll cache
139+
this.offset.scroll = false;
140+
138141
$.extend(this.offset, {
139142
click: { //Where the click happened, relative to the element
140143
left: event.pageX - this.offset.left,
@@ -433,18 +436,23 @@ $.widget("ui.draggable", $.ui.mouse, {
433436
var mod = d === "absolute" ? 1 : -1,
434437
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);
435438

439+
//Cache the scroll
440+
if (!this.offset.scroll) {
441+
this.offset.scroll = {top : scroll.scrollTop(), left : scroll.scrollLeft()};
442+
}
443+
436444
return {
437445
top: (
438446
pos.top + // The absolute mouse position
439447
this.offset.relative.top * mod + // Only for relative positioned nodes: Relative offset from element to offset parent
440448
this.offset.parent.top * mod - // The offsetParent's offset without borders (offset + border)
441-
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : scroll.scrollTop() ) ) * mod)
449+
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) ) * mod)
442450
),
443451
left: (
444452
pos.left + // The absolute mouse position
445453
this.offset.relative.left * mod + // Only for relative positioned nodes: Relative offset from element to offset parent
446454
this.offset.parent.left * mod - // The offsetParent's offset without borders (offset + border)
447-
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : scroll.scrollLeft() ) * mod)
455+
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : this.offset.scroll.left ) * mod)
448456
)
449457
};
450458

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

470+
//Cache the scroll
471+
if (!this.offset.scroll) {
472+
this.offset.scroll = {top : scroll.scrollTop(), left : scroll.scrollLeft()};
473+
}
474+
462475
/*
463476
* - Position constraining -
464477
* Constrain the position to a mix of grid, containment.
@@ -508,14 +521,14 @@ $.widget("ui.draggable", $.ui.mouse, {
508521
this.offset.click.top - // Click offset (relative to the element)
509522
this.offset.relative.top - // Only for relative positioned nodes: Relative offset from element to offset parent
510523
this.offset.parent.top + // The offsetParent's offset without borders (offset + border)
511-
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : scroll.scrollTop() ) ))
524+
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) ))
512525
),
513526
left: (
514527
pageX - // The absolute mouse position
515528
this.offset.click.left - // Click offset (relative to the element)
516529
this.offset.relative.left - // Only for relative positioned nodes: Relative offset from element to offset parent
517530
this.offset.parent.left + // The offsetParent's offset without borders (offset + border)
518-
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : scroll.scrollLeft() ))
531+
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : this.offset.scroll.left ))
519532
)
520533
};
521534

0 commit comments

Comments
 (0)