Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for nested scroll containers #131

Open
alexreardon opened this issue Oct 2, 2017 · 223 comments
Open

Support for nested scroll containers #131

alexreardon opened this issue Oct 2, 2017 · 223 comments

Comments

@alexreardon
Copy link
Collaborator

alexreardon commented Oct 2, 2017

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)

  • Grab all of the Droppable elements
  • Take the first one and walk up the DOM tree. If the element is a scroll container then add a data-* attribute to it. (eg data-react-beautiful-dnd-scroll-container=${index}). Cache the element and its result during the collection
  • When the node.parentElement is null then move onto the next Droppable. If an element is found that has previously been investigated then skip the rest of the upwards search. Use any previously found scroll parent ancestry.
  • When visiting an element an we also need to check to see if the element is position:fixed for our position:fixed support

Storage while dragging

  • When storing a Droppable we will keep a map (or linked list) that registers a Droppables scroll containers.
  • A droppable's frame will need to be updated when any of the ancestry changes
  • When calculating droppable displacement internally, all of the ancestry will need to be taken into account

Updates (scroll events)

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

  • If the source of a scroll event is a scroll container that has a data-react-beautiful-dnd-scroll-container attribute then trigger an action to update all relevant Droppables. One redux update for all Droppables (fixes Performances and scrolling issues  #244). The internal algorithms will need to be updated to account for 0 <-> many scroll containers
  • If a scroll is on the window then trigger a window scroll action. Currently, this is handled by the drag handles. Drag handles will no longer handle this
  • If the source of a scroll event is a scroll container that is NOT a data-react-beautiful-dnd-scroll-container then it can be ignored

Auto scrolling

  • There will need to be some investigation into how this will work.
  • Need to investigate how our push scroll holds up when moving with a keyboard

Clean up (drag end)

  • Remove all of the data-react-beautiful-dnd-scroll-container attributes
  • Remove single window event listener

Bonus

  • We could keep the scroll listeners active while a drop animation is occurring and flush any drop animations
@alexreardon
Copy link
Collaborator Author

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

@humphreybc
Copy link

@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
Copy link

goldo commented Jul 4, 2018

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
Copy link

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
Copy link

alxtz commented Jul 24, 2018

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
Copy link

sis commented Jul 29, 2018

@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
Copy link
Collaborator Author

alexreardon commented Aug 3, 2018

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

@goldo
Copy link

goldo commented Aug 3, 2018 via email

@alxtz
Copy link

alxtz commented Aug 7, 2018

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

@goldo
Copy link

goldo commented Aug 7, 2018

@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

This comment has been minimized.

@jebarjonet
Copy link

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
Copy link

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

@IsenrichO
Copy link

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

@dylmye
Copy link

dylmye commented Oct 24, 2018

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

@bmz1
Copy link

bmz1 commented Oct 24, 2018

Hi @alexreardon,

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

@alexreardon
Copy link
Collaborator Author

alexreardon commented Oct 24, 2018 via email

@kole
Copy link

kole commented Nov 19, 2018

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.

@AlexxMart
Copy link

The package @atlaskit/pragmatic-drag-and-drop-react-beautiful-dnd-migration totally worked for me. I found the info I needed here.

Docs

@alexreardon
Copy link
Collaborator Author

@abhay187 the docs for Pragmatic drag and drop will hopefully be public by the end of this year. I also plan on giving consumers of react-beautiful-dnd a large written update at that time as well

@obedparla
Copy link

obedparla commented Oct 23, 2023

I confirm @InstantHuman solution works. I had ovewflow-y: auto set by a Material UI's container and setting it to overflow: initial removed the warning. I tried overflow: auto but that still triggered the warning.

@gabberr
Copy link

gabberr commented Nov 2, 2023

Thank you for the work on the migration atlaskit/pragmatic-drag-and-drop-react-beautiful-dnd-migration.
Does anyone know why it's peerDependencies are different than for react-beautiful-dnd ?

    "react": "^16.8.5 || ^17.0.0 || ^18.0.0",
    "react-dom": "^16.8.5 || ^17.0.0 || ^18.0.0"

Versus migration:

    "react": "^16.8.0",
    "react-dom": "^16.8.0"

We are using react 17 and a newer version of npm (without the legacy peer install), so we cannot install the migration package because of this.

@githorse
Copy link

githorse commented Nov 3, 2023

@InstantHuman et al - could somebody post a sample of this solution? I've tried putting a div (actually, a MUI Box) with overflow: "hidden" (and overflow: "initial") as the immediate parent of both Droppable and of DragDropContext, and I'm still seeing this error.

chrisvxd added a commit to measuredco/puck that referenced this issue Nov 21, 2023
Having overflow: auto on a parent of the Droppable causes the following dnd warning:

> dnd.esm.js:31 @hello-pangea/dndDroppable: unsupported nested scroll container detected.A Droppable can only have one scroll parent (which can be itself)Nested scroll containers are currently not supported.We hope to support nested scroll containers soon: atlassian/react-beautiful-dnd#131

See https://github.com/atlassian/react-beautiful-dnd/issues/131\#issuecomment-1634398431
chrisvxd added a commit to measuredco/puck that referenced this issue Nov 22, 2023
Having overflow: auto on a parent of the Droppable causes the following dnd warning:

> dnd.esm.js:31 @hello-pangea/dndDroppable: unsupported nested scroll container detected.A Droppable can only have one scroll parent (which can be itself)Nested scroll containers are currently not supported.We hope to support nested scroll containers soon: atlassian/react-beautiful-dnd#131

See https://github.com/atlassian/react-beautiful-dnd/issues/131\#issuecomment-1634398431
@ronyfhebrian
Copy link

Can't believe this PR is older than my children

@MRLinhNB
Copy link

any way to fix this issue guys ?

@Abdukhaligov
Copy link

Rolled back to version 10. Works fine for me

@aryanxcodex
Copy link

One thing that i am really curious about is ...
How did the Trello Team workaround this issue ? I mean they use the same library right ?

@subratre
Copy link

subratre commented Feb 1, 2024

@alexreardon could you please share the solutions for the nested scrolling issue? Additionally, you mentioned providing pragmatic drag and drop documentation along with a written update for react-beautiful-dnd?

@davidlygagnon
Copy link

Yes +1 to the comment above! Would love to hear about this:

I also plan on giving consumers of react-beautiful-dnd a large written update at that time as well

@marlibon
Copy link

Dear @alexreardon ,

I visit this place every week hoping to see the long-awaited solution to the issue. I truly hope that you will find the time for this. Thank you for the work you have already done; it is truly impeccable. However, this option does not allow us to fully utilize the boards in swimlane mode.

Thank you.

@marlibon
Copy link

Hello, @alexreardon,

I keep coming back here and really hope that you will take the time to help solve the problem or point me in the right direction for finding a solution.

@muhammedaksam
Copy link

Rick Astley - Never Gonna Give You Up (Official Music Video)

@shamseer-ahammed
Copy link

+1

@gnomefin
Copy link

Greenday - Waiting Official Music Video

@antoniobeltran
Copy link

antoniobeltran commented Jul 19, 2024

@fzn0x
Copy link

fzn0x commented Jul 23, 2024

What's going on here, let me share another song Under Her Spell

@emilyjerger
Copy link

I really want this feature.

@ikrydev
Copy link

ikrydev commented Aug 15, 2024

anyone from 2024 ?

@marlibon
Copy link

friends.
If you're still waiting for a solution, don't wait, try this. The transition is painless, you just need to change the imported library. Why more libraries are needed so that the author react-beautiful-dnd - @alexreardon

The problem with scrolling is solved here.
@atlaskit/pragmatic-drag-and-drop-react-beautiful-dnd-migration

https://atlassian.design/components/pragmatic-drag-and-drop/optional-packages/react-beautiful-dnd-migration/about - info

@rayronvictor
Copy link

friends. If you're still waiting for a solution, don't wait, try this. The transition is painless, you just need to change the imported library. Why more libraries are needed so that the author react-beautiful-dnd - @alexreardon

The problem with scrolling is solved here. @atlaskit/pragmatic-drag-and-drop-react-beautiful-dnd-migration

https://atlassian.design/components/pragmatic-drag-and-drop/optional-packages/react-beautiful-dnd-migration/about - info

I think that this issue should be closed.

@klaytoncavalcante
Copy link

There are also other options from different authors.

The DND Toolkit is a good one: https://dndkit.com/

They have a working example: https://master--5fc05e08a4a65d0021ae0bf2.chromatic.com/?path=/story/presets-sortable-multiple-containers--many-items (zoom in if you don't have scrollbars)

I totally agree, this should be just closed, since even the author moved on. Sincerely, I'm here just to see how many people still have faith that this is going to be fixed.

@LiFaytheGoblin
Copy link

I ended up using React DnD, it went smoothely but costom styling is not so easy.

@sangeetha-armtek
Copy link

We are knee deep into using beautiful dnd it now. We want to implement virtua with dnd.
Does anybody have a solution for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests