Merge pull request #493 from thgreasi/helperCancelX3

fix(sortable): properly restore DOM after cancel()
angular-ui · Nov 28, 2016 · 57c5021 · 57c5021
2 parents da521c0 + 53a31b3
commit 57c5021
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 17 deletions.
diff --git a/bower.json b/bower.json
@@ -1,6 +1,6 @@
 {
   "name": "angular-ui-sortable",
-  "version": "0.15.0",
+  "version": "0.15.1",
   "description": "This directive allows you to jQueryUI Sortable.",
   "author": "https://github.com/angular-ui/ui-sortable/graphs/contributors",
   "license": "MIT",

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "angular-ui-sortable",
-  "version": "0.15.0",
+  "version": "0.15.1",
   "description": "This directive allows you to jQueryUI Sortable.",
   "author": "https://github.com/angular-ui/ui-sortable/graphs/contributors",
   "license": "MIT",

diff --git a/src/sortable.js b/src/sortable.js
@@ -369,29 +369,30 @@ angular.module('ui.sortable', [])
               // If the received flag hasn't be set on the item, this is a
               // normal sort, if dropindex is set, the item was moved, so move
               // the items in the list.
-              if(!ui.item.sortable.received &&
-                 ('dropindex' in ui.item.sortable) &&
-                 !ui.item.sortable.isCanceled()) {
+              var wasMoved = ('dropindex' in ui.item.sortable) &&
+                              !ui.item.sortable.isCanceled();
+
+              if (wasMoved && !ui.item.sortable.received) {
 
                 scope.$apply(function () {
                   ngModel.$modelValue.splice(
                     ui.item.sortable.dropindex, 0,
                     ngModel.$modelValue.splice(ui.item.sortable.index, 1)[0]);
                 });
-              } else {
-                // if the item was not moved, then restore the elements
+              } else if (!wasMoved &&
+                         !angular.equals(element.contents().toArray(), savedNodes.toArray())) {
+                // if the item was not moved
+                // and the DOM element order has changed,
+                // then restore the elements
                 // so that the ngRepeat's comment are correct.
-                if ((!('dropindex' in ui.item.sortable) || ui.item.sortable.isCanceled()) &&
-                    !angular.equals(element.contents(), savedNodes)) {
-
-                  var sortingHelper = getSortingHelper(element, ui, savedNodes);
-                  if (sortingHelper && sortingHelper.length) {
-                    // Restore all the savedNodes except from the sorting helper element.
-                    // That way it will be garbage collected.
-                    savedNodes = savedNodes.not(sortingHelper);
-                  }
-                  savedNodes.appendTo(element);
+
+                var sortingHelper = getSortingHelper(element, ui, savedNodes);
+                if (sortingHelper && sortingHelper.length) {
+                  // Restore all the savedNodes except from the sorting helper element.
+                  // That way it will be garbage collected.
+                  savedNodes = savedNodes.not(sortingHelper);
                 }
+                savedNodes.appendTo(element);
               }
 
               // It's now safe to clear the savedNodes

diff --git a/test/sortable.e2e.callbacks.spec.js b/test/sortable.e2e.callbacks.spec.js
@@ -68,6 +68,77 @@ describe('uiSortable', function() {
         li.simulate('drag', { dy: dy });
         expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
         expect($rootScope.items).toEqual(listContent(element));
+        // try again
+        li = element.find('[ng-repeat]:eq(1)');
+        dy = (1 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
+        expect($rootScope.items).toEqual(listContent(element));
+        // try again
+        li = element.find('[ng-repeat]:eq(1)');
+        dy = (1 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
+        expect($rootScope.items).toEqual(listContent(element));
+
+        li = element.find('[ng-repeat]:eq(0)');
+        dy = (2 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['Two', 'Three', 'One']);
+        expect($rootScope.items).toEqual(listContent(element));
+
+        li = element.find('[ng-repeat]:eq(2)');
+        dy = -(2 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
+        expect($rootScope.items).toEqual(listContent(element));
+
+        $(element).remove();
+      });
+    });
+
+    it('should cancel sorting of node "Two" and "helper: function" that returns an element is used', function() {
+      inject(function($compile, $rootScope) {
+        var element;
+        element = $compile(''.concat(
+          '<ul ui-sortable="opts" ng-model="items">',
+          beforeLiElement,
+          '<li ng-repeat="item in items" id="s-{{$index}}">{{ item }}</li>',
+          afterLiElement +
+          '</ul>'))($rootScope);
+        $rootScope.$apply(function() {
+          $rootScope.opts = {
+            helper: function (e, item) {
+              return item.clone();
+            },
+            update: function(e, ui) {
+              if (ui.item.sortable.model === 'Two') {
+                ui.item.sortable.cancel();
+              }
+            }
+          };
+          $rootScope.items = ['One', 'Two', 'Three'];
+        });
+
+        host.append(element);
+
+        var li = element.find('[ng-repeat]:eq(1)');
+        var dy = (1 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
+        expect($rootScope.items).toEqual(listContent(element));
+        // try again
+        li = element.find('[ng-repeat]:eq(1)');
+        dy = (1 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
+        expect($rootScope.items).toEqual(listContent(element));
+        // try again
+        li = element.find('[ng-repeat]:eq(1)');
+        dy = (1 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
+        expect($rootScope.items).toEqual(listContent(element));
 
         li = element.find('[ng-repeat]:eq(0)');
         dy = (2 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
@@ -115,6 +186,18 @@ describe('uiSortable', function() {
         li.simulate('drag', { dy: dy });
         expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
         expect($rootScope.items).toEqual(listContent(element));
+        // try again
+        li = element.find('[ng-repeat]:eq(1)');
+        dy = (1 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
+        expect($rootScope.items).toEqual(listContent(element));
+        // try again
+        li = element.find('[ng-repeat]:eq(1)');
+        dy = (1 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
+        li.simulate('drag', { dy: dy });
+        expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
+        expect($rootScope.items).toEqual(listContent(element));
 
         li = element.find('[ng-repeat]:eq(0)');
         dy = (2 + EXTRA_DY_PERCENTAGE) * li.outerHeight();