Merge pull request #515 from thgreasi/joujiahe-master

fix(sortable): fix incorrect helper returned from getSortingHelper()
angular-ui · Apr 15, 2017 · cfcb636 · cfcb636
2 parents 18cc295 + a9d796f
commit cfcb636
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 6 deletions.
diff --git a/bower.json b/bower.json
@@ -1,6 +1,6 @@
 {
   "name": "angular-ui-sortable",
-  "version": "0.17.0",
+  "version": "0.17.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.17.0",
+  "version": "0.17.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
@@ -36,6 +36,7 @@ angular.module('ui.sortable', [])
         },
         link: function(scope, element, attrs, ngModel) {
           var savedNodes;
+          var helper;
 
           function combineCallbacks(first, second){
             var firstIsFunc = typeof first === 'function';
@@ -185,13 +186,12 @@ angular.module('ui.sortable', [])
             return helperOption === 'clone' || (typeof helperOption === 'function' && ui.item.sortable.isCustomHelperUsed());
           }
 
-          function getSortingHelper (element, ui, savedNodes) {
+          function getSortingHelper (element, ui/*, savedNodes*/) {
             var result = null;
             if (hasSortingHelper(element, ui) &&
                 element.sortable( 'option', 'appendTo' ) === 'parent') {
               // The .ui-sortable-helper element (that's the default class name)
-              // is placed last.
-              result = savedNodes.last();
+              result = helper;
             }
             return result;
           }
@@ -331,6 +331,7 @@ angular.module('ui.sortable', [])
               // This is inside activate (instead of start) in order to save
               // both lists when dragging between connected lists.
               savedNodes = savedNodesOrigin.contents();
+              helper = ui.helper;
 
               // If this list has a placeholder (the connected lists won't),
               // don't inlcude it in saved nodes.
@@ -428,9 +429,10 @@ angular.module('ui.sortable', [])
                 savedNodes.appendTo(elementContext.savedNodesOrigin);
               }
 
-              // It's now safe to clear the savedNodes
+              // It's now safe to clear the savedNodes and helper
               // since stop is the last callback.
               savedNodes = null;
+              helper = null;
             };
 
             callbacks.receive = function(e, ui) {

diff --git a/test/sortable.e2e.callbacks.spec.js b/test/sortable.e2e.callbacks.spec.js
@@ -156,6 +156,65 @@ describe('uiSortable', function() {
       });
     });
 
+    it('should cancel sorting of node "Two" when then helper is appended to the `body`', 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().appendTo('body');
+            },
+            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();
+        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 a list element is used', function() {
       inject(function($compile, $rootScope) {
         var element;