SortableJS / Sortable

Reorderable drag-and-drop lists for modern browsers and touch devices. No jQuery or framework required.
https://sortablejs.github.io/Sortable/
MIT License
29.61k stars 3.7k forks source link

[bug] Issues with _ghostIsLast #2334

Closed KnSup closed 9 months ago

KnSup commented 10 months ago

When I was using the latest version, I found a problem. When I dragged from one container to another, I found that I couldn't add elements at the end. When I debugged it, I found that its calculation method is that if it's not vertical, it will judge that it's larger than the container's bottom by 10 before judging it as an addition. This will cause a problem, that is, if my container is 100%, no matter how you calculate, it will not exceed the value of the container's bottom, which will result in _ghostIsLast returning false.

The problem I described is the code in the following bold section: evt.clientY > sortableContentRect.bottom + spacer

  function _ghostIsLast(evt, vertical, sortable) {
    var lastElRect = getRect(lastChild(sortable.el, sortable.options.draggable));
    var sortableContentRect = getContentRect(sortable.el);
    var spacer = 10;
    return vertical ? evt.clientX > sortableContentRect.right + spacer || evt.clientY > lastElRect.bottom && evt.clientX > lastElRect.left : evt.clientY > sortableContentRect.bottom + spacer || evt.clientX > lastElRect.right && evt.clientY > lastElRect.top;
  }

I believe that it may be necessary to judge the last element's "buttom". If it exceeds, it is considered as the last one.

owen-m1 commented 10 months ago

There are other problems caused by using the last element for this, for example if your last element had a smaller height than the other elements, as soon as you try to drag "below" the last element it will make it jump to the end.

It should work if you add padding-bottom to your list. The sortableContentRect does not include padding. But there is nothing else since we don't detect dragging outside the list itself.

owen-m1 commented 9 months ago

This is fixed in 1.15.2, from now on it uses the aggregate rect formed by the child elements rather than the content rect