-
Notifications
You must be signed in to change notification settings - Fork 354
uiSortable not compatible with option "connectWith" #325
Description
The problem with uiSortable is that it assumes the update is a rearrangement of the group of elements, but in case of connectWith, the update could be a removal/insertion of an item, so re-sorting is not appropriate.
So we have at three cases for update: we are rearranging the list, we are removing an item from the list, or we are inserting an item into the list. The latter two cases can be handled by receive (since that implies a remove somewhere else). The first one can be handled inside update by checking if the parent element of ui.item is the same on start and update (a bit awkward but seems to work). Here is the updated onStart, onUpdate, and the added onReceive methods:
onStart = (e, ui) ->
# Save position of dragged item, original model, and original item parent
ui.item.data('ui-sortable-start', ui.item.index())
ui.item.data('ui-sortable-start-model', ngModel)
ui.item.data('ui-sortable-start-parent', ui.item.context.parentElement)
onUpdate = (e, ui) ->
# Fetch saved and current position of dropped element
start = ui.item.data('ui-sortable-start')
end = ui.item.index()
# Only sort if the original and target parent elements are the same
startParent = ui.item.data('ui-sortable-start-parent')
if startParent is ui.item.context.parentElement
# Reorder array and apply change to scope
ngModel.$modelValue.splice(end, 0, ngModel.$modelValue.splice(start, 1)[0])
onReceive = (e, ui) ->
start = ui.item.data('ui-sortable-start')
end = ui.item.index()
startModel = ui.item.data('ui-sortable-start-model')
# Remove the element from the start array and insert it in this one in the correct order
ngModel.$modelValue.splice(end, 0, startModel.$modelValue.splice(start, 1)[0])Note that I also removed the scope.$apply() statement in onUpdate which seemed to serve no purpose.
I haven't written any tests yet (there weren't any good tests for the original uiSortable to begin with, so this is definitely an import TODO for this module) but I've done a fair bit of ad-hoc testing and it seems to work as intended.
Any thoughts? Worth a pull request?