lukasbach / react-complex-tree

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

When the last folder is opened, no other items can be moved below the last folder. #261

Closed vinJeong closed 4 months ago

vinJeong commented 1 year ago

Describe the bug

If you try to move another item to the last folder when the last folder is open, it will only move into the last folder, not below the last folder.

To Reproduce Steps to reproduce the behavior:

  1. Leave the folder in its last location.
  2. Open the folder.
  3. Attempts to move another item below the last folder.

Expected behavior Instead of being moved under the last folder, it is moved into the last folder.

Screenshots May-10-2023 14-26-21

The gif above is from https://rct.lukasbach.com/I took it from the front page.

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

lukasbach commented 1 year ago

What exactly would be the behavior that you are looking for? If the last tree is expanded, the bottom-most drag-position can only have one implementation. If we don't have it move to the final inner-most folder, it makes it very unclear where the item should go. For example, consider a structure where the user drags the "a" item to the bottom-most location

- a
- b
  - c
    -d
      -e

At the moment, it would move the item into the bottom-most folder, so d, or e if e is an expanded folder with no items. If we don't want that, where should the item go then? c? or b? or into the root folder?

The current behavior matches the implementation that Microsoft's WinUI follows. I'm not sure what other good examples of established implementations exist, but I'm happy to see other examples of how this is done otherwise in practice.

winui-tree

felixfaire commented 1 year ago

We are also finding this issue. It is currently an inconsistent interaction as you can move items below an expanded item elsewhere in the tree (due to the 2 hover zones when there is an item beneath).

Figmas layers tree get's this right and might be a better reference than WinUI as it is very inconvenient to have to close the above tree to be able to drag something to the bottom, particularly when this interaction is possible elsewhere in the tree.

Export-1691408656865

lukasbach commented 9 months ago

I agree, but if the user drags at the bottom-most location, it can only target one possible target. The Figma example looks like a great solution, but also seems to approach the problem differently, by including the horizontal mouse position in the decision of where the item is dropped at. I honestly don't see that as that realistic for this library for a number of reasons, being that horizontal position is dependent on the item indendation, which is based on the custom render logic of the library consumer and might not be available to the library; the drop determination logic is already very complicated and will get even more hard to maintain if horizontal position is also included; and it would affect the logic for drop targets at other locations as well.

felixfaire commented 9 months ago

Whilst the figma-style horizontal interaction is certainly the ideal way to interact with a tree. The fact that react-complex-tree is capable of discerning between "Place below this open tree" and "Place inside this open tree" elsewhere in the tree (as long as there is at least element beneath) suggests a less complex solution may still be available. Would it be possible for example to add an invisible drop zone at the very bottom of the list such that the behaviour is more consistent with how interactions work elsewhere in the tree?

lukasbach commented 9 months ago

Hm yes that should be possible, there actually already is logic to catch drop events at the bottom of the tree, they are just interpreted as dropping on the bottom of the bottom-most item. I changed it to the behavior you suggested, you can try it out in this example: https://rct.lukasbach.com/storybook/?path=/story/core-basic-examples--drop-on-empty-tree

If you want to use this yourself, you need to make sure that the component rendered by renderTreeContainer actually fills the height of the full drop container, so that there is space at the bottom of the tree to drop at.

Is this solution closer to what you expect?

tonyketcham commented 9 months ago

In our case we're only allowing one type of item at the root level which can only exist at the root level, so this solution unfortunately does not work for us

Would it be possible to expose the item indent amount as a prop on the tree environment which either accepts a static number value for uniform depth indentation or a callback which calls with the current depth and DraggingPosition similar to the canDropAt callback?

lukasbach commented 9 months ago

@tonyketcham can you elaborate a bit on the prop you propose, I'm not sure I understand what you suggest it should do.

lukasbach commented 4 months ago

This has in the meantime been fixed as of v2.4.0. RCT now listens to the x-coordinate of the mouse to drop items on customizable depths, allowing users to drop in multiple different locations at the bottom of the tree if the open tree structure allows for that.

Big thanks to @tonyketcham and Modyfi, who sponsored the development of this feature, and with that supported the development of this library! ❤️