Add scrolling support vertically and horizontally #449

nguyenj · 2016-12-10T04:13:02Z

This PR will be addressing these drag and scrolling issues: #327, #121.

TODOS:

Add support scrolling containers
Add support for horizontal scrolling
Allow auto scrolling at scroll edge
Add touch support

steel · 2016-12-12T18:28:48Z

This PR is just full of awesome!

TODO: When hovering the dragged item at scrollEdge, the scrolling should automatically begin to scroll. Drag and scrolling code is inspired by jQuery UI's sortable and draggable widgets.

joshsmith · 2016-12-13T06:07:22Z

This looks fantastic.

What needs to happen for this to be mergeable?

joshsmith · 2016-12-13T06:08:22Z

dragula.js

+    // For iframe. When dragging an item and mouse moves out of the iframe and
+    // mouseup, then decides to move back, the event will be 0 so we should
+    // just call cancel.
+    if (whichMouseButton(e) === 0 { cancel(); }


@nguyenj this has a syntax error.

Should be:

if (whichMouseButton(e) === 0) { cancel(); }

I'll have this fixed. Thanks for the catch!

nguyenj · 2016-12-13T13:42:07Z

I think it's pretty much ready.

joshsmith · 2016-12-13T19:53:50Z

@nguyenj more asking the maintainers.

@bevacqua can you or someone else take a look at this when you have a free moment? Thanks!

Happy to help triage, as well.

purtuga · 2016-12-23T16:18:15Z

+++1 for this...

bevacqua

This looks nice, but has me worried about jank. Can you replace setInterval with requestAnimationFrame?

We can use this module: raf

bevacqua · 2016-12-23T16:42:48Z

dragula.js

+  if (node === null) { return null; }
+  if (node.scrollHeight > node.clientHeight) {
+    return node;
+  } else {


else here is redundant, please remove

bevacqua · 2016-12-23T16:42:52Z

dragula.js

+  } else {
+    if (!/(body|html)/i.test(node.parentNode.tagName)) {
+      return getScrollContainer(node.parentNode);
+    } else {


else here is redundant, please remove

bevacqua · 2016-12-23T16:44:02Z

dragula.js

+  if (node.scrollHeight > node.clientHeight) {
+    return node;
+  } else {
+    if (!/(body|html)/i.test(node.parentNode.tagName)) {


Extract regexp to a variable

I don't understand why I need a variable for this. Why would you want to hold on to the reference if it's not going to be used unless we're scrolling?

I created a commit for it, e9e4e5a; but I'm not sure if I'm happy with this path. I'm open to suggestions.

@nguyenj I just meant putting it in a variable like this:

var rbodyhtml = /(body|html)/i if (!rbodyhtml.test(node.parentNode.tagName)) { // ...

I avoid inline regular expressions in conditionals because they dampen readability. There's no need to hoist it to the top of the file, certainly.

purtuga · 2016-12-23T17:01:12Z

Just wanted to pass along some info...
I just pulled this fork into a Kanban board application I have and I'm not getting expected results... Does this fix support only scrolling when it reaches the edge of the browser? or also of the containers, which themselves could have fixed height with scrolling bars?
The issue (for me) seems to be around the getScrollContainer function... it seem to be returning the node provided on input every time... To give you an idea of my use case, I am working with the following screen:

Looks like for my use case, where I want each container to be able to scroll vertically when I move the card over it, I will likely need to write something custom... Hopefully dragula provides me with enough events to be able to do it without having to fork the library.

/Paul

nguyenj · 2016-12-23T21:03:21Z

@purtuga can you provide me a jsbin example? I have an example setup on the index.html for scrolling within a container. It would be easier to debug your issue if I'm able to see the markup and css.

purtuga · 2016-12-23T21:09:13Z

Hi @nguyenj, thanks for responding...
I don't have a public version of this app, but let me see if can setup something so you can take a look...

bevacqua · 2016-12-23T23:16:55Z

dragula.js

@@ -6,6 +6,7 @@ var classes = require('./classes');
 var doc = document;
 var documentElement = doc.documentElement;
 var _autoScrollingInterval; // reference to auto scrolling
+var BODY_HTML_TAG_REGEX = new RegExp(/(body|html)/, 'i');


You can just keep it as a literal /(body|html)/i

bevacqua · 2016-12-23T23:21:03Z

@nguyenj Could use use requestAnimationFrame / cancel via raf? This way we can support older browsers too 👍

purtuga · 2016-12-24T15:58:47Z

@nguyenj I setup a quick prototype here: http://jsbin.com/mahuwitani
I included the latest code from your fork's dist folder... I do see the containers scrolling as you drag a container child close to the edge of them, but am also seeing some weird behavior on multiple containers... Example: if you pickup a item and drag it across multiple containers, but close to the bottom edge of each container, they start to scroll by themselves... sometimes not stopping..

This prototype is also setup where the body of the window does not have a horizontal scroll, but rather, the element around the containers does... I don't believe your push is meant to address this use case (custom viewport elements), but wanted to provide you with a sample...

Thanks for your time on this and for contributing back.

belovedllj · 2017-01-06T19:44:27Z

looking forward to this PR can be merged into Dragula ASAP

nguyenj · 2017-01-06T22:04:26Z

@purtuga That's interesting; thanks for pointing that out. I'm not sure if I'll be able to address your particular issue with dragging in multi-directions. I suspect the issue to be that, it's only looking for the closest container "type" to that item and checking to see if it's scrollable-that's probably why it's not scrolling horizontal because the parent to the item is vertically scrollable.

purtuga · 2017-01-07T00:13:06Z

@nguyenj Thanks for the reply.
Yes, I figure my need would be a little different - basically dragula would have to support an input option that told it what the "scrolling view port" is... Your changes, I hope, will help me in my use case, since they will handle the scrolling of each individual droppable area... I think that I can then get my other need working by listening to dragula events and scrolling the horizontal viewing container myself.

gerbermichi · 2017-01-11T09:21:24Z

Hi guys, when do you add this PR into dragula? I am looking foreward to use this awesome feature :)

nguyenj · 2017-01-19T15:02:02Z

@bevacqua Let me know if there is anything else–I pushed up a simple raf polyfill.

purtuga · 2017-01-28T14:00:28Z

@nguyenj
I was able to fix my use case: scroll the area horizontally (a fixed-width DIV) when user drags card close the left/right edge using the drag and dragend events... Now I'm looking forward to @bevacqua merging your change in, which will complete my UX goal for what I'm working with...

nguyenj · 2017-04-04T22:50:56Z

@gxzhou519 your issues should be fixed.

Thanks @adam-h!

nguyenj · 2017-04-06T02:31:46Z

Tests are passing locally; but not entirely sure why CI is failing.

joshsmith · 2017-04-07T17:02:32Z

@nguyenj are you running the tests locally with the same configuration options as on Travis?

nguyenj · 2017-04-09T17:55:40Z

@joshsmith The only thing that is the same that I am running from .travis.yml file is node 4, everything else is on a mac.

jasonruesch · 2017-04-19T19:07:50Z

When will this be getting merged?

Also simplify animationFrame polyfills as browser support is excellent and older browsers are buggy (some have no support for cancelling) so it's better to fallback.

jasonruesch · 2017-04-25T19:09:04Z

I see that there was some activity. Is there an ETA on this being merged?

nguyenj · 2017-04-25T23:53:05Z

Good question! I have no idea.

replete · 2017-06-05T08:55:24Z

Wow, what do we need to do to get this merged? :O

ESSENTIAL for this great piece of software

shennan · 2017-06-22T09:50:03Z

@nguyenj Does this cover the case where the elements that you want to scroll are not always Dragula containers? For example, you have a Kanban style board, which has overflowed the viewport, and you want to scroll the viewport when you approach the top/bottom edges such that you can place cards in different vertices? Essentially, initiating a page scroll on an entire page instead of just the containers.

pmnevill · 2017-09-13T20:16:13Z

Any ETA?

gardinit · 2017-09-30T18:39:15Z

Awesome. I tried out these change though and couldn't get it to work when i wanted to scroll to a container that wasn't currently in view. I made some modifications, but I'm unsure if this would make the package better or if this a use case specific to me.

Essentially I'm checking the need to scroll when looking to see if I'm over a target that is dropable, and removing the mouseEvents from the startScroll function and grabing them from the params passed into findDropTarget.

function findDropTarget (elementBehindCursor, clientX, clientY) {
   startScroll(_item, clientX , clientY, o);
   var target = elementBehindCursor;
   while (target && !accepted()) {
      target = getParent(target);
     }
   return target;
 ...

 function startScroll(item, pageX, pageY, options) {
    var scrollContainer = getScrollContainer(item);
    caf(_autoScrollingInterval);
    / / If a container contains the list that is scrollable
    if (scrollContainer) {
      var scrollingElement = null;
      var scrollEdge = options.scrollEdge;
      var scrollSpeed = 20;
      // Scrolling vertically
      if (pageY - getOffset(scrollContainer).top < scrollEdge)
          ...

nguyenj · 2017-10-25T16:20:45Z

@gardinit sorry I’ve lost interest in pursuing to modify this PR as it seems that this library is in active and the author seems to have stopped maintaining this project-just take a look at all the unresolved issues and open PRs.

Merge remote-tracking branch 'scrollfork2/fix/scroll' into scroll # Conflicts: # dist/dragula.js # dist/dragula.min.js

pete-hotchkiss · 2018-04-25T12:30:38Z

I'd say that sadly this library should be considered dead. Given the lack of response form the author to PRs and issues he's moved his focus else where.

So those looking for a similar solution (with working scroll when dragging no less) would be well served with draggable

https://github.com/Shopify/draggable

lukasjuhas · 2018-04-25T16:44:13Z

That's indeed what I've switched to @pete-hotchkiss
It works great!

Lukas

jeffal · 2018-04-25T17:58:55Z

I have switched to https://github.com/clauderic/react-sortable-hoc with great success!

danielehrhardt · 2018-05-24T15:48:09Z

Is there a fork with this PR?

benelliott · 2018-05-29T13:55:22Z

@danielehrhardt You can see if https://github.com/benelliott/dragula works for you

nguyenj · 2018-05-29T18:37:14Z

@danielehrhardt have you looked into https://github.com/Shopify/draggable? The community there is a little more vibrant.

nguyenj · 2018-06-18T20:27:54Z

Closing this as this project is DEAD.

lukasjuhas · 2018-06-19T09:50:27Z

⚰️

mdhtr · 2018-10-17T14:15:12Z

For those that are stuck with dragula: try dom-autoscroller.
It's this easy to set it up to work with dragula (example taken from the link above):

autoScroll([
        document.querySelector('#list-container'),
        document.querySelector('#container2')
    ],{
        margin: 20,
        maxSpeed: 5,
        scrollWhenOutside: true,
        autoScroll: function(){
            //Only scroll when the pointer is down, and there is a child being dragged.
            return this.down && drake.dragging;
        }
    }
);

This was referenced Dec 10, 2016

Allow ability to scroll screen on drag #327

Open

Scroll while dragging #121

Closed

scroll-while-dragging #129

Closed

nguyenj added 4 commits December 12, 2016 12:55

Add scrolling support vertically and horizontally

4ac8b49

TODO: When hovering the dragged item at scrollEdge, the scrolling should automatically begin to scroll. Drag and scrolling code is inspired by jQuery UI's sortable and draggable widgets.

Add auto scrolling support

030323a

Add drag + scrolling example to the index

a4438f4

Cleanup getScrollContainer function

624e638

joshsmith suggested changes Dec 13, 2016

View reviewed changes

Execute cancel for iframe when dragging in and out of iframe

ec1ac5e

bevacqua requested changes Dec 23, 2016

View reviewed changes

Replace setInterval to requestAnimationFrame

3660289

bevacqua reviewed Dec 23, 2016

View reviewed changes

nguyenj added 2 commits January 19, 2017 06:56

Create var for body/html regex

13875c0

Add simple requestAnimationFrame polyfill

f6cb5cd

Add notes

1e319d5

Add scrollEdge as an options

f6a7d55

Add cancelAnimationFrame polyfill (#1)

dd20e3c

Also simplify animationFrame polyfills as browser support is excellent and older browsers are buggy (some have no support for cancelling) so it's better to fallback.

benelliott pushed a commit to benelliott/dragula that referenced this pull request Nov 13, 2017

Add scrolling from bevacqua#449 (with modifications)

37612f4

Merge remote-tracking branch 'scrollfork2/fix/scroll' into scroll # Conflicts: # dist/dragula.js # dist/dragula.min.js

nguyenj closed this Jun 18, 2018

nguyenj deleted the fix/scroll branch June 18, 2018 20:27

chris-erxnieves mentioned this pull request Aug 8, 2018

feat: Added scrolling support bullhorn/dragula#2

Closed

BrockReece mentioned this pull request Jan 30, 2019

cant scroll while dragging BrockReece/vue-kanban#9

Open

mdlama mentioned this pull request Nov 5, 2019

Implement scroll while dragging serenity-r/dndselectr#4

Open

Add scrolling support vertically and horizontally #449

Add scrolling support vertically and horizontally #449

Conversation

nguyenj commented Dec 10, 2016 • edited Loading

steel commented Dec 12, 2016

joshsmith commented Dec 13, 2016

joshsmith Dec 13, 2016

Choose a reason for hiding this comment

nguyenj Dec 13, 2016 • edited Loading

Choose a reason for hiding this comment

nguyenj commented Dec 13, 2016

joshsmith commented Dec 13, 2016

purtuga commented Dec 23, 2016

bevacqua left a comment

Choose a reason for hiding this comment

bevacqua Dec 23, 2016

Choose a reason for hiding this comment

bevacqua Dec 23, 2016

Choose a reason for hiding this comment

bevacqua Dec 23, 2016

Choose a reason for hiding this comment

nguyenj Dec 23, 2016

Choose a reason for hiding this comment

nguyenj Dec 23, 2016

Choose a reason for hiding this comment

bevacqua Dec 23, 2016 • edited Loading

Choose a reason for hiding this comment

purtuga commented Dec 23, 2016

nguyenj commented Dec 23, 2016

purtuga commented Dec 23, 2016

bevacqua Dec 23, 2016

Choose a reason for hiding this comment

bevacqua commented Dec 23, 2016 • edited Loading

purtuga commented Dec 24, 2016

belovedllj commented Jan 6, 2017

nguyenj commented Jan 6, 2017

purtuga commented Jan 7, 2017

gerbermichi commented Jan 11, 2017

nguyenj commented Jan 19, 2017

purtuga commented Jan 28, 2017

nguyenj commented Apr 4, 2017

nguyenj commented Apr 6, 2017 • edited Loading

joshsmith commented Apr 7, 2017

nguyenj commented Apr 9, 2017

jasonruesch commented Apr 19, 2017

jasonruesch commented Apr 25, 2017

nguyenj commented Apr 25, 2017

replete commented Jun 5, 2017

shennan commented Jun 22, 2017

pmnevill commented Sep 13, 2017

gardinit commented Sep 30, 2017

nguyenj commented Oct 25, 2017

pete-hotchkiss commented Apr 25, 2018

lukasjuhas commented Apr 25, 2018 • edited Loading

jeffal commented Apr 25, 2018

danielehrhardt commented May 24, 2018

benelliott commented May 29, 2018

nguyenj commented May 29, 2018

nguyenj commented Jun 18, 2018

lukasjuhas commented Jun 19, 2018

mdhtr commented Oct 17, 2018 • edited Loading

nguyenj commented Dec 10, 2016 •

edited

Loading

nguyenj Dec 13, 2016 •

edited

Loading

bevacqua Dec 23, 2016 •

edited

Loading

bevacqua commented Dec 23, 2016 •

edited

Loading

nguyenj commented Apr 6, 2017 •

edited

Loading

lukasjuhas commented Apr 25, 2018 •

edited

Loading

mdhtr commented Oct 17, 2018 •

edited

Loading