search

atlassian / react-beautiful-dnd

Beautiful and accessible drag and drop for lists with React
https://react-beautiful-dnd.netlify.com
Other
32.53k stars 2.5k forks source link

Support for nested scroll containers #131

Open alexreardon opened 6 years ago

alexreardon commented 6 years ago

Adding support for n-level deep scroll containers. Currently, only a single level is supported

Current plan

This plan will allow for nested scroll containers, and also improve the performance of scroll updates

Collection (drag start)

Storage while dragging

Updates (scroll events)

A single scroll listener is added to the window as a capture:true listener. This will capture all scroll events.

Auto scrolling

Clean up (drag end)

Bonus

alexreardon commented 6 years ago

Given the complexity in supporting a single level, I think this is out of scope for now!

humphreybc commented 5 years ago

@alexreardon Does this issue affect Core boards / Jira Software boards? I'm trying to improve our board experience in Dovetail and running into a lot of issues that seem to be caused by this one. I would love to know a little bit about how the Core / Software teams have tackled this.

goldo commented 5 years ago

Hi @alexreardon, We just bumped to 8.0.1 🎉 and are noticing this warning. We have a horizontal scrollable board with vertical columns. It is no problem for us that we can't drag, then horizontal sroll, then drop at the right place, but we would like to clear this console warning, even in development mode. Is that possible ?

Thanks

mtsc-rrapiteanu commented 5 years ago

I am running into this issue too (scrollable columns + horizontal scroll on the board). I think it would be very helpful if you could tackle this in the near future. Thank you!

alxtz commented 5 years ago

Doesn't quite understand this issue, the long lists in a short container seems to work fine

Also, agree to @goldo, spamming too much console.warn() in the codebase isn't healthy

sis commented 5 years ago

@alxtz Nested scroll containers would mean that you can vertically scroll through individual columns whilst having another parent scroll container as right now you can't.

alexreardon commented 5 years ago

@goldo perhaps we could add an option to opt out of all warnings - even in development? 🤔

goldo commented 5 years ago

Hmm might be dangerous wouldnt it ? Id like to know if there is a warning in dev mode, but if i understand it and I accept it, I would like to get rid of it. Maybe disabling only this warning would be best ? On 3 Aug 2018 at 07:05 +0200, Alex Reardon notifications@github.com, wrote:

@goldo perhaps we could add an option to opt out of all warnings? 🤔 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

alxtz commented 5 years ago

@goldo would making the disableWarning option to be true by default solve your concern?

goldo commented 5 years ago

@alxtz like I said, I think warnings are useful, specially in case of bumps, so I would prefer not to remove all the warnings. In this case, we are totally OK with this issue, it's not a problem at all, in our application. That's why we would like to remove only this specific warning, and keep all the others. If it's too complex to do, I think we will keep all the warnings, just in case of problems (at least in dev mode)

FEliuyg commented 5 years ago

when to support dragging items from the parent list into a child list?

jebarjonet commented 5 years ago

This issue is a real problem for Kanban/Trello like apps (a big use case for D&D apps I guess?). The warning is very nice, but it is pretty unclear before making an app, you realize it when it is too late. I hope this will be fixed in the near future though 🙏

wesleywong commented 5 years ago

an option to opt out of all warnings will be good. Too much warning while the functionality work as per normal.

IsenrichO commented 5 years ago

@jebarjonet brings up a great point. I'd echo others' points that the option to opt out would be highly appreciated!

dylmye commented 5 years ago

Would it be okay for me to make a PR implementing this opt-out functionality? Should I open a new issue for it?

bmz1 commented 5 years ago

Hi @alexreardon,

are you planning to implement this feature in the near future?

alexreardon commented 5 years ago

We have a dev warning opt out coming in v10 On Thu, 25 Oct 2018 at 1:49 am, Boér Máté notifications@github.com wrote:

Hi @alexreardon https://github.com/alexreardon,

