Opened 7 years ago

Closed 7 years ago

#9121 closed bug (duplicate)

Auto scrolling (i.e. scroll:true) with Sortable/Draggable hasn't worked since jquery-ui 1.8.24 powered by jQuery 1.8.2

Reported by: dcarrith Owned by: dcarrith
Priority: minor Milestone: none
Component: ui.sortable Version: 1.10.1
Keywords: Cc:
Blocked by: Blocking:

Description

The difference between jQuery UI 1.8.24 and jQuery UI 1.10.1 seems to be how the helper is determining the this.scrollParent. In jQuery UI 1.8.24, it's correctly using either window or html. However, in jQuery UI 1.10.1, this.scrollParent seems to be the parent of the element on which the sortable was instantiated (which doesn't work because the div isn't the container in which it needs to scroll, the document and/or html element is). In my case, it's setting this.scrollParent to the div that wraps the ul element on which I instantiated the sortable (so the li elements can be sortable).

I fixed it in my application by commenting out the if block "if(this.scrollParent[0] !== document && this.scrollParent[0].tagName !== "HTML") {" so that it always treats it as if the this.scrollParent[0] is document or html. This works for my needs, but I'm guessing there's a better way to fix it.

Do scrolling if(this.options.scroll) {

/*if(this.scrollParent[0] !== document && this.scrollParent[0].tagName !== "HTML") {

if((this.overflowOffset.top + this.scrollParent[0].offsetHeight) - event.pageY < o.scrollSensitivity) {

alert("scrolling"); alert("this.scrollParent[0].id: "+this.scrollParent[0].id+"\nthis.scrollParent[0].tagName: "+this.scrollParent[0].tagName); this.scrollParent[0].scrollTop = scrolled = this.scrollParent[0].scrollTop + o.scrollSpeed; alert("this.scrollParent[0].scrollTop: "+this.scrollParent[0].scrollTop);

} else if(event.pageY - this.overflowOffset.top < o.scrollSensitivity) {

this.scrollParent[0].scrollTop = scrolled = this.scrollParent[0].scrollTop - o.scrollSpeed; alert("scrolling"); alert("this.scrollParent[0].id: "+this.scrollParent[0].id+"\nthis.scrollParent[0].tagName: "+this.scrollParent[0].tagName); alert("this.scrollParent[0].scrollTop: "+this.scrollParent[0].scrollTop);

}

if((this.overflowOffset.left + this.scrollParent[0].offsetWidth) - event.pageX < o.scrollSensitivity) {

this.scrollParent[0].scrollLeft = scrolled = this.scrollParent[0].scrollLeft + o.scrollSpeed;

} else if(event.pageX - this.overflowOffset.left < o.scrollSensitivity) {

this.scrollParent[0].scrollLeft = scrolled = this.scrollParent[0].scrollLeft - o.scrollSpeed;

}

} else {*/

if(event.pageY - $(document).scrollTop() < o.scrollSensitivity) {

alert("scrolling"); alert("$(window).height(): "+$(window).height()+"\nevent.pageY: "+event.pageY+"\n$(document).scrollTop(): "+$(document).scrollTop()+"\no.scrollSensitivity: "+o.scrollSensitivity); scrolled = $(document).scrollTop($(document).scrollTop() - o.scrollSpeed);

} else if($(window).height() - (event.pageY - $(document).scrollTop()) < o.scrollSensitivity) {

alert("scrolling"); alert("$(window).height(): "+$(window).height()+"\nevent.pageY: "+event.pageY+"\n$(document).scrollTop(): "+$(document).scrollTop()+"\no.scrollSensitivity: "+o.scrollSensitivity); scrolled = $(document).scrollTop($(document).scrollTop() + o.scrollSpeed);

}

if(event.pageX - $(document).scrollLeft() < o.scrollSensitivity) {

scrolled = $(document).scrollLeft($(document).scrollLeft() - o.scrollSpeed);

} else if($(window).width() - (event.pageX - $(document).scrollLeft()) < o.scrollSensitivity) {

scrolled = $(document).scrollLeft($(document).scrollLeft() + o.scrollSpeed);

}

}

