Add node Variable moved. And fire the nodeReleased Event

[Trackhe]

Add node Variable moved. And fire the nodeReleased Event on every release, not only if the Node was not moved. If you want to check if it was released without movement, check it with:

	function nodeReleased(e: CustomEvent) {
		console.log(e.type);
		console.log("moved: ", get(e.detail.node.moved));
	}

I created an example too. routes/events, open the javascript console and try it out.

[Trackhe]

I want to add this because, i want to update the position in DB of the node after the movement.

For supporting the old functionality, nodeReleased without movement (that was the the functionality before my change) i added the moved value. With that can be checked if it moved an in addition to that, in what direction and how far it get moved.

[briangregoryholmes]

I'll take a look at this over the weekend. At the moment, I think my preference would be to add a separate on:nodeMoved event, but I'll see what it's like with this approach and get back to you.

[Trackhe]

I understand you but less events are maybe better, and the original motivation to change it, was that released was not fired if the node was moved.

[Trackhe]

I added a way to delete an Edge with the edgeClick function. With this way it makes much more things Possible. I use it for my permission editor that changes directly all the stuff in Database.

<script>
	import { Edge } from "svelvet"

	function deleteConnection(edge){
		console.log(edge)
//delete node
edge.source.node.id
edge.target.node.id
	}




</script>

<Edge let:path let:destroy on:hover let:edge enableHover enableDestroyOnClick edgeClick={deleteConnection}><!---->
	<path class="edge" d={path} />
	<button on:click={() => {
		deleteConnection(edge)
		destroy()}} slot="label">
		<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" ><path class="edge-button-svg" fill="currentColor" d="M19 4h-3.5l-1-1h-5l-1 1H5v2h14M6 19a2 2 0 0 0 2 2h8a2 2 0 0 0 2-2V7H6v12Z"/></svg>
	</button>
</Edge>

<style>

  :global(.target), .edge, button > svg > path{
      transition: opacity 0.1s ease-out;
	}

	:global(.edges-wrapper):hover{
			z-index:1 !important;
	}

  :global(.edges-wrapper):hover .edge{
      stroke: #ff2e2e;
	}

  :global(.edges-wrapper):hover :global(.target){
      opacity: 60% !important;
      --prop-target-edge-color: black !important;
  }

  :global(.edges-wrapper):hover button path{
      opacity: 80%;
  }

  .edge {
      stroke: white;
      stroke-width: 2px;
  }

  :global(.target){
      stroke-width: 5px !important;
	}

  button > svg > path {
			opacity: 0;
      color: white;
      stroke: white !important;
      stroke-width: 0;
  }

  button {
      cursor: pointer;
      height: 2em;
      width: 2em;
  }
</style>

[briangregoryholmes]

I understand and appreciate what you're trying to achieve @Trackhe, but don't think this is the right pattern. I believe that there is an intuitive way that developers expect this to behave based on analogous native browser events and the expectations of end users. I think the naming in the current library has been poorly chosen (by me) and I believe additional events are needed, but I want to stick as closely as possible to familiar, intuitive patterns and believe the decision to have this particular event fire only when the Node has not moved is the correct one.

To explain, consider the following:

<div class="wrapper">
  <div
    class="element"
    draggable="true"
    on:dragstart={() => console.log('start')}
    on:drag={() => console.log('drag')}
    on:dragend={() => console.log('end')}
    on:mouseup={() => console.log('mouse up')}
    on:mousedown={() => console.log('mouse down')}
  />
</div>

on:nodeReleased is essentially equivalent to the native on:mouseup event. You'll find that, in the above case, on:mouseup does not fire after on:dragstart has fired. This is true even when the drag delta is small enough that the cursor is still over the element. I believe this behavior makes sense and was chosen to make the most common modes of interaction simpler.

For the sake of argument, consider something as simple as a draggable button. Obviously, there is some functionality that should be triggered when the button is clicked. Let's say the button changes from red to blue. If the button can also be moved around the page via some kind of drag interaction, I believe the vast majority of users would expect the color of the button to remain unchanged after repositioning it. Yes, they clicked the button and then released, but their intention was not to "click" the button, it was to move it.

My point is that 99 times out of 100, any event you want to trigger on:mouseup is one you likely don't want to trigger if the mouse up happens after the element has been moved. Because of that, I believe the pattern found in the library should enable that method of interaction first and foremost. That is to say, we should have an event that fires on:mouseup, but only when the Node hasn't moved. That is what we currently have.

If we were to implement the method proposed in this PR, the vast majority of functions triggered by this event would require a conditional check as the first line of the function to ensure the Node has not moved. Instead, we should add an additional event, analogous to on:dragend, that developers can use to trigger specific functions based on that event. If they'd like some function to fire every time a mouseup event occurs regardless of if the Node has moved, they can simply add both events. This feels like a good compromise because it makes the most common pattern easier and a less common pattern slightly more inconvenient (rather than the other way around).

I'll also argue that more events are the preferred, standard API. You don't just have on:click with a conditional property that says whether or not the mouse is currently down, you have on:click, on:mousedown and on:mouseup. Developers expect a larger number of well named events and not fewer events with conditional details/properties.

That said, thank you very much for the work put into this PR. I'd be happy to continue to discuss the changes around this space and review any PRs in the future.

[BOJIT]

I'd definitely be keen for an on:dragend (or similar) as a separate event. For example, I'm currently using on:nodeReleased to launch a context menu, and binding to the position attribute to get position updates. However, performing my application updates on every change of the position store isn't ideal. Having an extra event that fires after the drag is finished would be quite nice :).

Also, this event would be very handy for implementing undo/redo as described in #426.

[Trackhe]

nodeReleased

If I have time, I will make the code changes and split it into two events. However, even with these changes, the "nodeReleased" event should still fire every time the node is released, regardless of its positioning, as that could be misleading.

In my opinion is the renaming of the original event necessary. I would prefer here to get rid of the "nodeClicked" and "nodeReleased" and name it, as you already mentioned "on:click, on:mousedown and on:mouseup" but i m open for other and at least this is your project.

like what are you saying

<div class="wrapper">
  <div
    class="element"
    draggable="true"
    on:dragstart={() => console.log('start')}
    on:drag={() => console.log('drag')}
    on:dragend={() => console.log('end')}
    on:mouseup={() => console.log('mouse up')}
    on:mousedown={() => console.log('mouse down')}
  />
</div>

if i found time i implement it like this.