Skip to content

Commit

Permalink
fix(ionReorderButton): stop ngRepeat:dupes error when reordering
Browse files Browse the repository at this point in the history 
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/).
  • Loading branch information
@ajoslin
ajoslin committed Jul 6, 2014
1 parent 0dad2ed commit ba1859b
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 94 deletions.
4 changes: 2 additions & 2 deletions js/angular/directive/itemDeleteButton.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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');
}
};
Expand Down
19 changes: 11 additions & 8 deletions js/angular/directive/itemReorderButton.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
* <ion-list ng-controller="MyCtrl">
* <ion-item ng-repeat="item in items">
* Item {{$index}}
* Item {{item}}
* <ion-reorder-button class="ion-navicon"
* on-reorder="moveItem(item, $fromIndex, $toIndex)">
* </ion-reorder>
Expand All @@ -46,19 +47,21 @@ 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);
$element[0].setAttribute('data-prevent-scroll', true);
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
});
Expand All @@ -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');
}
};
Expand Down
9 changes: 4 additions & 5 deletions js/angular/directive/list.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {

This comment has been minimized.

Sign in to view

Copy link
@gkalpak

gkalpak Jan 2, 2015

@ajoslin, I know this is an old commit, but do you happen to remember why $timeout instead of $evalAsync ?
(The comment above is outdated btw.)
itemScope.$onReorder(oldIndex, newIndex);
});
}
Expand All @@ -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);
});
Expand Down
47 changes: 26 additions & 21 deletions js/views/listView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
});
}
}
}
Expand All @@ -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) {
Expand All @@ -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);
Expand All @@ -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();
Expand Down
63 changes: 5 additions & 58 deletions test/html/list.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@ <h1 class="title">Subheader</h1>
<div class="item item-divider">
Type 1
</div>
<ion-item href="#">
<button class="button button-energized">
Hello
</button>
</ion-item>
<ion-item href="hello">
Woah!
</ion-item>
<ion-item ng-repeat="item in items"
ng-click="alert(item.id)"
class="item item-avatar-left item-icon-right"
Expand Down Expand Up @@ -106,10 +98,9 @@ <h2>Item {{ item.id }}</h2>
};

$scope.moveItem = function(item, from, to) {
console.log('beforeReorder', item, from, to, $scope.items.slice(0,5));
console.log('beforeReorder', item.id, from, to, $scope.items.slice(0,5));
$scope.items.splice(from, 1);
$scope.items.splice(to, 0, item);
console.log('afterReorder', $scope.items.slice(0,5));
};

$scope.alert = window.alert.bind(window);
Expand All @@ -125,54 +116,10 @@ <h2>Item {{ item.id }}</h2>
$scope.items.splice($scope.items.indexOf(item), 1);
};

$scope.items = [
{ id: 1 },
{ id: 2 },
{ id: 3 },
{ id: 4 },
{ id: 5 },
{ id: 6 },
{ id: 7 },
{ id: 8 },
{ id: 9 },
{ id: 1 },
{ id: 2 },
{ id: 3 },
{ id: 4 },
{ id: 5 },
{ id: 6 },
{ id: 7 },
{ id: 8 },
{ id: 9 },
{ id: 1 },
{ id: 2 },
{ id: 3 },
{ id: 4 },
{ id: 5 },
{ id: 6 },
{ id: 7 },
{ id: 8 },
{ id: 9 },
{ id: 1 },
{ id: 2 },
{ id: 3 },
{ id: 4 },
{ id: 5 },
{ id: 6 },
{ id: 7 },
{ id: 8 },
{ id: 9 },
{ id: 1 },
{ id: 2 },
{ id: 3 },
{ id: 4 },
{ id: 5 },
{ id: 6 },
{ id: 7 },
{ id: 8 },
{ id: 9 },
{ id: 10 }
];
$scope.items = [];
for (var i=0; i<100; i++) {
$scope.items.push({id:i});
}

});
</script>
Expand Down

0 comments on commit ba1859b

Please sign in to comment.