React-sortable-tree: 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.