lukasbach / react-complex-tree

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

Drag a folder/a file into a file cause the folder to disappear #228

Closed hihahihahoho closed 1 year ago

hihahihahoho commented 1 year ago

Describe the bug Drag a folder/a file into a file cause the folder to disappear

To Reproduce Drag a folder/a file into a file

Expected behavior The folder/file should be the child of the file

Desktop (please complete the following information):

Additional context Add any other context about the problem here.

lukasbach commented 1 year ago

Which version of react-complex-tree are you on? Because I fixed a very similar problem (#277) just a few hours before your report. If latest (2.1.1), can you reproduce the issue in one of the storybook instances, and describe on which story and which items you are dropping where?

hihahihahoho commented 1 year ago

Which version of react-complex-tree are you on? Because I fixed a very similar problem (#277) just a few hours before your report. If latest (2.1.1), can you reproduce the issue in one of the storybook instances, and describe on which story and which items you are dropping where?

I'm on the newest version (2.1.1), the link i'm using the demo: https://rct.lukasbach.com/storybook/?path=/story/core-drag-and-drop-configurability--only,

and the reproduce clip:

https://user-images.githubusercontent.com/29968144/222028522-45a9244a-bff4-4700-b2c3-9410a14f0e7a.mp4

-drop-on-items-without-children

lukasbach commented 1 year ago

In this Demo, canDropOnNonFolder is enabled, which means that you can drop an item on something that is not a folder, but that on its own won't turn it into a folder. Which behavior are you expecting?

hihahihahoho commented 1 year ago

In this Demo, canDropOnNonFolder is enabled, which means that you can drop an item on something that is not a folder, but that on its own won't turn it into a folder. Which behavior are you expecting?

  • If you want users to drop only on folders, enable canDropOnFolder instead of canDropOnNonFolder, and also canReorderItems if you want to allow reordering
  • If you want to have items turn into folders with children when you drop an item on it, use a class as data provider that implements TreeDataProvider.onChangeItemChildren to handle the new items by turning the item into a folder and notifying rct of changes with the listeners passed by TreeDataProvider.onDidChangeTreeData. Or implement that accordingly with StaticTreeDataProvider or an controlled environment by handling that yourself.
  • Similarly, if you want other special handling of items that are dropped on other non-folder items, but not turn the target into a folder, handle the drop event just as above, but for this case it is very much necessary for rct not to impose implicit changes like the drop target automatically turning into a folder.

i mean the expected behavior should be create make the file become a folder if you drag another item in it. Anyway if the item disappear then it should be considered a bug. I understand your point of view but if you decided to the keep it as a non bug, but i think you should update the behavior on the doc

lukasbach commented 1 year ago

The expectation differs very much of the consumers semantic of an "item". The fact that the drop target magically turns into a folder is logic that regards the tree structure, and this library purposefully focuses mostly on the rendering of the tree, not on the logic that regards the data structure. Making impositions on tree structure logic is intentionally left out of the library to support as many use cases as possible. The page linked above is a storybook instance, not a doc page, the goal here is to demonstrate some simple features the library brings, and I think this demo gives a good overview of what functionality the library provides, and what not.

To maybe demonstrate that this behaviour could very well be intentional, the Outlook Calendar view works in a very similar way (please disregard the unread mail count lol): outlook-dragging While usually you would expect emails being items, and mail folders being folders, in this view, an folder is a mail folder with other folders as children, and an item is also a mail folder but without children. Dropping an email on an item doesn't expand it to be an folder, as long as that folder only contains emails, not other folders. It is a more complex behaviour than just "everything that can receive drops will be turned into a folder", and such usecases are not possible when making strict assumptions on the drop behaviour.

hihahihahoho commented 1 year ago

Very fair point, but i think a doc updating doesn't hurt, maybe an example for that, it's up to you, feel free to close the issue :)

lukasbach commented 1 year ago

Btw I've started to collect items for an FAQ and added this to there to make it a bit more visible :) https://rct.lukasbach.com/docs/faq

AustinMKerr commented 4 months ago

this is how you fix this for anyone else who finds this. notice specifically at onChangeItemChildren

const dataProvider = useMemo( () => { class CustomDataProviderImplementation { data = convertedItems;

            treeChangeListeners = [];

            async getTreeItem(itemId) {

                const node = convertedItems[itemId]
                console.log({itemId,node});
                return node
            }

            async onChangeItemChildren(itemId, newChildren) {
                // console.log('onChangeItemChildren',{itemId,newChildren},this.data[itemId]);
                this.data[itemId].children = newChildren;
                this.data[itemId].isFolder = newChildren.length > 0;
                this.treeChangeListeners.forEach(listener => listener([itemId]));
            }

            onDidChangeTreeData(listener) {
                // console.log('onDidChangeTreeData',{listener});
                this.treeChangeListeners.push(listener);
                return {
                    dispose: () =>
                        this.treeChangeListeners.splice(
                            this.treeChangeListeners.indexOf(listener),
                            1
                        ),
                };
            }

            async onRenameItem(item, name) {
                this.data[item.index].data = name;
            }

            injectItem(name) {
                const rand = `${Math.random()}`;
                this.data[rand] = { data: name, index: rand };
                this.data.root.children.push(rand);
                this.treeChangeListeners.forEach(listener => listener(['root']));
            }

        }
        const customData = new CustomDataProviderImplementation()
        return customData
    },
    [convertedItems]
);