ClayTreeView: Improvements for drag and drop

[sandrodw3]

Hey guys, I've been testing the newest drag and drop improvements in ClayTreeView and I have some feedback!

• Currently we are using the draggable prop for both drag and drop. It would be good to have them separated, as it could happen that an item is draggable but not droppable (it’s happening in our use case for example).
• If we do it and add droppable prop, it would be awesome if it was a callback that gets the dragged item, or having some way to condition it to the dragged item. Sometimes, an item is droppable for some type of item but not droppable for the rest.
• About onItemMove, I have found out that with the current API, we can’t know the new index of the item if we are moving it to a different position inside the same parent. Would it be possible to expose also the new index? So we can pass to the component a function like (item, parentItem, newIndex) => {…}

What do you think? 😄 (cc @matuzalemsteles)

[matuzalemsteles]

Hey @sandrodw3 thanks for the feedback, well I'm just concerned to provide two APIs with the same purpose, so before we go with that let me understand the goals here.

Looking at the first two points it looks like you have use cases that would be supported by onItemMove right? or do you think he still wouldn't be enough? Because if we add a droppable we would have the same behavior as onItemMove could do, you can add your rule of an item it can be droppable in some other item or not, this is more dynamic.

Maybe we can separate the droppable from the draggable API but keep it simple by just accepting a conditional. Because I think if we add a callback we will have the same behavior as onItemMove. The draggable is just a conditional that actually disables the DnD reference so it is before enabling DnD for an element as opposed to onItemMove which is an after rule... they are good for preventing draggable animation is created, droppable doesn't have that same purpose because it doesn't have an animation, so you can do it using onItemMove.

Just a question to better understand the third case, if the item only moves position within the same parent, do you also need to ignore droppable in some scenarios? I think we can provide more information about the position.

[sandrodw3]

Hey @matuzalemsteles thanks for your answer!

Let's see, for the droppable and onItemMove issue. You are right that returning false on onItemMove works, but the thing is that happens after the drop, so we have to show the animation of dropping and then let the user know that the drop hasn't been done. What I thought was having a callback that we can check on hover and not show the animation if we do not want to. I mean, it's different to check it after dropping than on hover. Maybe we could have an onItemHover and make it work similar to onItemMove? Return false if we want to cut the drag. Please let me know if I explained it well.

Um, about the third scenario, I guess yes, we should also ignore some scenarios, why do you ask? I guess adding a newIndex to the onItemMove callback would be enough, right?

Thanks for your help!

[matuzalemsteles]

Let's see, for the droppable and onItemMove issue. You are right that returning false on onItemMove works, but the thing is that happens after the drop, so we have to show the animation of dropping and then let the user know that the drop hasn't been done. What I thought was having a callback that we can check on hover and not show the animation if we do not want to. I mean, it's different to check it after dropping than on hover. Maybe we could have an onItemHover and make it work similar to onItemMove? Return false if we want to cut the drag. Please let me know if I explained it well.

Hmm I see, I think I understand this behavior now, yes that makes sense. You want to check before the drop happens, yes I think a onItemHover fits very well here, the droppable it won't work well for that because it disables, which is before starting a drag for example or if it was the case of the drop, as you probably need to know who is in drag to allow the drop I think droppable will not be enough because you would need to store in a ref or state which item is in drag to do the droppable conditional.

Um, about the third scenario, I guess yes, we should also ignore some scenarios, why do you ask? I guess adding a newIndex to the onItemMove callback would be enough, right?

I'm just wondering if there are other use cases, if you need to know for example the previous position.

[sandrodw3]

Hmm I see, I think I understand this behavior now, yes that makes sense. You want to check before the drop happens, yes I think a onItemHover fits very well here, the droppable it won't work well for that because it disables, which is before starting a drag for example or if it was the case of the drop, as you probably need to know who is in drag to allow the drop I think droppable will not be enough because you would need to store in a ref or state which item is in drag to do the droppable conditional.

Cool! Makes sense, so onItemHover sounds like a good solution then!

I'm just wondering if there are other use cases, if you need to know for example the previous position.

Um, I hadn't think about it. I think in our case we don't need the previous position, but I guess it would be cool to add it also just in case it's needed for other cases.

Thanks @matuzalemsteles 🙂

[github-actions[bot]]

This issue has been merged and will be released in DXP at https://issues.liferay.com/browse/LPS-166807