if(scrolled !== false && $.ui.ddmanager && !o.dropBehaviour) {

$.ui.ddmanager.prepareOffsets(this, event);

}

}

Change History (7)

comment:1 Changed 7 years ago by tj.vantoll

Owner: set to dcarrith
Status: newpending

Hi dcarrith,

Thanks for taking the time to contribute to the jQuery UI project. Could you please provide a reduced test case that shows the issue that you're experiencing. You can use this as a starting point - http://jsfiddle.net/tj_vantoll/wjxe4/.

If you're having these issues in IE this might be related to #9057.

Thanks!

comment:2 Changed 7 years ago by onereason

Ran into the same problem.

Used your starting point and created a simple version that breaks in > IE9: http://jsfiddle.net/vtYGg/2/

Try dragging any elements down and you will see that it never scrolls in IE. Works in all other browsers I tested.

Last edited 7 years ago by onereason (previous) (diff)

comment:3 Changed 7 years ago by dcarrith

Status: pendingnew

I did some more testing this morning after I received the email notification when @onereason commented on the issue. I noticed a difference in how "scrolled" was being set. So, I modified the section I had originally commented out (see above in original ticket write-up). This actually fixes the scrolling issue, but it causes another weird issue where the sorted item gets overlayed on top of the item below where it's dropped (i.e. the items don't get shifted up or down appropriately). So, I'm wondering why in the if block, the scrolled variable is getting set to an integer. But, in the else block it gets set to an object. So, to "fix" it, I changed it so the scrolled variable would get set to an object or false, to be in line with the else block (which I know works fine).

		//Do scrolling
		if(this.options.scroll) {

			if(this.scrollParent[0] !== document && this.scrollParent[0].tagName !== "HTML") {

				if((this.overflowOffset.top + this.scrollParent[0].offsetHeight) - event.pageY < o.scrollSensitivity) {
					
					// The original line results in scrolled being set to an integer 
					//this.scrollParent[0].scrollTop = scrolled = this.scrollParent[0].scrollTop + o.scrollSpeed;
					
					// This modified line results in scrolled being set to an object
					scrolled = this.scrollParent[0].scrollTop( this.scrollParent[0].scrollTop + o.scrollSpeed );
				
				} else if(event.pageY - this.overflowOffset.top < o.scrollSensitivity) {
					
					// The original line results in scrolled being set to an integer 
					//this.scrollParent[0].scrollTop = scrolled = this.scrollParent[0].scrollTop - o.scrollSpeed;

					// This modified line results in scrolled being set to an object
					scrolled = this.scrollParent[0].scrollTop( this.scrollParent[0].scrollTop - o.scrollSpeed );
				}

				if((this.overflowOffset.left + this.scrollParent[0].offsetWidth) - event.pageX < o.scrollSensitivity) {
					this.scrollParent[0].scrollLeft = scrolled = this.scrollParent[0].scrollLeft + o.scrollSpeed;
				} else if(event.pageX - this.overflowOffset.left < o.scrollSensitivity) {
					this.scrollParent[0].scrollLeft = scrolled = this.scrollParent[0].scrollLeft - o.scrollSpeed;
				}

			} else {

				if(event.pageY - $(document).scrollTop() < o.scrollSensitivity) {
					scrolled = $(document).scrollTop($(document).scrollTop() - o.scrollSpeed);
				} else if($(window).height() - (event.pageY - $(document).scrollTop()) < o.scrollSensitivity) {
					scrolled = $(document).scrollTop($(document).scrollTop() + o.scrollSpeed);
				}

				if(event.pageX - $(document).scrollLeft() < o.scrollSensitivity) {
					scrolled = $(document).scrollLeft($(document).scrollLeft() - o.scrollSpeed);
				} else if($(window).width() - (event.pageX - $(document).scrollLeft()) < o.scrollSensitivity) {
					scrolled = $(document).scrollLeft($(document).scrollLeft() + o.scrollSpeed);
				}

			}

			if(scrolled !== false && $.ui.ddmanager && !o.dropBehaviour) {
				$.ui.ddmanager.prepareOffsets(this, event);
			}
		}
