lukasbach / react-complex-tree

Unopinionated Accessible Tree Component with Multi-Select and Drag-And-Drop
https://rct.lukasbach.com
MIT License
915 stars 72 forks source link

OnDrop returns not correct childIndex for target.targetType = 'between-items' #344

Closed HannaHabr closed 4 months ago

HannaHabr commented 5 months ago

Describe the bug Hi! I understand that this could be intentional, but when I drop 'between-items' I don't get expected target.childIndex.

To Reproduce

  1. Create a tree with 3 items in the root, let's name them A,B,C. Enable reordering of items. Log target object in onDrop callback
  2. Drag item C to the beginning, observe target.childIndex = 0which is expected
  3. Drag item C between A and B, observe target.childIndex = 2

Expected behavior I expect that in step 3 I get childIndex = 1. Could you confirm that above behavior is correct?

Screenshots

Desktop (please complete the following information):

Additional context My goal is too get new order position of the item. Right now I can't do this by using onDrop. If you could recommend how to better get new position order that would be great!

lukasbach commented 5 months ago

This is currently how the drop logic is intended, I'm happy to hear alternative suggestions for how this could be handled, but this is the indended way how the new child index is computed.

The complication comes from the drop child index being computed based on the original tree state. When C is dropped between A and B, there are two items above the new drop position, A and C, so the new index is "2", thus the new index is computed and returned before C is moved away, and the new drop position shifts upwards. There exist use cases where the old items are note removed from the old position when they are dragged away, so shifting this position upwards before that happens doesn't always work.

Unless you have a complicated use case, it might be easier not to use the onDrop handler directly, and use an uncontrolled tree environment instead, where you can handle drop logic with a tree data provider.

If you want to handle the drop logic yourself, and want to be able to have items be moved down, you need to account for updating the index based on items being dragged and originating from the drop container yourself. The uncontrolled tree environment does that for you if you use that, maybe its implementation can give you a starting point for how to handle that.

HannaHabr commented 4 months ago

Hi, I really appreciate your answers, playing around with drag and drop I finally realized the reason you described, at least I got confirmation on what I was thinking. I was able to get new position by getting children of parent item from the dataProvider and searching for dropped item index. This made a thing. Thanks a lot. image

lukasbach commented 4 months ago

Happy to hear that that helped!