You may not call monitor.canDrop() inside your canDrop() implementation

[zourbuth]

In canDrop, before returning false, we want to update state to display notification about why it can't be dropped, but got this error:

Uncaught Error: You may not call monitor.canDrop() inside your canDrop() implementation. Read more: http://react-dnd.github.io/react-dnd/docs-drop-target-monitor.html

Is there a way to deal with it or may be a callback function?

[wuweiweiwu]

Hi @zourbuth Do you mind posting a repro? So I can better understand your issue

[zourbuth]

I've tested using Storybook example Drag from external source to prevent drop Baby Rabbit into Papa Rabbit, then shows message why it is prevented.

class UnwrappedApp extends Component {
  constructor(props) {
    super(props);
    this.dropMessage = this.dropMessage.bind(this);  
    this.state = {
      treeData: [{ title: 'Mama Rabbit' }, { title: 'Papa Rabbit' }],
      message: 'Sample message',
    };
  }
  
  dropMessage(object) {
    if ( object.nextParent && object.nextParent.title === 'Papa Rabbit' ) {
      this.setState({ message: 'Papa Rabbit is sleeping, play outside!' }); // --> ERROR HERE
      return false;    
    }
	
    return true;
  }
  
  render() {
    return (
      <div>
        <div className="message">{this.state.message}</div>
        <div style={{ height: 300 }}>
          <SortableTree
            treeData={this.state.treeData}
            onChange={treeData => this.setState({ treeData })}
            canDrop={object => this.dropMessage( object )}
            dndType={externalNodeType}
          />
        </div>
        <YourExternalNodeComponent node={{ title: 'Baby Rabbit' }} />
        ← drag this
      </div>
    );
  }
}

[wuweiweiwu]

@zourbuth this issue is confirmed and I think its due to how we bind this when we create DropTarget. I will have to investigate more later

Seems related to https://github.com/react-dnd/react-dnd/issues/265

I will consider possibly writing so canDrop to allow a callback. Or possibly passing the same object to onDragStateChanged. Seems to be a tricky issue

[spcfran]

+1 for being able to specify the reason why something can't be dropped - sometimes it can be the result of some complex logic rules and users could be frustrated not knowing why.

I'm not too convinced however about altering the state from inside a function named canDrop which is supposed to be pure and just return information.

What about something like allowing the canDrop callback to return not just a boolean but also an object like { canDrop: false, tip: "xxx" }, which could be rendered inside the red box?

  dropMessage(object) {
    if ( object.nextParent && object.nextParent.title === 'Papa Rabbit' ) {
      return { canDrop: false, tip: 'Papa Rabbit is sleeping, play outside!' };
    }
    else if ( /* some other condition */ ) {
      return { canDrop: true, tip: 'Oh yes drop me here please' };
    }
    return true;
  }

[wuweiweiwu]

I was thinking allowing the canDrop callback to have access to the node and/or location it is going to be dropped as parameters, thus the user can use that information to render a message or whatever they want to do

[buzztone]

#261 appears to cover a similar issue that could be addressed by these solutions.

[zourbuth]

@wuweiweiwu we would like to help, but don't know where to start, what piece of code need to modify. Perhaps you can point it to us.

[wuweiweiwu]

hi @zourbuth that would be much appreciated! https://github.com/frontend-collective/react-sortable-tree/blob/master/src/utils/dnd-manager.js#L146 is where the this.props.canDrop is called.

This would requiring allowing the user specifying a callback parameter in canDrop can calling it with the required parameters (ex: node, location)

[zourbuth]

Hi @wuweiweiwu, in this case, I didn't see the line as the problem. I found the line https://github.com/frontend-collective/react-sortable-tree/blob/master/src/utils/dnd-manager.js#L252 which calls canDrop inside canDrop prop.

function nodeDropTargetPropInjection(connect, monitor) {
  const dragged = monitor.getItem();
  return {
    connectDropTarget: connect.dropTarget(),
    isOver: monitor.isOver(),
    canDrop: monitor.canDrop(), // <--- calls canDrop inside canDrop
    draggedNode: dragged ? dragged.node : null,
  };
}

I understand the DnD collect parameter above that passed canDrop is used for cancel pad class in placeholder-renderer-default.js and node-renderer-default.js.

If we remove canDrop from the collect parameter, it works, the message is updated, but always shows cancel pad instead of landing pad.

Please suggest.

[wuweiweiwu]

@zourbuth I was thinking of adding a callback, however, in hindsight, it probably is not a good long term fix. I will be updating our react-dnd dependency soon #87 (it is a major change) So I will revisit this issue once that is done.

Thanks!

[zourbuth]

Hello @wuweiweiwu.

Any update with this issue?

Thanks.

[kohsah]

I did a hacky workaround for this issue, since showing a rejection notification in canDrop() was triggered multiple times, so i got dozens of rejection notifications when only one was neccessary.

so i have a variable in state which just records the last success or failure of canDrop.

then in onDragStateChanged if its not isDragging i check the variable from state, and display the error notif, if it was a failure. For a complete drag action, onDragStateChanged is triggered once so it works.

[wsz7777]

have some update ?

[mak009]

I m also facing the same issue. CanDrop() called multiple times while we drop the external node into tree. Please suggest some workaround.

[eugenrecaj]

generateNodeProps={() => ({
  onDragEnd: () => {
    // Works if canDrop returns false
    // Add code here
  },
})};

I think until this issue is resolved this workaround might be a good solution.