Opened 5 years ago

Closed 5 years ago

#10800 closed bug (duplicate)

Sortable: Cancelling a sort does not return the item to its original position with regard to non-element nodes

Reported by: morchard Owned by:
Priority: minor Milestone: none
Component: ui.sortable Version: 1.11.2
Keywords: Cc:
Blocked by: Blocking:

Description

When sorting starts, sortable records the original DOM position by storing the current item's previous item and the current item's parent.

_mouseStart: function() {
    ...

    this.domPosition = { prev: this.currentItem.prev()[0], parent: this.currentItem.parent()[0] };

    ...
}

If sorting is cancelled these are used to place the item back to its original position.

cancel: function() {
    ...

    if(this.domPosition.prev) {
        $(this.domPosition.prev).after(this.currentItem);
    } else {
        $(this.domPosition.parent).prepend(this.currentItem);
    }

    ...
}

However, because of the use of prev when storing the previous item, any other non-element nodes between the current item and the previous item are ignored. This means that when returning the current item to its original position, it will actually be placed before those non-element nodes, rather than after them.

This is an isolated JSFiddle showing the bug occurring in a simple case: http://jsfiddle.net/morchard/pups17xL

I think it's quite common to have text nodes between items, especially white-space, and if anything relies on the order of these nodes then it will break.

Our case is that the items are rendered using a KnockOut foreach binding, and that binding records the surrounding nodes of each item to determine if anything has changed when it is re-evaluated. After cancelling a sort, the nodes are no longer in the correct order, despite the notion that nothing has changed, and this causes it to re-evaluate incorrectly.

Change History (2)

comment:1 Changed 5 years ago by morchard

I've created a PR for a fix for this bug here: https://github.com/jquery/jquery-ui/pull/1433

And created a JSFiddle showing the fix in action here: http://jsfiddle.net/morchard/z60ox3so/

comment:2 Changed 5 years ago by Scott González

Resolution: duplicate
Status: newclosed

Duplicate of #9727.

Note: See TracTickets for help on using tickets.