ui-sortable sort dom becomes out of sync

[ghost]

Hello,

I have noticed a few weird bugs with using ui-sortable component which I'm still in process of debugging:

1) when moving sortable items quickly which calls the moved" action to sync up the sort order to the original content array using the old and the next indexes, for whatever reason the sortable dom eventually becomes out of sync relative to the actual order of the original content, ie:. this may not be noticeable when moving items at lower frequency.

2) if attempting a sort, and then followed by deleting the item (which simply removes the item from the content array), the corresponding sortable dom element will not be removed. deleting items works fine if no sort is performed beforehand. likely required in this case to update the indexes on delete (data attributes), something like this may do:

        let index = 0;
        this.$().children().each(function(i) { 
          Ember.$(this).data('oldIndex', index);
          index++;
        });

this out of sync problem problems seems like it can be solved utilizing the ember run loop and syncing the items "oldIndex" data attribute on the elements, and in the case of delete also ui.item.remove() required, but I can't say for sure now.

[ToddSmithSalter]

Hey @apokinsocha. Sorry I haven't responded to this sooner. Yes, DOM syncing is an issue, with both the Sortable widget and a likely Ember.Array, who is sorted itself, both vying for control. I'm not sure I know the exact path to fix, though tying to the Ember run loop seems likely, as well as not letting Sortable have any control of the DOM, other than the action of moving the item from one place to another.

I recall another addon I'd used, which immediately dropped the change from the DOM, letting the component handle the update, as it should be. I'll do some digging, but would certainly appreciate insight, or if you wish to work on a PR together, I'm game.

[ToddSmithSalter]

I think this may help...

https://github.com/IvyApp/ivy-sortable/blob/master/addon/views/ivy-sortable.js#L119

[ghost]

Hi Todd,

You're correct. I can confirm that decoupling the DOM via sortable('cancel') is the right way to do it. I looked code from past project and I did it like that, so it should work. But updating the indexes I did a bit differently. Instead of updating the index of a single item, on sort change I sent all the indexes to update the items. Then I call sortable('cancel') right after, I guess on the next run loop. I never kept track of indexes (newIndex, oldIndex etc...) for the items within the sortable component. Would it make any sense to keep track of indexes within the component at all since all we really care about is the original content and let Ember handle the DOM? plus calling sortable('cancel') would render internal indexes via data attributes irrelevant, I think. I implemented this for your addon, but I think I still ran into some problems, I can't remember what as of now. I'll see about a PR for this if I can in the near future, but as of now I can't say when.

[ToddSmithSalter]

@apokinsocha I prefer to let jQuery UI Sortable handle the indexes, which is what we're passing to the moved action. I favour only updating the index of the one item and let the action pass the modified content array up, then have the modified data passed back down.

I'm my use case, which is different than yours, we store a position integer on each item. When we get the new index back via the moved action, we recalculate on the parent controller and update the content array, passing it back to the ui-sortable component. I'm quite sure we could recalculate if we had the entire modified content array back, but I prefer this way.

I'm open to extending the functionality by sending the updated content array as a fourth argument on the moved action. I think we'd need to keep a clone of the content array, since I don't think Sortable does.

For now, I'll push an update to cancel the DOM update on the stop and insert events.

[ToddSmithSalter]

I published a new version, 0.3.2, which adds cancelling, but it's a regression. I'll have to dig a little deeper in. Updating the content array alone isn't updating the DOM by itself. The component is not mutating when changes occur.