-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix parent to child drops #563
Fix parent to child drops #563
Conversation
@alexreardon actually, false alarm! It was a simple fix, the storybook was just accidentally not rendering the So now dragging from a parent to a child (or a child to a parent) works! |
Hi @ianstormtaylor, thanks for putting this forward. It is a really interesting addition! However, at this point I do not think it best to proceed with this PR. Rationale:
You're a legend for raising this. Hopefully we can look into it after #77 lands!! |
Hey @alexreardon thanks for the quick response. But I'm slightly confused... It seems like the changes in #493 don't significantly alter the logic in Similarly, for #77, I'd imagine that would be handled in a way that is continues to be separate from the concern of "which droppable is being targeted", since almost all the existing placeholder animating/sizing logic comes into play after that already, once the destination has already been chosen. Put differently, this change actually has a very small footprint. Previously, if two droppables overlapped, the library would simply take the first one as the target (since it used This is actually (as far as I can tell) the only missing feature (dragging from a parent into a child) that prevents this library from being unopinionated as to whether people are building one of the three major structures for drag-and-drop: lists, boards and trees. I think it would be sad for it to be restricted to only lists and boards over a change as small as this one. When maintaining my own projects, I sometimes put things off that seem like they overlap with other areas that I'm currently working on, but I also often do this accidentally in cases where I probably shouldn't, simply to reduce my mental overhead, without realizing that folks also have other valid use cases that don't infringe on mine much. I feel like that's what may be happening here? I think this change is very small, but it unlocks and entirely new trees use case, and one that many people have requested. Merging it allows those people to start using it for that use case, and allows them to contribute new fixes/ideas that stem from it as they're discovered. It doesn't seem to impact anything in the list/board use cases, and would unlock a lot of people. Would you please reconsider? |
Hey @alexreardon any thoughts on this? I was just thinking about this more, I feel like this is the best way to get folks like myself and others from #134, #301, #456, etc. being able to using this functionality so that we can submit fixes/improvements as we use it. Otherwise it's a Catch 22 of never being able to use it, and then not finding the deeper issues. (I really think there are many fewer issues than we think there will be.) Thank you! |
This lets draggable items be dragged from a parent and dropped into a nested child list, making this library suitable for not just boards but also trees.
This was previously impossible, since the parent list would only ever register itself, but by dropping into the smallest list instead, we can drop into children too.
Right now this change works, in that you can now drag and drop either from a child into a parent, or from a parent into a child.
But there's one thing missing, which is that when dragging between nested lists, when a placeholder is rendered in the non-home list, for some reason it doesn't also increase the list's height to account for the placeholder. You can see this behavior here:
@alexreardon I couldn't for the life of me figure out where this logic was in the codebase. Is there any way this is something you could finish? or point me to where I need to make a change?
Thank you for this great library!
Fixes #456
Fixes #301
Fixes #134