Last edited 7 years ago by dcarrith (previous) (diff)

comment:4 Changed 7 years ago by Scott González

@dcarrith: Would you be interested in sending a pull request with your proposed fix?

comment:5 in reply to:  4 Changed 7 years ago by dcarrith

Replying to scott.gonzalez:

@dcarrith: Would you be interested in sending a pull request with your proposed fix?

Sure, but with this fix in place, there is still an issue where the items aren't being shifted up or down. I can try to fix that, but won't be able to look at that until tonight. I would also want to make sure it still works in a simple jsfiddle.

Update: scratch that. My "fix" wasn't much of a fix. I noticed the js error caused by calling scrollTop on an HTML DIV element, which is not a function. I'll look at it some more tonight. It's kind of strange how that caused it to work though.

Last edited 7 years ago by dcarrith (previous) (diff)

comment:6 Changed 7 years ago by dcarrith

Okay, this seems to fix it. Notice the additional check to make sure this.scrollParent[0].offsetHeight is less than $(window).height() in the 2nd line of code:

		//Do scrolling
		if(this.options.scroll) {

			if( ( this.scrollParent[0] !== document && this.scrollParent[0].tagName !== "HTML" ) && ( this.scrollParent[0].offsetHeight < $(window).height() ) ) {

				if((this.overflowOffset.top + this.scrollParent[0].offsetHeight) - event.pageY < o.scrollSensitivity) {
					this.scrollParent[0].scrollTop = scrolled = this.scrollParent[0].scrollTop + o.scrollSpeed;
				} else if(event.pageY - this.overflowOffset.top < o.scrollSensitivity) {
					this.scrollParent[0].scrollTop = scrolled = this.scrollParent[0].scrollTop - o.scrollSpeed;
				}

				if((this.overflowOffset.left + this.scrollParent[0].offsetWidth) - event.pageX < o.scrollSensitivity) {
					this.scrollParent[0].scrollLeft = scrolled = this.scrollParent[0].scrollLeft + o.scrollSpeed;
				} else if(event.pageX - this.overflowOffset.left < o.scrollSensitivity) {
					this.scrollParent[0].scrollLeft = scrolled = this.scrollParent[0].scrollLeft - o.scrollSpeed;
				}

			} else {

				if(event.pageY - $(document).scrollTop() < o.scrollSensitivity) {
					scrolled = $(document).scrollTop($(document).scrollTop() - o.scrollSpeed);
				} else if($(window).height() - (event.pageY - $(document).scrollTop()) < o.scrollSensitivity) {
					scrolled = $(document).scrollTop($(document).scrollTop() + o.scrollSpeed);
				}

				if(event.pageX - $(document).scrollLeft() < o.scrollSensitivity) {
					scrolled = $(document).scrollLeft($(document).scrollLeft() - o.scrollSpeed);
				} else if($(window).width() - (event.pageX - $(document).scrollLeft()) < o.scrollSensitivity) {
					scrolled = $(document).scrollLeft($(document).scrollLeft() + o.scrollSpeed);
				}

			}

			if(scrolled !== false && $.ui.ddmanager && !o.dropBehaviour) {
				$.ui.ddmanager.prepareOffsets(this, event);
			}
		}

comment:7 Changed 7 years ago by tj.vantoll

Resolution: duplicate
Status: newclosed

Duplicate of #7546.
This is a duplicate of #7546 so I'm closing this issue in favor of the existing one. @dcarrith, feel free to continue with your pull request but I believe the core problem is in scrollParent. See my last comment on #7546: http://bugs.jqueryui.com/ticket/7546#comment:6.

Note: See TracTickets for help on using tickets.