From ba1859b308b0769e4af2e72cb229cb7a87ade0e3 Mon Sep 17 00:00:00 2001 From: Andrew Date: Sun, 6 Jul 2014 13:39:56 -0600 Subject: [PATCH] fix(ionReorderButton): stop `ngRepeat:dupes` error when reordering Closes #1601. BREAKING CHANGE: Reordering with ion-reorder-button no longer changes the order of the items in the DOM. This change will only break your list if you were not using the onReorder callback as described in the documentation. Before, while reordering an element in a list Ionic would swap the elements underneath as the reordering happened. This sometimes caused errors with angular's ngRepeat directive. Now, reordering an element in a list does not change the order of elements in the DOM. It is expected that the end developer will use the index changes given in the `onReorder` callback to reorder the items in the list. This is simple to do, see the [examples in the ionReorderButton documentation](http://ionicframework.com/docs/api/directive/ionReorderButton/). --- js/angular/directive/itemDeleteButton.js | 4 +- js/angular/directive/itemReorderButton.js | 19 ++++--- js/angular/directive/list.js | 9 ++-- js/views/listView.js | 47 +++++++++-------- test/html/list.html | 63 ++--------------------- 5 files changed, 48 insertions(+), 94 deletions(-) diff --git a/js/angular/directive/itemDeleteButton.js b/js/angular/directive/itemDeleteButton.js index e5c78580d7..f5e43ddd5b 100644 --- a/js/angular/directive/itemDeleteButton.js +++ b/js/angular/directive/itemDeleteButton.js @@ -33,7 +33,7 @@ IonicModule .directive('ionDeleteButton', ['$animate', function($animate) { return { restrict: 'E', - require: ['^ionItem', '^ionList'], + require: ['^ionItem', '^?ionList'], //Run before anything else, so we can move it before other directives process //its location (eg ngIf relies on the location of the directive in the dom) priority: Number.MAX_VALUE, @@ -48,7 +48,7 @@ IonicModule container.append($element); itemCtrl.$element.append(container).addClass('item-left-editable'); - if (listCtrl.showDelete()) { + if (listCtrl && listCtrl.showDelete()) { $animate.removeClass(container, 'ng-hide'); } }; diff --git a/js/angular/directive/itemReorderButton.js b/js/angular/directive/itemReorderButton.js index 7c275c5340..96f6df71d9 100644 --- a/js/angular/directive/itemReorderButton.js +++ b/js/angular/directive/itemReorderButton.js @@ -14,17 +14,18 @@ var ITEM_TPL_REORDER_BUTTON = * * Can be dragged to reorder items in the list. Takes any ionicon class. * -* When an item reorder is complete, the `on-reorder` callback given in the attribute is called -* (see below). +* Note: Reordering works best when used with `ng-repeat`. Be sure that all `ion-item` children of an `ion-list` are part of the same `ng-repeat` expression. * -* See {@link ionic.directive:ionList} for a complete example. +* When an item reorder is complete, the expression given in the `on-reorder` attribute is called. The `on-reorder` expression is given two locals that can be used: `$fromIndex` and `$toIndex`. See below for an example. +* +* Look at {@link ionic.directive:ionList} for more examples. * * @usage * * ```html * * -* Item {{$index}} +* Item {{item}} * * @@ -46,10 +47,10 @@ var ITEM_TPL_REORDER_BUTTON = * Parameters given: $fromIndex, $toIndex. */ IonicModule -.directive('ionReorderButton', ['$animate', function($animate) { +.directive('ionReorderButton', ['$animate', '$parse', function($animate, $parse) { return { restrict: 'E', - require: ['^ionItem', '^ionList'], + require: ['^ionItem', '^?ionList'], priority: Number.MAX_VALUE, compile: function($element, $attr) { $attr.$set('class', ($attr['class'] || '') + ' button icon button-icon', true); @@ -57,8 +58,10 @@ IonicModule return function($scope, $element, $attr, ctrls) { var itemCtrl = ctrls[0]; var listCtrl = ctrls[1]; + var onReorderFn = $parse($attr.onReorder); + $scope.$onReorder = function(oldIndex, newIndex) { - $scope.$eval($attr.onReorder, { + onReorderFn($scope, { $fromIndex: oldIndex, $toIndex: newIndex }); @@ -68,7 +71,7 @@ IonicModule container.append($element); itemCtrl.$element.append(container).addClass('item-right-editable'); - if (listCtrl.showReorder()) { + if (listCtrl && listCtrl.showReorder()) { $animate.removeClass(container, 'ng-hide'); } }; diff --git a/js/angular/directive/list.js b/js/angular/directive/list.js index ea0ba95067..4fbca79367 100644 --- a/js/angular/directive/list.js +++ b/js/angular/directive/list.js @@ -105,7 +105,7 @@ function($animate, $timeout) { //Make sure onReorder is called in apply cycle, //but also make sure it has no conflicts by doing //$evalAsync - itemScope.$evalAsync(function() { + $timeout(function() { itemScope.$onReorder(oldIndex, newIndex); }); } @@ -115,18 +115,17 @@ function($animate, $timeout) { } }); - if (angular.isDefined($attr.canSwipe)) { + if (isDefined($attr.canSwipe)) { $scope.$watch('!!(' + $attr.canSwipe + ')', function(value) { listCtrl.canSwipeItems(value); }); } - - if (angular.isDefined($attr.showDelete)) { + if (isDefined($attr.showDelete)) { $scope.$watch('!!(' + $attr.showDelete + ')', function(value) { listCtrl.showDelete(value); }); } - if (angular.isDefined($attr.showReorder)) { + if (isDefined($attr.showReorder)) { $scope.$watch('!!(' + $attr.showReorder + ')', function(value) { listCtrl.showReorder(value); }); diff --git a/js/views/listView.js b/js/views/listView.js index 33d65347df..b517a2cd9b 100644 --- a/js/views/listView.js +++ b/js/views/listView.js @@ -254,17 +254,17 @@ if (e.gesture.deltaY < 0 && pixelsPastTop > 0 && scrollY > 0) { this.scrollView.scrollBy(null, -pixelsPastTop); //Trigger another drag so the scrolling keeps going - setTimeout(function() { + ionic.requestAnimationFrame(function() { self.drag(e); - }.bind(this)); + }); } if (e.gesture.deltaY > 0 && pixelsPastBottom > 0) { if (scrollY < this.scrollView.getScrollMax().top) { this.scrollView.scrollBy(null, pixelsPastBottom); //Trigger another drag so the scrolling keeps going - setTimeout(function() { + ionic.requestAnimationFrame(function() { self.drag(e); - }.bind(this)); + }); } } } @@ -280,32 +280,37 @@ this._currentDrag.currentY = scrollY + pageY - offset; - this._reorderItems(); + // this._reorderItems(); } }); // When an item is dragged, we need to reorder any items for sorting purposes - ReorderDrag.prototype._reorderItems = function() { + ReorderDrag.prototype._getReorderIndex = function() { var self = this; var placeholder = this._currentDrag.placeholder; var siblings = Array.prototype.slice.call(this._currentDrag.placeholder.parentNode.children) .filter(function(el) { - return el !== self.el; + return el.nodeName === self.el.nodeName && el !== self.el; }); - var index = siblings.indexOf(this._currentDrag.placeholder); - var topSibling = siblings[Math.max(0, index - 1)]; - var bottomSibling = siblings[Math.min(siblings.length, index+1)]; - var thisOffsetTop = this._currentDrag.currentY;// + this._currentDrag.startOffsetTop; - - if(topSibling && (thisOffsetTop < topSibling.offsetTop + topSibling.offsetHeight)) { - ionic.DomUtil.swapNodes(this._currentDrag.placeholder, topSibling); - return index - 1; - } else if(bottomSibling && thisOffsetTop > (bottomSibling.offsetTop)) { - ionic.DomUtil.swapNodes(bottomSibling, this._currentDrag.placeholder); - return index + 1; + var dragOffsetTop = this._currentDrag.currentY; + var el; + for (var i = 0, len = siblings.length; i < len; i++) { + el = siblings[i]; + if (i === len - 1) { + if (dragOffsetTop > el.offsetTop) { + return i; + } + } else if (i === 0) { + if (dragOffsetTop < el.offsetTop + el.offsetHeight) { + return i; + } + } else if (dragOffsetTop > el.offsetTop - el.offsetHeight / 2 && + dragOffsetTop < el.offsetTop + el.offsetHeight * 1.5) { + return i; + } } - + return this._currentDrag.startIndex; }; ReorderDrag.prototype.end = function(e, doneCallback) { @@ -315,7 +320,7 @@ } var placeholder = this._currentDrag.placeholder; - var finalPosition = ionic.DomUtil.getChildIndex(placeholder, placeholder.nodeName.toLowerCase()); + var finalIndex = this._getReorderIndex(); // Reposition the element this.el.classList.remove(ITEM_REORDERING_CLASS); @@ -324,7 +329,7 @@ placeholder.parentNode.insertBefore(this.el, placeholder); placeholder.parentNode.removeChild(placeholder); - this.onReorder && this.onReorder(this.el, this._currentDrag.startIndex, finalPosition); + this.onReorder && this.onReorder(this.el, this._currentDrag.startIndex, finalIndex); this._currentDrag = null; doneCallback && doneCallback(); diff --git a/test/html/list.html b/test/html/list.html index d4c2824abb..5ac2839e4c 100644 --- a/test/html/list.html +++ b/test/html/list.html @@ -54,14 +54,6 @@
Type 1
- - - - - Woah! -