are you planning to implement this feature in the near future?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/atlassian/react-beautiful-dnd/issues/131#issuecomment-432689102, or mute the thread https://github.com/notifications/unsubscribe-auth/ACFN7RipwmL8IBPWNBqOOuCr8kUSykoeks5uoH3dgaJpZM4PrKeZ .

kole commented 5 years ago

Just to clarify the core of this issue (not the dev warning complaints)... it is currently impossible to create a Trello clone (for example) with this library because you cannot have scrolling columns and a horizontally scrolling board container.

alexreardon commented 5 years ago

We will be starting work on this feature soon On Tue, 20 Nov 2018 at 3:37 am, Nick Johnson notifications@github.com wrote:

Just to clarify the core of this issue (not the dev warning complaints)... it is currently impossible to create a Trello clone (for example) with this library because you cannot have scrolling columns and a horizontally scrolling board container.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/atlassian/react-beautiful-dnd/issues/131#issuecomment-439957615, or mute the thread https://github.com/notifications/unsubscribe-auth/ACFN7cJPPXA1D5XdUzin9FJpX-PeyvMJks5uwt5OgaJpZM4PrKeZ .

kole commented 5 years ago

@alexreardon That's great news! Do you anticipate this being a v11 improvement because of the complexity or might this fit into a v10 minor release? Just trying to get a very loose timeline picture. Thx

alexreardon commented 5 years ago

It will be a big piece of work, but it will probably be a minor sem ver release On Tue, 20 Nov 2018 at 5:28 am, Nick Johnson notifications@github.com wrote:

@alexreardon https://github.com/alexreardon That's great news! Do you anticipate this being a v11 improvement because of the complexity or might this fit into a v10 minor release? Just trying to get a very loose timeline picture. Thx

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/atlassian/react-beautiful-dnd/issues/131#issuecomment-439994317, or mute the thread https://github.com/notifications/unsubscribe-auth/ACFN7dGNe0RDDArVCTkun2z2j1wCzkGHks5uwvhYgaJpZM4PrKeZ .

sunran357 commented 5 years ago

@alexreardon scrollable columns and horizontal scroll on the board

daviddworsky commented 5 years ago

Correct me if I'm wrong, but this example from the react-beautiful-dnd example page has nested scroll containers does it not?

How does this example work without giving the error message "Droppable: unsupported nested scroll container detected" ?

alexreardon commented 5 years ago

I am starting to think about algorithms for solving this problem

Collection (drag start)

At the end of the collection, a Droppable will have 0 <-> many associated scroll container indexes which match the indexes applied to data-react-beautiful-dnd-scroll-container=${index}.

Updates (scroll events)

A single scroll listener is added to the window as a capture:true listener. This will capture all scroll events.

Clean up (drag end)

sunran357 commented 5 years ago

Hi，How long will it take to get online?

alexreardon commented 5 years ago

@daviddworsky the example is mounted in an iframe, so it only has one scroll container inside of a window

alexreardon commented 5 years ago

@sunran357 not sure. You can follow here for updates: https://github.com/atlassian/react-beautiful-dnd/milestone/2

TrySound commented 5 years ago

@alexreardon Awesome! Do you know which browsers support window scroll event with capture?

alexreardon commented 5 years ago

All it looks like On Mon, 3 Dec 2018 at 10:42 pm, Bogdan Chadkin notifications@github.com wrote:

@alexreardon https://github.com/alexreardon Awesome! Do you know which browsers supports window scroll event with capture?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/atlassian/react-beautiful-dnd/issues/131#issuecomment-443681901, or mute the thread https://github.com/notifications/unsubscribe-auth/ACFN7d8qjsx6glb_lFYAmEw8Q6a3LFB5ks5u1Q4hgaJpZM4PrKeZ .

Teippo2020 commented 5 years ago

Any updates on when this will be done and released?

