I really like the great library that you guys have created. Our team is currently considering the option to start using react-beautiful-dnd instead of react-sortable-hoc we currently use.
However, it looks like there is a blocker for adopting your library for us due to inability to pass state/callback from Draggable into the onDragEnd of the DragDropContext. I hope I just missed the option in the documentation, but I did make a thorough search before opening the issue.
Your documentation recommends wrapping whole application with a DragDropContenxt. For most real-life production applications it means that DragDropContext will be multiple levels of hierarchy away form the Draggables. Even if multiple DragDropContexts are used, it is still very likely that the DragDropContext (probably with Droppable) will be not in the same file/component as the Draggables.
It is definitely not a big issue to create something like a top-level reducer inside of the onDragEnd that first filters results based on type, then applies corresponding logic, getting needed data from the Redux store. And this solution will work in some cases.
However, there are several drawbacks and even blockers with this approach:
It is extra work to get the Redux store data and process it in the corresponding onDragDrop reducer. This work was already probably conducted at the component level - Draggable or Droppable. And simply passing it would be much easier than trying to do it again.
Much bigger problem is that not all Component state lives in Redux store. This is a quite common and even recommened (e.g. https://github.com/reactjs/redux/issues/1287 ) behavior. This means that onDragEnd currently simply does not have a way to get needed state, unless substantial refactoring of existing code is done to put all needed state in a Redux store (and even then it might be not convenient).
Other libraries do not seem to have same limitation, as both offer a callback on the Draggable level to run after dragging ends.
E.g. React DnD offers endDrag for the DragSource. React-sortable-hoc has onSortEnd for the SortableComponent.
Your library looks superior to the above mentioned libraries in many aspects, but current limitation of not being able to define/pass a onDragEnd callback or at least some state from the Draggable is unfortunately greatly undermining its usability, especially if we are talking not about greenfield projects, but ones already developed and have lots of (presentational) state on the components level.
I hope I just missed some piece of the documentation and you can direct me where to have a look.
I did find something that seems related (but still does not seem to answer my question):
https://github.com/atlassian/react-beautiful-dnd/issues/120
https://github.com/atlassian/react-beautiful-dnd/issues/466
It would be really great if there actually was some way to pass a callback from Draggable. I would appreciate if you could help me find out how to do this.
Thanks.
Thank you for the thought-provoking issue. The core issue is that there is no clear recommendation for rolling up lots of lists into a single onDragEnd. It is difficult to make a recommendation as this library has no opinions about how you manage your state. Perhaps some additional hooks are a possible approach. There may even be some sort of nesting context approach that could work.
We have aimed to keep the api as light as possible. The understanding is that if we return the id of the things that have been dragging then a consumer should be able to look up whatever data they need from that id. If that data is unavailable then that is a separate issue that needs to be managed by your implementation.
A 'hack' to pass data now is to json encode your data as your id. Then you will get a json string as the id in your onDragEnd that you can parse. That is a hack, but it might be enough for you for now.
I will no open up the api to allow direct passing of data until we explore this problem more
Thank you for your prompt reply to the issue and offering the ‘hack’.
I’ll discuss it with my team to see if they will be willing to adopt this approach, as it is still quite a bit less convenient compared what the currently library we are using is offering.
I really like that you are trying to keep the api of the library as light as possible. And it is great that as you say it “this library has no opinions about how you manage your state”.
My understanding that the core issue is that while react-beautiful-dnd does not have opinions about how state is managed, it is based on an assumption that all needed state is easily available to the onDragEnd either via a (redux) store, or via its placement in the same file as Draggable.
As I indicated above, it is often not the case, as it is common in the React/Redux world to have some state inside the components, not Redux. And big code-bases are very unlikely to have DragDropContext and Draggable in the same file.
Additional hooks/callbacks seem to be a good approach to solve the problem.
Anyway, thanks again for putting so much effort in this great library. I think that resolving the issue will help a much wider adoption of the library.
I prefer to add functions inside the ID, rather than using json encoding.
In my case, I add source-fn and destination-fn to handle what is happening to the source droppable and destination droppable. Those functions are then extracted and called by the on-drag-end callback.
Those functions capture a reference to the state at level of the droppable component.
This means that the DragDropContext is completely independent from the droppable components.
Interesting @Frozenlock. Does that work with our current setup?
It should work. (I'm still on version 6.)
The only caveat is that I'm using Clojurescript.
The principle should be the same however : store functions inside IDs and call those functions with the on-drag-end callback.
Very interesting. We currently say it needs to be a string, but I do not think we do any string operations on the values. As long as referential equality is maintained it could work. This gives some great food for thought. Thanks @Frozenlock !
I need to work through whether that would enable people to get done what they want to. Using a function is a brilliant idea
Playing a bit around (with multiple droppables this time), here's what seems to work inside the droppable ID:
Maps do not work.
Maps will be passed to the on-drag-end callback as expected, but things break when multiple droppables exist. (The equality test must break with maps...)
Here's a cool hack however :
An array with identifiers AND a map.
The identifiers will insure multiple droppables are not considered the same, and all sorts of goodies can be stored into the map to extract in the on-drag-end.
Thanks for looking into this @Frozenlock. Glad to hear there is an option for people to use today. I think it would be a good idea to formalise this and provide a better first class api - as simple as an extra prop on a draggable that is passed on all the hooks which could contain anything - data or a function.
For now it looks like users can hack the id to achieve this. Keep in mind though that this might have unexpected bugs as we assume the id is a string. Also, the id needs to be a unique identifier for the item so you will need to ensure that referential equality is never the same for multiple draggables
Adding a prop with a name like:
data (clash with DOM data-*)metaparamsassociatedextrato pass around to the hooks seems like a good idea.
Not sure what to name the prop though
extra or params convey the intention pretty well I think.
This field would be available at the top level (with the draggableId), and also inside the source and destination fields, allowing the user 3 different entry points for extra data. Is that it?
Details to be confirmed, but something like that.
Upgraded to 7.x.x, doesn't work anymore. :cry:
Warning: Failed child context type: Invalid child context
private-react-beautiful-dnd-key-do-not-use-droppable-idof typearraysupplied toDroppable, expectedstring.
Hopefully the new field will make its way in the next version.
Thank you guys for the ideas you shared on this subject. Based on it, I decided to pass stringified objects. The onDragEnd function arguments structure the information by draggable and droppables for source and destination, so, I followed the same pattern. The example below shows how I am using it:
The droppableId property of my <Droppable> source looks like this:
droppableId={JSON.stringify({
id:'droppable_people',
table: 'people'
})}
The droppableId property of one of my <Droppable> destination looks like this:
droppableId={JSON.stringify({
id:team.id,
table: 'team'
})}
Passing the objects (stringified in this case) is useful to manage my hierarchies since I have different kinds of draggables, for example, the draggableId of my <Draggable> looks like this:
draggableId={JSON.stringify({
id:people.id,
table: 'people'
})}
or like this:
draggableId={JSON.stringify({
id:task.id,
table: 'task'
})}
so on my onDragEnd function handler of my <DragDropContext> I parse the objects:
const params = {
droppable: {
source: JSON.parse(source.droppableId),
destination: JSON.parse(destination.droppableId)
},
draggable: JSON.parse(result.draggableId)
};
and use it as needed, for example, on those conditions:
if(params.droppable.source.id === 'droppable_people' &&
params.droppable.destination.table === 'team' )
and
if(params.droppable.source.table === params.droppable.destination.table)
I think we're going about this the wrong way. It's good that "this library has no opinions about how you manage your state" but I'm finding that, in most cases, it's in the context of Droppable that I know what I want to accomplish, not at a much higher context of DragDropContext. It's too detached.
The current suggestions are to pass up state from the Draggable to the DragDropContext. But the fact that you're mapping everything back to the Droppables seems to indicate that the logic is far from the relevant area of responsibility. With enough types and Droppables and the fact that all of that gets handled in onDragEnd, you're bound to have complex logic even though you've most likely pulled the same state at the relevant Droppables.
Here's my suggestion. We leave onDragStart, onDragUpdate, and onDragEnd for global UX concerns like blocking updates on drags and backwards compatibility. Droppables get a onDroppedOnto and some kind of "leave" hook (onDroppedFrom?). They'd receive what onDragEnd receives now, more or less.
I think this solves most use cases. onDragEnd doesn't have to get complicated because the Droppables will receive the events that matter. Since the Droppable most likely handles retrieving, removing, persisting, sorting, etc from state, it's the right place to change the state from the drag.
Also, I think passing information about the relevant Draggable will become much less necessary. "The understanding is that if we return the id of the things that have been dragging then a consumer should be able to look up whatever data they need from that id." I think you're right but we're already looking up that data at the Droppables. The Droppables will need the state to have generated the Draggables in the first place, so it's the right place to look things up by id.
Here's my super hack to accomplish this for myself:
DragDropContext.onDragEnd is emitting events on an EventEmitter where the event names correspond to the droppable ids.Droppables have it.Droppables are listening to the events corresponding to their own droppable id. (Make sure to remove listeners on unmount!)Super interesting @saiichihashimoto. Thank you
First off, thank you for this great library. It's really well architected.
I just wanted to +1 the approach @saiichihashimoto advocates. I think that both Droppable and Draggable should be allowed to define the onDrag* handler methods themselves, so that we can handle state changes right where they need to be handled in the component hierarchy.
For Redux-driven apps the current top-level DragDropContext makes a lot of sense. But for apps using any render-prop-based APIs to manage state, we need be able to define drag/drop handlers inline where the state actually lives. For example in components that are rendered with react-apollo, or REST alternatives like holen or react-request.
(This also applies to apps using HOC-style state management libraries like react-refetch, or even just the built-in state/setState primitives React provides.)
For example, an app might be structured something like:
<DragDropContext>
<App>
...
...
<Query query="some list query">
{({ list, updateList }) => (
<Droppable ...>
{data.map(item => {
<Draggable ...>
...
</Draggable>
})
</Droppable>
)}
</Query>
</App>
</DragDropContext>
In this case, the list of items and the updateList method that is used to synchronously update the state are only available at the level of the tree where the droppable is defined.
To update them from the DragDropContext.onDragEnd requires lots of hacks (using event emitters and refs as @saiichihashimoto mentioned) to work.
Instead, it would be ideal to be able to just define an onDragEnd on the <Droppable> container, and have it scoped to only be called when that specific droppable container is either the source or destination of a drag.
The same actually goes for <Draggable>. I have a use case in my app right now where I need to "collapse" a draggable item before its dragged, and right now doing that requires the same hacks since the expanded state is only available deeper in the render tree.
So I think that both <Droppable> and <Draggable> should be allowed to define all three handlers: onDragStart, onDragUpdate, onDragEnd. For droppables, the handlers should only be called when the source.droppableId or destination.droppableId matches the droppable. (And obviously for draggables only when the draggableId matches the draggable.)
(@saiichihashimoto suggests separating them into two handlers, like onDroppedFrom and onDroppedOnto, but I think this can be handled by simply checking the source.droppableId and destination.droppableId instead to keep things simple.)
That would make this library incredibly flexible, an unopinionated to any of the different approaches to state management.
Thanks!
There are some things that need to be thought through (such as what hooks are called when moving from one list to another), but I am liking the direction of this discussion. It seems like adding some callbacks could be a useful addition!
This affects me as well. I have a case where the are multiple types of "droppables" nested within one another. Since I need to use one DragDropContext for the whole thing, I need to pass some information into the onDragEnd (or potentially any other hook functions) in order to work out what to do, based one what was dropped where.
I'd think this could be implemented in a fairly straight forward way by allowing a prop to be set on the Droppable and Draggable components which gets passed in to the hook functions when they are called. I don't have a strong opinion on what this prop should be called.
type DraggableLocation = {|
...
droppableId: DroppableId,
params: object
|};
type DropResult = {|
...
source: ?DraggableLocation,
destination: ?DraggableLocation,
params: object
|}
This would be huge. I'm cloning and modifying a list before submitting a result to a database, which will, in-turn, result in the redux store being updated. A user will modify the list via an "edit" popup. The cloned list is held in the Editor component's state and is inaccessible to DragDropContext.
I was thinking that I could pass a bound function as the id (per suggestions earlier in this thread) for updating that state, but it seems like this hack is no longer possible.
I gotta say, though, thank you for all of your excellent and creative work on this library. It's really fantastic.
Whatever the path taken, I'd really appreciate if we can have the ability to pass arbitrary data (functions) around.
will this feature be applied in last version ?
UPDATE - turns out this seems to be causing an intermittent bug where the library throws an error at the start of a drag operation - cannot modify draggables during a drag operation.
I'm stuck on v7.1.3 so can't say if this works with the current version, but the following sneaky hack seems to work. Provide an object as your draggableId like so:
{payload: arbitraryData, toString: () => "unique string"}
Note this doesn't work with droppableId, due to the Failed child context type mentioned above. @alexreardon maybe just loosening the prop-type for droppableId is all that would be needed to get this to work?
@Frozenlock I happen to also be doing this from ClojureScript (reagent), i.e.
{:draggable-id #js {:payload my-data :toString (fn [] "...")}}
The fact that this is a JS object means Reagent doesn't mess with the payload, so ClojureScript data comes through intact. I guess that doesn't apply to you as your payload would be a function, but worth mentioning.
I am super open to suggestions for API proposals for this one.
Would allowing passing a data object be enough? I am leaning towards calling it payload. Would we need to have hooks for Droppables if we had this payload?
If the payload could be type any, then it could be a function that you could do what you want with
/cc @Frozenlock @saiichihashimoto
A complication could be: if we allow a payload prop, then we would capture that at drag start and keep it a reference to it. We would not recollect it during a drag. Could pass this payload to onDragEnd (maybe the other hooks too?).
This could be strange if people pass in a new function during a drag - it won't get called. The old one will.
I think this is okay as long as we call it out.
Thoughts @Frozenlock ?
I don't see this as a problem in my case.
This could be strange if people pass in a new function during a drag - it won't get called.
Is this an optimization issue? I can imagine cases where the payload would benefit from an update while in mid-drag.
I am thinking about how we perform our capturing. We currently do not recapture any information from a Draggable once the initial lift is done. Needing to recapture the payload would be a pain
Ah I see.
Not a big deal, especially considering there are hooks available that can do the same thing.
I like the payload proposal. I would rather have them on the onDragEnd, as I can then have a complete separate DragDropcontext component that acts as a stateless gatekeeper that checks every event and authorizes/denies actions by looking at the source, destination AND payload.
Still, if it's on onDragStart, we can just redux it, no problem. Yet, this is a very welcome change, as it avoids me to go into ref forward hell in my component hierarchy to get the Draggable elements.
Can I infer from your words that you will add this feature in your todo list?
The simpliest path forward would be to add a payload prop to Draggable that would be of any type. This prop would be captured at drag start and provided to onBeforeDragStart, onDragStart, onDragUpdate and onDragEnd.
It would be an any so a function could be passed in. You could then call this function in your hooks. Something like:
onDragEnd = (result) => {
// you could pass the whole result to your payload function if you want
result.payload(result);
}
Would this be sufficient @Frozenlock? I think this is the most flexible option. I am struggling to come up with a generic api for Droppables / Draggables given that you can move from one list to another.
onDragEnd for a Draggable: can it see the data that generated it?
onDragEnd for a Droppable: which droppable do we call? The original, destination, both?
I've re-read my previous comment on this issue and double-checked my current code : I'm associating functions to the droppable. The droppable itself can know how to deal with an element added/removed, while the element being dragged simply has to know its identity.
For example, let's say I drag an element from a list of 10 elements (droppable1) to a list with at most 1 element (droppable2).
One can also imagine a scenario where the dragged object needs to be "transformed" before being added to a particular list.
The draggable doesn't know how it should act when dropped on a list. This is the droppable's job.
(It doesn't mean that Draggable shouldn't have a payload, simply that Droppable should have one.)
Any chances this could make its way into #838?
I'm still on 6.0.0 because of this issue and I'm eager to use a newer version.
To follow up on @frozenlock’s point about payloads on droppable, I just want to clarify that a payload on the draggable is definitely what I would like to see added.
Of course, I don’t have any objection to there being something similar on the droppable, although I would suggest “payload” is not the right term in that case, as it implies something is moving and it has some contents inside.
Tom
On 5 Oct 2018, 19:22 +0100, Frozenlock notifications@github.com, wrote:
Any chances this could make its way into #838?
I'm still on 6.0.0 because of this issue and I'm eager to use a newer version.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
@tslocke Would you mind detailing a little how you'd use the payload on the draggable? Especially considering you're using Reagent, I wonder how we ended up at opposed ends.
It seems like a generally useful thing to have a Draggable > payload. It could open up a few control flow possibilities, but it seems like people would like it as a mechanism to pass information to onDrag*.
Given this I think I will try to get it into #838 @Frozenlock.
Thoughts?
export type DragStart = {|
draggableId: DraggableId,
type: TypeId,
+ payload: ?mixed,
source: DraggableLocation,
mode: MovementMode,
|};
And Draggable props would get payload: ?mixed
This payload would be captured once for a drag, on drag start (same as draggableId)
I have added Draggable > payload to [email protected]
I have removed it for now until we get a clearer way forward
I'm still unclear about the advantages of having a payload on the draggable as opposed to the droppable. (see previous comment)
Could someone give me a control flow example with the draggable?
I was thinking that you could pass something into the draggable payload through context if needed. But perhaps something more robust is needed. Thinking out load:
payloads: {|
draggable: ?mixed,
// draggable payloads:
source: ?mixed,
destination: ?mixed,
|}
That way a Draggable and a Droppable could specify a payload. It just feels a little strange for a Droppable to define a payload as it does not move
I do not think a Droppable payload makes a lot of sense. But hooks on a Droppable do for the purpose of updates. It is almost as if we need a Draggable > payload as well as onDrag* hooks on Droppables. The part I have not figured out is the calling of the Droppable hooks. It is a bit confusing because often operations span multiple Droppables. Do we just call all the hooks on the Droppables if they are impacted by an update / drop? 🤔
I was thinking that you could pass something into the draggable payload through context if needed
Wouldn't this only work for the source Droppable? I can easily store something from the source Droppable into the Draggable's payload, but what about the destination Droppable?
My initial approach was to store functions (source-fn and destination-fn) into the Droppable IDs because they both end up in the onDragEnd hook. In other words, it was possible for my Droppable components to provide all the manipulation functions and leave the DragDropContext as universal as possible.
Let's give a simple example with a food Draggable :
Droppable;Droppable.The way I see it is that each Droppable is responsible for providing its manipulation functions.
By keeping everything separated, I can even decide to add a completely new Droppable without modifying any of the existing code. For example I can add a Cook Droppable.
However, considering I haven't seen many people discussing the dispatching issue, I must be missing some obvious alternate way of dispatching on the onDragEnd hook.
After you asked for more details @frozenlock I went back and had a look at the code and was reminded that my previous comment was completely wrong : ) I put EDN stringified data on both the droppable and the draggable.
I have what I think is fairly typical for ClojureScript apps - a single big associative data structure holding the state of the app. Drag and drop allows the user to move some data from one place in that structure to another, so for each droppable I store the path to the collection (a vector of keywords and integers). On the draggable I store the path to the data itself, although that is redundant really - the path on the droppable plus the index of the draggable gives the same information.
When the user drags some data I can use get-in to find the data, and update-in to make the changes (remove from source, add to destination).
So it’s a very data-centric approach. I rarely need to run any custom code for a drag-drop operation, but if I do I have a simple system for adding data watchers on nodes in the data, which can trigger anything (via core.async channels)
Tom
On 8 Oct 2018, 03:22 +0100, Frozenlock notifications@github.com, wrote:
I was thinking that you could pass something into the draggable payload through context if needed
Wouldn't this only work for the source Droppable? I can easily store something from the source Droppable into the Draggable's payload, but what about the destination Droppable?
My initial approach was to store functions (source-fn and destination-fn) into the Droppable IDs because they both end up in the onDragEnd hook. In other words, it was possible for my Droppable components to provide all the manipulation functions and leave the DragDropContext as universal as possible.
Let's give a simple example with a food Draggable :• Freeze Droppable;
• Thaw Droppable.They way I see it is that each Droppable is responsible for providing its manipulation functions.
By keeping everything separated, I can even decide to add a completely new Droppable without modifying any of the existing code. For example I can add a Cook Droppable.
However, considering I haven't seen many people discussing the dispatching issue, I must be missing some obvious alternate way of dispatching on the onDragEnd hook.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
(...) so for each droppable I store the path to the collection (a vector of keywords and integers). On the draggable I store the path to the data itself, although that is redundant really - the path on the droppable plus the index of the draggable gives the same information.
Thanks @tslocke, it appears you also store data inside the Droppables to use later. I'm not just a random crazy person! :-p
I am keen to get something in. However, we currently lack a cohesive path forward.
Draggable > payload seems reasonable
But what about Droppables? Should they too have a payload or should we expose hooks (eg onDragStart) on them?
What would the exact behaviour of a onDragUpdate / onDragEnd be when moving between lists?
I think this feature would solve a problem we have too, I accidentally broke our react-beautiful-dnd solution by changing the string ID used on Draggable, unaware of this workaround:
private handleDragEnd = (result: DropResult) => {
const { onMoveQuestion, onMoveOption } = this.props
// Check if it was dropped outside of the list
if (result.destination == null) {
return
}
// Since we handle both question and option drags with the same
// context we have to differentiate them by the draggableid
const draggableId = result.draggableId.split(".")
const dragSubject = draggableId[0]
const sectionIndex = parseInt(draggableId[1], 10)
const questionIndex = parseInt(draggableId[2], 10)
if (dragSubject === "question") {
onMoveQuestion(
sectionIndex,
result.source.index,
result.destination.index
)
} else if (dragSubject === "choice") {
onMoveOption(
sectionIndex,
questionIndex,
result.source.index,
result.destination.index
)
}
}
Sorry I've been MIA for a while. :-) I think this library is wonderful and I've used it in a ton of projects. But yes, the context in which I want to handle drag events could still be with the Droppable, imo. I'm starting to lose track of this conversation, so let me share my wrapper around this library I currently use.
https://gist.github.com/saiichihashimoto/2c956fac9184d881922ab636628787f6
I think this exemplifies what a few people have mentioned. It generates a unique droppableId for each Droppable. These IDs have only mattered me just to achieve what comes next:
Droppables are receiving the drop event. It receives DragDropContext.onDragEnd to have a reference to draggableId. I'm usually generating a list of Draggables in a Droppable, so I probably have react-redux or whatever I'm using to map draggableId back to my data handy.
This is where I'd rather a payload. You mostly certainly use some object to render Draggables but, at onDragEnd, you just have a draggableId. I'd rather have that same object. Whether I'm using redux, flux, whatever, I'd prefer the object that represents the Draggable, because a draggableId just means I have to go find the object again.
This would let us get back to recommended react paradigms. I think of this library as an excellent React UI/UX for passing elements between lists. We usually think of these lists getting or receiving elements rather than a global context handling all of their interactions, and having the hooks reflect that would be very intuitive. We no longer need droppableId or draggableId, Draggables just unroll from arrays (using key, most likely), and then Droppables handle when they gain/lose their elements.
Anyways, I love this library. The interactions are beautiful and solid, and I can't imagine the DOM nightmare that it's keeping away from me. I think having this would make the component management rock solid and intuitive.
I like @saiichihashimoto's approach, and I'm all in for adding hooks to the Droppables. It could be a single onDrop hook on the Droppable, that would fire both on the source and the destination Droppable, no matter if the Draggable is moved between different lists, etc. However it could be a problem if the hook was fired twice when a Draggable is moved within the same Droppable (the Droppable would both be the source and the destination). eg. if we need to tell the backend that our list have updated, we don't want to tell it twice.
As @saiichihashimoto mentions, it is common to have the DragDropContext pretty far away (hierarchy wise) from the actual lists, Droppables and Draggables. What I currently do, is essentially lifting the state up, and passing it down to the Droppables using Context. So I have a parent component high up that manages the different lists, and holds the DragDropContext, thereby giving the onDragEnd access to the lists state. The list are then passed down via context to the Droppables, Which means they can't manage the lists in any way. My understanding is that this is a bit different from @saiichihashimoto's approach, where the lists are located at the Droppables, but they listen to Events that are passed from higher up the tree. But the problem is the same, we need to pass something (the actual state, or an event to change the state) from a parent component down to our list containers, instead of just giving the lists their own way of managing the lists.
I hope I at least contributed a little to the discusion. Have a nice day y'all!
So @alexreardon, I think it boils down to:
Draggable has some kind of payload/state/whateverDroppable gets the payload in events like onDragStart, onDragEnd, onDrop. Whatever you'd like that lets them know they received or lost something.DragDropContext. If we throw all of the drag start/end hooks into Droppable, the only thing left here is onDragUpdate and I have a hard time coming up with a practical use for this.Droppables involved would get the hook?@saiichihashimoto actually my workaround for this was adding a new middleware where I can get the store without connecting (to redux) the component that has the DragDropContext.
it works pretty well for me.
A good time to make this change could be when we change the API to be hook based #871
This is definitely something I want to improve!!
Starting small it might be sufficient to add a payload prop to a Draggable which would unlock some behaviour.
When we move to hooks #871:
Responders on the DragDropContext and have them all on the DroppableResponders on Droppables@alexreardon Hi Alex. I'd really like to use this this payload prop on a Draggable. Any updates on this issue?
I'm also looking to use the payload prop to pass functions up the component tree. Just waiting to see how this issue progresses, or if I should go with a workaround in the same light as saiichihashimoto's solution.
Is there any way we can move this forward? What would get us to some kind of action?
I am busy working on some other core features right now. Once those land I am hoping to do some api redesign. Move to hooks etc. This would be a part of that
Nice lib, thank you very much :) Here's what I ended up with, will call handlers in props of Droppables involved, both destination and source
edit: removed code, see next comment
Made the workaround from my previous comment into a gist, and
payload to Draggable and DroppableDraggable in addition to those in Droppable and DragDropContexthttps://gist.github.com/mikkokaar/5caff07a33f4711aa3dd72a40b4e7a73
Thanks for providing the workaround. I ended up doing this.
const DragDropContext = createContext()
const DragDrop = ({ children }) => {
const [eventsMap] = useState(() => new Map())
const addEvent = (event, callback) => {
eventsMap.set(event, callback)
}
const emitEvent = (event, ...rest) => {
if (!eventsMap.has(event)) return
eventsMap.get(event)(...rest)
}
const removeEvent = event => {
eventsMap.delete(event)
}
const handleDragEnd = item => {
if (!item.destination) return
emitEvent(item.destination.droppableId, item)
}
return (
<Fragment>
<DragDropContext.Provider value={{ addEvent, removeEvent }}>
<DragDropContext
onDragStart={handleDragStart}
onDragEnd={handleDragEnd}
>
{children}
</DragDropContext>
</TransportContext.Provider>
</Fragment>
)
}
Then in drop components
const { item, addEvent, removeEvent } = useContext(DragDropContext)
useEffect(() => {
addEvent(DND.SIDEBAR, item => {
console.log(item)
})
return () => removeEvent(DND.SIDEBAR)
}, [])
@cocacrave Thanks for the temporary solution to this! I've tried it using version 10.0.4 and it's working great.
I was just wondering whether you've tried it using 10.1.0? I'm having problems with the drop area incorrectly calculating height and I'm struggling to work out if it's something that changed in the library which is incompatible with the work around or just some flex problem in my app.
@llamamoray You're right. It's working fine on 10.0.4 but breaks badly on 10.1.0. Well that sucks... Hopefully the library implements this RFC before I need to upgrade the package.
Things I noticed were the "placeholders" not being held in place on drag. And maybe as a result the drop moveTo is placed at a weird location.
I have no clue why the workaround above would mess with positioning... doesn't make sense.
I am working on a hooks api right now, and this sort of thing is being considered #871
Thank you to everyone for your input so far!
I am keen for people thoughts:
Currently we have onBeforeDragStart, onDragStart, onDragUpdate and onDragEnd on the DragDropContext (the Responders). While this works, often people want to be able to manage these concerns closer to where the drag is happening: at the Droppable level.
Allow Responders on Droppables
Responders on DragDropContext remain unchangedDroppable can also register any Responder 👍Droppable (the Droppable that the Draggable starts in) (essentially a mirror of DragDropContextonDragUpdate fires on a foreign Droppable when dragging over the Droppable and also when leaving the Droppable.onDragEnd fires on a foreign Droppable when dropping into itMy current thinking is that the order of calling will be:
DragDropContext RespondersDroppable RespondersThese calls will be synchronous so the order is not super important
DragDropContext onDragEnd: no longer requiredBecause onDragEnd could be handled in a number of places, we would no longer make onDragEnd required for the `DragDropContext
payloadI suspect that we could not need a payload in this new setup as reordering could be handled much closer to the drag operations
↑ @spatialvlad, @Frozenlock, @saiichihashimoto, @ianstormtaylor, @rhys-vdw, @JReinhold
All in all it sounds like a great approach. A few comments:
Droppable is home/foreignIf onDragEnd is both fired on DragDropContext, home Droppable and foreign Droppable, I expect almost all onDragEnd calls to start with a check that figures out if the responder was called because this Droppable was the home or the foreign droppable. Eg. something like:
<Droppable droppableId={myId} onDragEnd={(result) => {
if (result.source.droppableId === myId){
// this is home droppable, do something related to that
} else if (result.destination.droppableId === myId) {
// this is foreign droppable, to something else
}
}} />
My point is, that if we are certain that this check always have to be made, we might as well supply the information directly in the result to make it easier. something like adding a boolean to the result that specifies whether this Droppable is the source or destination. I haven't really thought the specific API through, but I guess you get the idea. (this probably applies to the other responders as well, I just used onDragEnd as an example). Although I acknowledge that this would increase the surface area of the API for very little gain, which would be a valid argument not to move forward with this.
payloadI've never used the payload pattern described by the others to solve this issue, so I might not be the best to chime in on this, but here goes. On one hand I agree that in this new situation the payload would not be needed, as in most cases the list of the Draggables is available when initialising the Droppable and it's responders. Therefore, we are always able to find the data the Draggable represents in the Droppable's responder. A contrived example:
const MyList = (props) => {
const todos = props.todos;
const handleDragEnd = (result) => {
const todoId = todoIdFromDraggableId(result.draggableId);
const todo = todos.find((todo) => todo.id === todoId);
// BOOM, we have the todo, do whatever we want with it
// eg. apiRequestToMoveTodo(todo.id, result.index, droppableId)
}
return <Droppable ... onDragEnd={handleDragEnd}>
{todos.map(todo =>
<Draggable ... draggableId={draggableIdFromTodoId(todo.id)} />
)}
</Droppable>
}
However if we do include the payload (in this case, an object describing the todo), we wouldn't have to find the todo in the list, (and we wouldn't have to maintain the todoIdFromDraggableId() and draggableIdFromTodoId() functions). This makes it much easier to work with the data in the responders. I don't know what the performance implications are of including the payload, but if it becomes a problem, we could always have tip that explains how to not use the payload, and instead look up the data in the responders like I do above.
Anyways, that's all I had, so only very minor details from my end - I think it looks solid!
I'm on a flight to Japan for a nine day trip without my laptop so I won't be able to weigh in properly.
This is great news! I think it's much better to have all Droppable able to handle the drop action themselves :+1:
This makes it trivial to implement my previously given example :
Food Draggable :
Droppable;Droppable;Droppable.Now each Droppable can have its own function to handle the drop.
In my particular case, having Droppable register as Responder would remove the need for the dirty hack of inserting functions inside the droppable ID.
I still think that having a payload would be very useful.
While I can generate unique ID strings for react-beautiful-dnd components, internally my objects IDs are not strings. By having a payload, I could skip the whole "find my objects given a generated string ID", or even skip "find my objects" and directly apply functions to the payload. Unless it affects the speed, having the ability to pass a payload enables users to do pretty much everything they want.
But @JReinhold already made the case better than I could.
@spatialvlad, @Frozenlock, @saiichihashimoto, @ianstormtaylor, @rhys-vdw, @JReinhold I have created a spike: #1250.
It does not include a payload or @JReinhold's suggestion. However, it does set up the architecture. Right now it simply allows for a Droppable to specify responders 💃
I am keen to get feedback
I'm not exactly sure what all has happened in this discussion. But in my opinion the solution that makes this library flexible enough for any architecture is...
For both <Droppable> and <Draggable> to have the onBeforeDragStart, onDragStart, onDragUpdate and onDragEnd props. And for them to be scoped exactly how you'd expect, eg. they are only called when the drag/drop relates to them. Simple to understand, and gives you access to the events anywhere.
(And for the <DragDropContext> to not make any of them required. And if we're being honest, for <DragDropContext> to eventually be unnecessary.)
So it sounds like maybe this is halfway towards the ideal? Which is a good first step. Although I'd still want them on the <Draggable> to be honest.
Was wondering on where this has landed.
I started to look into this library to find out the onDragEnd must be at the top level ( which means it is outside of the context I need it to be in for the droppable) the solution posted back in 2019 sounded PERFECT , ( the logic for the drag and to be on the thing that accepts it makes sense)
However, looking at the other issue linked to this, it looks like there was some snags with moving to public facing hooks, did that kill this issue?
Same question here. I followed the discussions, but it looks that this did not make it in the main branch. Any update, do you think that you will be able to push it soon?
Note: For folks still looking for this, I found the solution posted by @mikkokaar to help, till the final solution would be added: https://gist.github.com/mikkokaar/5caff07a33f4711aa3dd72a40b4e7a73
Most helpful comment
I think we're going about this the wrong way. It's good that "this library has no opinions about how you manage your state" but I'm finding that, in most cases, it's in the context of
Droppablethat I know what I want to accomplish, not at a much higher context ofDragDropContext. It's too detached.The current suggestions are to pass up state from the
Draggableto theDragDropContext. But the fact that you're mapping everything back to theDroppablesseems to indicate that the logic is far from the relevant area of responsibility. With enoughtypesandDroppablesand the fact that all of that gets handled inonDragEnd, you're bound to have complex logic even though you've most likely pulled the same state at the relevantDroppables.Here's my suggestion. We leave
onDragStart,onDragUpdate, andonDragEndfor global UX concerns like blocking updates on drags and backwards compatibility.Droppablesget aonDroppedOntoand some kind of "leave" hook (onDroppedFrom?). They'd receive whatonDragEndreceives now, more or less.I think this solves most use cases.
onDragEnddoesn't have to get complicated because theDroppableswill receive the events that matter. Since theDroppablemost likely handles retrieving, removing, persisting, sorting, etc from state, it's the right place to change the state from the drag.Also, I think passing information about the relevant
Draggablewill become much less necessary. "The understanding is that if we return the id of the things that have been dragging then a consumer should be able to look up whatever data they need from that id." I think you're right but we're already looking up that data at theDroppables. TheDroppableswill need the state to have generated theDraggablesin the first place, so it's the right place to look things up by id.Here's my super hack to accomplish this for myself:
DragDropContext.onDragEndis emitting events on an EventEmitter where the event names correspond to the droppable ids.Droppableshave it.Droppablesare listening to the events corresponding to their own droppable id. (Make sure to remove listeners on unmount!)