BUG: CanvasWrapper's onDrop event assumes canvas is positioned in the top left of the document.

[phlickey]

Right now the onDrop event handler assumes determines the position to place a node when the on drop event fires based on the clientX and clientY properties of the drop event, correcting for the Chart's offset.

This works when the canvas is in the top left hand corner of the document. However, when the Canvas is positioned to the right, or down, the clientX and clientY values do not correspond to the position of a users mouse when the drop event occurred.

Proposing making the following change to the onDrop method that gets passed to ComponentInner in Canvas.wrapper.tsx

onDrop={ (e) => {
  const data = JSON.parse(e.dataTransfer.getData(REACT_FLOW_CHART))
  let elementOffset;
  if (this.ref.current){
    elementOffset = {
      x: this.ref.current.offsetLeft,
      y: this.ref.current.offsetTop
    }
  }else{
    elementOffset = {x: 0, y: 0}
  }
  onCanvasDrop({ data, position: {
    x: e.clientX - (position.x + elementOffset.x),
    y: e.clientY - (position.y + elementOffset.y),
  }})
} }

this would get the actual offset of the HTML element on the page and include this when calculating where the node should drop.

Does anyone see any issues with this? Should I make a PR?

[phlickey]

On further testing, offsetLeft and offsetTop probably aren't sufficient.

Would probably need to be something like:

let elementOffset;
  if (this.ref.current){
    let {x,y} = this.ref.getBoundingClientRect()
    elementOffset = { x, y }
  }else{
    elementOffset = {x: 0, y: 0}
  }

Is there a better place that this offset could be calculated and stored somewhere stateful? Does it make sense to add it to the resize observer?

[mayansalama]

This has been bugging me as well - would be great to get this sorted.

[phlickey]

Looks like the offset is already being stored in state, and updated through the resize observer, just not being used when calculating the position property of the new node created on the onDrop event.

Made a PR that adds it in, since it seems pretty cut and dry and unlikely to affect anything else
#34