sunran357 commented 5 years ago

Hi, when can this Issues be used? We have this problem.

jseminck commented 5 years ago

Hello! I also ran into this warning when trying to make a board with horiztonal scroll and vertical scroll per column. However, all of the functionality seems to be working, despite the warning?

Here is my Proof-of-concept: https://73v6rvzml0.codesandbox.io/ - this is based on the example from the README. I didn't bother to fix the onDrop logic. But the general drag-and-drop works fine.

Am I missing something? Or can I just ignore this warning?

nanopx commented 5 years ago

@jseminck Hi, it looks like your example is using the window as the parent scroll container. Maybe this example does the same? (note that the example is in an iframe)

What we want here is to use another container (like a div) instead of the browser's window, which is not supported now

jseminck commented 5 years ago

Hello @nanopx, thanks for your answer!

That is correct. Actually, we also do not want it to be on the window. We need a header that needs to remain in place when scrolling horizontally on the board.

I created another example with the scroll on another container. This seems to work fine, except the scroll does not happen if you want to move an item from the first column to the last column. 😞 But the scrolling does work within a column... 🤔

Actually, for us, the horizontal movements are more important than the vertical movements. So if I could figure out a way to do automatic horizontal scrolling when dragging left/right instead of vertical automatic scrolling, then it would be fine.

https://73v6rvzml0.codesandbox.io/

nanopx commented 5 years ago

@jseminck same here, I've also tried to scroll the parent container outside of react-beautiful-dnd, but the card's placeholder gets out of position during a horizontal drag... 😢

So if I could figure out a way to do automatic horizontal scrolling when dragging left/right instead of vertical automatic scrolling, then it would be fine.

I don't think this is possible at this moment 🤔 (sorry if I'm wrong...

If you don't need a scroll in each list, maybe this example could help?

jseminck commented 5 years ago

We do need vertical scroll in the columns, but we do not care for vertical automatic scroll when a Draggable element is dragged towards the top or bottom of the list.

I will dig into the code and see if I can reverse the detection algorithm for scroll containers. Currently, I think it first checks inside the Droppable element and then it checks outside of the Droppable elements. If this can be reversed, maybe my issue can be fixed.

Additionally, I'm happy to help to solve this problem so both scroll containers would work. If there's any development already done I do not mind to take a look and continue working on it. It definitely sounds like an interesting issue, perhaps a bit outside my comfort zone, but with some guidance, I'd definitely like to take a shot at it! 💪

alexreardon commented 5 years ago

We will start work on this soon. It is a sizeable piece of work. I am keen to see this take flight!

jseminck commented 5 years ago

Thank you @alexreardon, I really appreciate the work on this library, the egghead.io course and so on!

In the meantime, I would like to try to reverse the order in which the scroll containers are detected to solve my issue: currently automatic scrolling is working vertically but not horizontally. I would prefer it the other way around. Example: https://73v6rvzml0.codesandbox.io/

Do you think it would work if I switch the order in which scroll containers are detected by first looking outside of the Draggable?

I understand currently the library doesn't support this and probably it's not something that you want. But if this works it might be a good short-term solution for me through a fork.

A quick scan through the code and seems these are the files that would need adjusting:

https://github.com/atlassian/react-beautiful-dnd/blob/master/src/view/droppable-dimension-publisher/get-closest-scrollable.js#L70-L71 https://github.com/atlassian/react-beautiful-dnd/blob/master/src/view/droppable-dimension-publisher/check-for-nested-scroll-container.js#L7-L8

Edit: Just to confirm, this approach worked!

Flax95 commented 5 years ago

Awesome work @alexreardon! Loving the library. Are there any updates on this? Did work on this awesome improvement to this masterpiece take flight?

cleverbeagle commented 5 years ago

