Skip to content

Commit

Permalink
Draggable: Ignore scroll offsets for abspos draggables. Fixes #9315 -…
Browse files Browse the repository at this point in the history 
… Draggable: jumps down with offset of scrollbar
  • Loading branch information
@mikesherov
mikesherov committed Aug 12, 2013
1 parent 1c58573 commit 263d078
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 6 deletions.
25 changes: 25 additions & 0 deletions tests/unit/draggable/draggable_core.js
View file
Expand Up @@ -139,6 +139,31 @@ test( "#6258: not following mouse when scrolled and using overflow-y: scroll", f
}); });
}); });


test( "#9315: Draggable: jumps down with offset of scrollbar", function() {
expect( 2 );

var element = $( "#draggable2" ).draggable({
stop: function( event, ui ) {
equal( ui.position.left, 11, "left position is correct when position is absolute" );
equal( ui.position.top, 11, "top position is correct when position is absolute" );
$( "html" ).scrollTop( 0 ).scrollLeft( 0 );
}
}),
contentToForceScroll = $( "<div>" ).css({
height: "10000px",
width: "10000px"
});

contentToForceScroll.appendTo( "#qunit-fixture" );
$( "html" ).scrollTop( 300 ).scrollLeft( 300 );

element.simulate( "drag", {
dx: 1,
dy: 1,
moves: 1
});
});

test( "#5009: scroll not working with parent's position fixed", function() { test( "#5009: scroll not working with parent's position fixed", function() {
expect( 2 ); expect( 2 );


Expand Down
22 changes: 16 additions & 6 deletions ui/jquery.ui.draggable.js
View file
Expand Up @@ -452,7 +452,12 @@ $.widget("ui.draggable", $.ui.mouse, {


var mod = d === "absolute" ? 1 : -1, var mod = d === "absolute" ? 1 : -1,
document = this.document[ 0 ], document = this.document[ 0 ],
scroll = this.cssPosition === "absolute" && !( this.scrollParent[ 0 ] !== document && $.contains( this.scrollParent[ 0 ], this.offsetParent[ 0 ] ) ) ? this.offsetParent : this.scrollParent; useOffsetParent = this.cssPosition === "absolute" && ( this.scrollParent[ 0 ] === document || !$.contains( this.scrollParent[ 0 ], this.offsetParent[ 0 ] ) ),
scroll = useOffsetParent ? this.offsetParent : this.scrollParent,
// we need to test if offsetParent was used here because Blink incorrectly reports a 0 scrollTop
// on document.documentElement when the page is scrolled. Checking for offsetParent normalizes
// this across browsers. Blink bug: https://code.google.com/p/chromium/issues/detail?id=157855
scrollIsRootNode = useOffsetParent && ( /(html|body)/i ).test( scroll[ 0 ].nodeName );


//Cache the scroll //Cache the scroll
if (!this.offset.scroll) { if (!this.offset.scroll) {
Expand All @@ -464,13 +469,13 @@ $.widget("ui.draggable", $.ui.mouse, {
pos.top + // The absolute mouse position 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.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.offset.parent.top * mod - // The offsetParent's offset without borders (offset + border)
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : this.offset.scroll.top ) * mod ) ( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) ) * mod)
), ),
left: ( left: (
pos.left + // The absolute mouse position 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.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.offset.parent.left * mod - // The offsetParent's offset without borders (offset + border)
( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : this.offset.scroll.left ) * mod ) ( ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : scrollIsRootNode ? 0 : this.offset.scroll.left ) * mod)
) )
}; };


Expand All @@ -481,7 +486,12 @@ $.widget("ui.draggable", $.ui.mouse, {
var containment, co, top, left, var containment, co, top, left,
o = this.options, o = this.options,
document = this.document[ 0 ], document = this.document[ 0 ],
scroll = this.cssPosition === "absolute" && !( this.scrollParent[ 0 ] !== document && $.contains( this.scrollParent[ 0 ], this.offsetParent[ 0 ] ) ) ? this.offsetParent : this.scrollParent, useOffsetParent = this.cssPosition === "absolute" && ( this.scrollParent[ 0 ] === document || !$.contains( this.scrollParent[ 0 ], this.offsetParent[ 0 ] ) ),
scroll = useOffsetParent ? this.offsetParent : this.scrollParent,
// we need to test if offsetParent was used here because Blink incorrectly reports a 0 scrollTop
// on document.documentElement when the page is scrolled. Checking for offsetParent normalizes
// this across browsers. Blink bug: https://code.google.com/p/chromium/issues/detail?id=157855
scrollIsRootNode = useOffsetParent && ( /(html|body)/i ).test( scroll[ 0 ].nodeName ),
pageX = event.pageX, pageX = event.pageX,
pageY = event.pageY; pageY = event.pageY;


Expand Down Expand Up @@ -542,14 +552,14 @@ $.widget("ui.draggable", $.ui.mouse, {
this.offset.click.top - // Click offset (relative to the element) 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.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.offset.parent.top + // The offsetParent's offset without borders (offset + border)
( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : this.offset.scroll.top ) ( this.cssPosition === "fixed" ? -this.scrollParent.scrollTop() : ( scrollIsRootNode ? 0 : this.offset.scroll.top ) )
), ),
left: ( left: (
pageX - // The absolute mouse position pageX - // The absolute mouse position
this.offset.click.left - // Click offset (relative to the element) 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.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.offset.parent.left + // The offsetParent's offset without borders (offset + border)
( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : this.offset.scroll.left ) ( this.cssPosition === "fixed" ? -this.scrollParent.scrollLeft() : ( scrollIsRootNode ? 0 : this.offset.scroll.left ) )
) )
}; };


Expand Down

1 comment on commit 263d078

@alanclarke
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.