Draggable: auto scroll bug introduced in gsap 1.18.1

[jamesbrobb]

Bower's just auto updated me to 1.18.2 and i've noticed that there's now an issue with Draggable... which at this point i'm presuming relates to autoScroll. If i have a container with scrollable content and try to drag below the page fold, it begins to move and then jumps the container back to the top. So i'm no longer able to drag content in scrolling containers.

I'm just flagging this with a brief description (as i don't have time to investigate further this evening) incase anything immediately comes to mind?

I'll do some further investigation tomorrow/saturday and put together an example.

[jamesbrobb]

The issue i'm seeing is caused by the highlighted changes to the Draggable update method.

this.update = function(applyBounds, ignoreExternalChanges) {
    var x = self.x,
        y = self.y;
    updateMatrix();
    if (applyBounds) {
        self.applyBounds();
    } else {
        if (dirty && ignoreExternalChanges) {
            render();
        }
        syncXY(true);
    }
    if (self.isPressed && ((allowX && Math.abs(x - self.x) > 0.01) || (allowY && (Math.abs(y - self.y) > 0.01 && !rotationMode)))) {
        recordStartPositions();
    }
    // THIS ADDITION IN 1.18.1 CAUSES THE ISSUE
    if (self.autoScroll) {
        _recordMaxScrolls(target.parentNode);
        checkAutoScrollBounds = true;
        render();
    }
    /////////////////////////////////////////////////////////////////////////////////
    return self;
};

---

I have an onDrag handler that checks the current position of the element being dragged and then repositions/updates the other elements/Draggables in the list if necessary. To reposition/update an element and its associated draggable i use this logic.

TweenLite.to(element, 0.3, {
    css: {
        transform: 'translate3d(0,' + (y - props.y) + 'px,0)'
    },
    onUpdate: draggable.update
});

As specified in the docs (https://greensock.com/docs/#/HTML5/GSAP/Utils/Draggable/update/) i use the onUpdate callback to update the Draggables position through draggable.update.

Details Updates the Draggable's x and y properties so that they reflect the target element's current position. This can be useful if, for example, you manually change or tween the element's position, but then you want to make sure the Draggable's x and y reflect those changes. You could even point a tween's onUpdate to the Draggable's update method to ensure things are synchronized throughout a tween.

But due to the addition in 1.18.1 i've highlighted, this callback now also continually calls the render function, causing the behaviour i'm seeing.

[jamesbrobb]

Also i've just noticed that using https://cdnjs.cloudflare.com/ajax/libs/gsap/latest/utils/Draggable.min.js does not load the latest version of Draggable.js, it loads 0.14.1

[jackdoyle]

Thanks for the details, @jamesbrobb. Very helpful. A few comments/questions:

1) Are you saying that you try to tween he target's position WHILE it is being dragged? For example, the user could be in the middle of dragging, and you start animating the dragging element to a different position and yet you want the user's mouse/finger movement to ALSO control the element simultaneously? If I understood it correctly, it seems counterintuitive and I wouldn't really expect that to work since two different inputs are fighting with each other, but maybe I misunderstood.

2) Do you have a reduced test case, perhaps in a codepen or jsfiddle? That'd be super helpful for troubleshooting and making sure I'm solving the right problem.

Regarding the "/latest/" CDN directory, that's unfortunately out of our control and frankly it's pretty frustrating. We've talked to the CDNJS and CloudFlare folks MANY times about this, and the CloudFlare team (who is ultimately responsible) refuses to support the "/latest/" directory now. CDNJS is great and has tried hard to help, but the Cloudflare team won't do the cache purge that would be required to drop a new version in there. The whole industry seems to have moved away from the whole "/latest/" directory thing anyway, so we no longer support it either. We removed mention of that on our site. I really wish we didn't have to stop supporting it because it feels like a broken promise to users, but unfortunately we just don't have any control over that. We have pestered them many, many times. Sorry about the hassle/confusion.

[jamesbrobb]

Hi @jackdoyle thanks for the quick response.

1) Not the target, the position of all the other draggable elements in the list (or rather the one that needs to be repositioned) to reposition them when the user drags the target over them. So similar UI functionality to this (but not this code):

http://codepen.io/anon/pen/vLgXrE

So if i don't call update on the Draggable object associated with the element that gets repositioned/tweened, then the Draggable object's x and y get's out of sync with the elements x and y. So i'm using it for the purpose described in the docs. I think?

2) i'll try and put something together tomorrow. The code's coupled to Angular at the moment, so i need to remove that dependency to simplify it. Which i was trying to avoid :)

---

No problem about the cdn, i can imagine how annoying it is. I use bower myself, but just noticed it when i was tweaking the codepen example above and thought i should mention it.

[jamesbrobb]

I'm afraid i haven't had time to decouple my logic from Angular, but i've simplified it as much as i can, so hopefully it's clear what's going on.

The handlers for each event flow are:

onPress - line 77 onDrag - line 86 onRelease - line 103

And the function where the issue occurs is _positionElements (line 241).

Please let me know if anything needs explaining.

1.18.0 example 1.18.1 example

[jackdoyle]

Thanks so much for the examples. SUPER helpful. And sorry about the delayed response. It took a little while to track down a solid solution that also resolves the original issue that the new code was supposed to fix. I think I've got it nailed now. Feel free to take a peek at a preview of the upcoming release (uncompressed): https://s3-us-west-2.amazonaws.com/s.cdpn.io/16327/Draggable-latest-beta.js

It seems to work great in your demo.

[jamesbrobb]

Hey @jackdoyle, sorry for the delay in getting back to you.

Great work, that looks like it's fixed it!

1.18.2 example

Although bizarrely it's introduced another issue in my actual project that doesn't occur in the example, whereby the further the page scrolls down, the further the element being dragged moves away from the mouse. So the element being dragged moves at a slower speed than the scroll?

I'm wondering if it relates to the layout, as it's more complex than the example and the scrolling container's buried inside several other elements. So feel free to close this and i'll open another issue if i find it relates to GSAP.

Thanks again for your work fixing this, it's much appreciated.

[jackdoyle]

Yeah, I'd be very interested to see an example of the new drifting issue you mentioned. Any chance you could craft a reduced test case in codepen or something? I definitely want to fix any bugs - I just need a little help seeing the issue.

[jamesbrobb]

yeah no problem, i'll see if i can reproduce it in a codepen tomorrow

[rmarscher]

I was going crazy over this one before realizing it was a known issue in Draggable. I had fixed it by checking if self.pointerX and self.pointerY was zero during the render... but it looks like the fix @jackdoyle made was to set checkAutoScrollBounds = self.isDragging; in update. That's cleaner. All works great after the fix. Thanks.

[jackdoyle]

@rmarscher sorry about that :)

So can I go ahead and close this issue now that it has been addressed in 1.18.2? Anyone else seeing a problem here?

[rmarscher]

@jackdoyle no problem. Thanks for the awesome product. The fix is in Draggable 0.14.4 and I don't think that has been shipped in a release yet. My copy of 1.18.2 from npm has Draggable 0.14.3.

[jackdoyle]

Should be all set in the GSAP 1.18.3 push. Thanks again.