Fix drag events not moving indicator when staying in the same node #222
Conversation
|
✔️ Deploy Preview for craftjs ready! 🔨 Explore the source changes: 97cc9bc 🔍 Inspect the deploy log: https://app.netlify.com/sites/craftjs/deploys/610169c9ac30650008791d69 😎 Browse the preview: https://deploy-preview-222--craftjs.netlify.app |
| onDragOver(x: number, y: number, targetId: NodeId) { | ||
| const draggedElement = DefaultEventHandlers.draggedElement; | ||
| if (!draggedElement) { | ||
| return; | ||
| } | ||
|
|
||
| if (targetId) { | ||
| const node = this.store.query.node(targetId).get(); | ||
| if (!node) { | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| let node = (draggedElement as unknown) as Node; | ||
|
|
||
| if ((draggedElement as NodeTree).rootNodeId) { | ||
| const nodeTree = draggedElement as NodeTree; | ||
| node = nodeTree.nodes[nodeTree.rootNodeId]; | ||
| } | ||
|
|
||
| const indicator = this.store.query.getDropPlaceholder(node, targetId, { | ||
| x, | ||
| y, | ||
| }); | ||
|
|
||
| if (!indicator) { | ||
| return; | ||
| } | ||
| this.store.actions.setIndicator(indicator); | ||
| DefaultEventHandlers.indicator = indicator; | ||
| } |
There was a problem hiding this comment.
This part seems to be the same as the dragenter handler used in the drop connector.
Can we rename this function to something like computeIndicator, so we can use for both the dragover and dragenter handler?
On that note - perhaps we don't even need the dragenter handler anymore, since this updated dragover handler is doing the same thing?
|
Great work @nicoladefranceschi this is looks like its solving a core bug that has been nagging us for a while any chance of resolving this confilets before the next release? |
|
@hugominas sorry I'm quite busy right now, maybe someone else can take over and make the necessary changes so that this can be merged? |
|
I think I fixed the merge conflicts and added @prevwong suggestion to rename But... I cannot tell the difference between the before https://craft.js.org and the new behavior. Did I mess up while resolving the merge conflict? |
|
Hi @ankri thank you! To be honest after testing this PR locally I did not find any noticeable difference. I was facing a drag and dropping issue caused by the Elements not having enough space between them and I only realised this after. Probably @nicoladefranceschi can help us. |
This pull request fixes a problem when dragging nodes.
Drag events are not moving indicator when the mouse is staying in the same node.
This gifs explain it better:
If the mouse moves on different nodes, it works as expected:
If you move the mouse staying in the same node, it doesn't work:
