Dropping into self

[impactvelocity]

Describe the bug You can drag an item into it self if it has a child that accepts drop.

https://p195.p4.n0.cdn.getcloudapp.com/items/geud4ggR/d26a0b2d-f670-4252-8347-ef92a302968b.gif?v=907ccf16be512a91e317756cf823cf47

Expected Disallow this sort of thing.

canDropAt={(items, target) => {

// prevent self drop
if(items[0].index == target.parentItem) return false

            // check if target is a child of parent
            const isSelf = itemsSource[items[0].index].children.findIndex(i => i == target.parentItem)
            if(isSelf !== -1) return false
          
return true
 } 
}

That is some basic code to check. If the fist item is the same as the a parent. I also check if parentItem is a child of the first item checking the items. (I called it itemsSource).

This isn't great and doesn't check for multiple selects. But it kind of works. :)

[janmejayspurohit]

+1

[lukasbach]

Hi, thanks for reporting! Just as an heads up, I've looked into it, and it is not quite as trivial to fix; since checking whether someone is dropping an item into itself is not a trivial operation (can be a high depth through which is being dropped, and arbitrarily many items can be dragged at once, so the check is required for every dragged item), this will require some calculation. And, currently, the drop check happens on the drag event, not the drop event (so that the library can already prevent dropping on not allowed items), so this might not scale well for bigger trees. I'm currently working on improving the overall performance of the library, and then plan to only do the check during the initial drag event, hopefully in a way that only requires linear effort. Until then, a workaround similar to what @impactvelocity suggested is probably the easiest way to go. I'll keep you updated on the progress!

[C7X]

Hi @lukasbach, great work so far! I’m so glad to hear that you’re working on performance right now. We’re currently trying to use your library for a drag and drop tree that has thousands of nodes and we’re finding that it slows down to the point of not being usable when we try to drag and drop something. Will your performance upgrades make it possible for us to use this library for trees that have thousands of nodes?

[lukasbach]

Hi @C7X! When working with lists with thousands of entries, list virtualization starts to become inevitable. This is something I want to support with react-complex-tree in the future, but there is still a bigger road ahead for that and that will not come with the performance fixes I am working on right now, sorry. So for now, I unfortunately have to say that other libraries might be a better starting point until rct supports virtualization.

[rozzzly]

I was able to solve this quite easily, I'm not sure about performance but if I recall my time complexity notation correctly, it should be O(n). I wouldn't imagine this would be a performance hit unless you had 1k+ leaf nodes individually selected (not their parents)

Here's my approach in brief:

• my items double linked so they maintain the ID of their parent (if any) as well as the IDs of any children and my typedef looks like this:

  interface Item {
      _id: string;
      name: string;
      parent?: string; // if no parent, Item is the root node
      children: string[]
  }	
  

• I prepare them for react-complex-tree like this

  export interface TreeListProps {
      initialItems: Item[]
  }
  export const TreeList: React:FC<TreeListProps> = ({ initialItems ]) => {
      const { treeItems, rootID } = useMemo(() => {
          let rootID = '';
          const result: Record<string, TreeItem<any>> = {};
          for (const item of initialItems) {
              if (!item.parent) rootID = item._id;
              result[item[idKey]] = {
                  data: { ...item },
                  index: item._id,
                  children: item.children,
                  hasChildren: item.children.length > 0,
                  canMove: true
              };
          }
          if (rootID === '') throw new Error('could not find root item!')
          return { treeItems: result, rootID };
      }, []);
  
      // ... rest of component
  };
  

• Next I created a custom TreeDataProvider. I just copypasted StaticTreeDataProvider but changed the onChangeItemChildren method to keep parent up to date

  public async onChangeItemChildren(parentID: TreeItemIndex, nChildIDs: TreeItemIndex[]): Promise<void> {
      // update parent node
      const parent = this.data.items[parentID];
      parent.children = nChildIDs;
      if (nChildIDs.length) {
          parent.hasChildren = false;
      } else {
          parent.hasChildren = true;
      }
      parent.data.children = nChildIDs;
  
      // re-parent child nodes
      for (const childID of nChildIDs) {
          const child = this.data.items[childID];
          if (child.data.parent !== parentID) { // this child's parent has changed
              console.log(
                  `${child.data.name} changed from [${this.data.items[child.data.parent].data.name}] => [${parent.data.name}]`
              );
              child.data.parent = parentID;
          }
      }
      this.onDidChangeTreeDataEmitter.emit([parentID]);
  }
  

• the data provider is initialized like so

      const  myCustomDataProvider = useMemo(() => (
          new CustomTreeDataProvider(treeItems, (item, name) => ({ ...item, data: {...item.data, tagName: name }}));
      ), [ treeItems ]);
  

• now that refs to parent are tracked, it is simple to check if user is attempting to drop a node into one of its descendants

      <UncontrolledTreeEnvironment
          // ... other props
          dataProvider={myCustomDataProvider}
          canDropAt={(items, target) => {
              let parentID = target.parentItem;
              // check to ensure dragged node(s) is(are) not trying to drop into one of its(their) own descendants
              while (parentID) { // go up until we reach the root node
                  if (items.some(item => item.index === parentID)) {
                      console.log('can\'t drop a node into its own descendant');
                      return false;
                  } else {
                      parentID = treeItems[parentID].data.parent;
                  }
              }
              return true;
          }}
          // .. more props
      >
          // ...
      </UncontrolledTreeEnvironment>
  

So that's all the logic I needed to achieve the desired behavior. At ~100 items, I haven't seen any performance issues. My attention span isn't long enough to setup a stress test. If some tries this and hits a wall at 1k selected items or something, I'd be interested to hear about it. I imagine debounce/throttle on canDropAt would help against very large selections although you would probably need to have some logic to test parameters so the return value would always be valid for the given selection/target.

[lukasbach]

Going through issues, I believe that this issue has been fixed quite a few versions ago, where the library now checks if a drop target is a viable target that does not result in an item being dropped into itself. I will close this, please reopen or create a follow up if I missed something.