Discussion: placement of handles and flow of data

[spatialvlad]

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. reduxjs/redux#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):
#120
#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.

[alexreardon]

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

[spatialvlad]

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.

[Frozenlock]

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.

[alexreardon]

Interesting @Frozenlock. Does that work with our current setup?

[Frozenlock]

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.

[alexreardon]

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 !

[alexreardon]

I need to work through whether that would enable people to get done what they want to. Using a function is a brilliant idea

[Frozenlock]

Playing a bit around (with multiple droppables this time), here's what seems to work inside the droppable ID:

• String (obviously)
• Array
• Functions

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.

[alexreardon]

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

[alexreardon]

Adding a prop with a name like:

• data (clash with DOM data-*)
• meta
• params
• associated
• extra
  ...? Suggestions!?

to pass around to the hooks seems like a good idea.

Not sure what to name the prop though

[Frozenlock]

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?

[alexreardon]

Details to be confirmed, but something like that.

[Frozenlock]

Upgraded to 7.x.x, doesn't work anymore. 😢

Warning: Failed child context type: Invalid child context private-react-beautiful-dnd-key-do-not-use-droppable-id of type array supplied to Droppable, expected string.

Hopefully the new field will make its way in the next version.

[davps]

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)

[saiichihashimoto]

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:

• My DragDropContext.onDragEnd is emitting events on an EventEmitter where the event names correspond to the droppable ids.
• I'm passing the EventEmitter as a prop throughout my component tree so the Droppables have it.
• Those Droppables are listening to the events corresponding to their own droppable id. (Make sure to remove listeners on unmount!)
• No more routing logic and every Droppable can handle it's own events.
• Besides having to pass an EventEmitter around everywhere, this has been a much better experience.

[alexreardon]

Super interesting @saiichihashimoto. Thank you

[ianstormtaylor]

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!

[alexreardon]

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!

[twheys]

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
|}

[euroclydon37]

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.

[Frozenlock]

Whatever the path taken, I'd really appreciate if we can have the ability to pass arbitrary data (functions) around.

[Aarbel]

will this feature be applied in last version ?

[tslocke]

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.

[alexreardon]

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

[12tp12]

@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.

[alexreardon]

A good time to make this change could be when we change the API to be hook based #871

[alexreardon]

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:

• we might be able to do away with any Responders on the DragDropContext and have them all on the Droppable
• mirror the Responders on Droppables

[shtabnoy]

@alexreardon Hi Alex. I'd really like to use this this payload prop on a Draggable. Any updates on this issue?

[bmaclean]

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.

[saiichihashimoto]

Is there any way we can move this forward? What would get us to some kind of action?

[alexreardon]

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

[mikkokaar]

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

[mikkokaar]

Made the workaround from my previous comment into a gist, and

• added payload to Draggable and Droppable
• made it call handlers in Draggable in addition to those in Droppable and DragDropContext

https://gist.github.com/mikkokaar/5caff07a33f4711aa3dd72a40b4e7a73

[cocacrave]

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)
  }, [])

[llamamoray]

@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.

[cocacrave]

@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.

[alexreardon]

I am working on a hooks api right now, and this sort of thing is being considered #871

[alexreardon]

Thank you to everyone for your input so far!

I am keen for people thoughts:

Background

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.

Proposal

Allow Responders on Droppables

• Existing Responders on DragDropContext remain unchanged
• A Droppable can also register any Responder 👍

Details

• all responders fire for all updates on a home Droppable (the Droppable that the Draggable starts in) (essentially a mirror of DragDropContext
• onDragUpdate 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 it

Order

My current thinking is that the order of calling will be:

1. DragDropContext Responders
2. Droppable Responders

These calls will be synchronous so the order is not super important

DragDropContext onDragEnd: no longer required

Because onDragEnd could be handled in a number of places, we would no longer make onDragEnd required for the `DragDropContext

Open question: payload

I suspect that we could not need a payload in this new setup as reordering could be handled much closer to the drag operations

[alexreardon]

↑ @spatialvlad, @Frozenlock, @saiichihashimoto, @ianstormtaylor, @rhys-vdw, @JReinhold

[JReinhold]

All in all it sounds like a great approach. A few comments:

Extra info about which Droppable is home/foreign

If 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.

Not needing the payload

I'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!

[saiichihashimoto]

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.

[Frozenlock]

Responder

This is great news! I think it's much better to have all Droppable able to handle the drop action themselves 👍

This makes it trivial to implement my previously given example :
Food Draggable :

• Freeze Droppable;
• Thaw Droppable;
• Cook 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.

Payload

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.

[alexreardon]

@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

[ianstormtaylor]

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.

[molster]

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?

[danvln]

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

[amaster507]

@alexreardon what is the current status of this? Still looking for feedback for the API or has this been implemented somewhere and I have overlooked it.

I have added beautiful dnd into my project and have added the provider at a scope where I had my state logic. But now I need to drag items from within that scope to an entire different scope of a distant cousin component. If I move the provider up, I loose my state that needs updated. I am using react hooks for state and not redux. I am trying to tie all of this together with GraphQL using Apollo Client hooks as well. The main problem is that depending on what "layer" of droppables the item is dropped into, it runs different mutations based upon the layer and the id. I would like to keep this as simple as possible still to. But I think I might need to convert to React DnD to accomplish my goals as that ties the onDrop event to the droppable which receives the draggable props.