For folks trying to achieve a Trello-style kanban, I was able to hack this together utilizing the onDragUpdate callback:

  handleOnDragUpdate = (update) => {
    const { draggableId, source, destination } = update;
    const sourceId = _.get(source, 'droppableId', null);
    const destinationId = _.get(destination, 'droppableId', null);

    if (sourceId !== destinationId) {
      const versionsContainer = document.getElementById('versions');
      const draggingItem = document.getElementById(draggableId);
      if (draggingItem) {
        const { x } = draggingItem.getBoundingClientRect();
        versionsContainer.scroll({
          top: 0,
          left: x,
          behavior: 'smooth',
        });
      }
    }
  };

Here, update is the value passed from onDragUpdate. That tells you what's moving and where it's trying to go. The sourceId and destinationId are me saying "if the these exist (source always should), use them." I use the lodash _.get() method as a safety mechanism instead of destination.destinationId, etc. which could fail if a destination isn't available.

Next, if those values exist and they're not equal (meaning, we're moving to another list), I get the scrollable container element (in my case, a div with an id of versions which wraps all of my kanban lists) and then I get the card being dragged in the list as draggingItem.

If I find a draggingItem, I grab its getBoundingClientRect() position which roughly tells me where the card is at in the dom (which conveniently updates as onDragUpdate is called).

From there, I just use the native .scroll() method on the scroll container (#versions) to augment the horizontal (x) position of the list.

Notes:

humphreybc commented 5 years ago

@cleverbeagle Nice one! I gave this a go, and while the window does scroll horizontally, the items are still not dropped in the correct list when you let go of the cursor.

cleverbeagle commented 5 years ago

@humphreybc unfortunately, yeah, found the same once I got a bit deeper into solving this. Had the wind in my hair when I posted this 😜

FEliuyg commented 5 years ago

Are there any updates on this?

sunran357 commented 5 years ago

hi sorry Are there any updates? How long it will take
I need to use it

alexreardon commented 5 years ago

I have updated the description with the current plan: https://github.com/atlassian/react-beautiful-dnd/issues/131#issue-262207001

ghost commented 4 years ago

Can appreciate the complexity of this feature. If it helps any, I suspect supporting the nesting of 2 scrolls would be sufficient for most use cases, for example, an outer horizontal scroll and an inner vertical scroll.

alexreardon commented 4 years ago

I have an active plan for how to solve this. My focus right now is on landing virtual lists and then i hope to action this

On Fri, May 24, 2019 at 6:16 AM leantide notifications@github.com wrote:

Can appreciate the complexity of this feature. If it helps any, I suspect supporting the nesting of 2 scrolls would be sufficient for most use cases, for example, an outer horizontal scroll and an inner vertical scroll.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/atlassian/react-beautiful-dnd/issues/131?email_source=notifications&email_token=AAQU33P63BGIAXQTV3OR7YLPW33TFA5CNFSM4D5MU6M2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWDLWTI#issuecomment-495369037, or mute the thread https://github.com/notifications/unsubscribe-auth/AAQU33O4LE3JOZHGENGH7ITPW33TFANCNFSM4D5MU6MQ .

NickEmpetvee commented 4 years ago

Hi guys. I embedded a react-beautiful-dnd two column component in a material-ui dialog. It seems to work fine. However it gave me a message about unsupported nested scroll container detected and pointed me to this issue. Is the message a cause for concern and is my approach not recommended, or is the message meant for a similar but separate case? I'm only looking to drag and drop in the DragAndDropContext container, and not in the dialog outside of it.

Thanks, Nick

nicubarbaros commented 4 years ago

@NickEmpetvee seems that you have 2 containers that are scrollable.

NickEmpetvee commented 4 years ago

@nicubarbaros Right, that's probably the material-ui dialog which houses the two-column react-beautiful-dnd component. It seems to work fine in allowing draggables to be moved from one column to another.

I'm just trying to figure out if there's a potential other issue I need to be aware of because of the message that I didn't catch in my basic drag-drop testing.