Skip to content

Commit ba1859b

Browse files
committed
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/).
1 parent 0dad2ed commit ba1859b

File tree

5 files changed

+48
-94
lines changed

5 files changed

+48
-94
lines changed

Diff for: js/angular/directive/itemDeleteButton.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ IonicModule
3333
.directive('ionDeleteButton', ['$animate', function($animate) {
3434
return {
3535
restrict: 'E',
36-
require: ['^ionItem', '^ionList'],
36+
require: ['^ionItem', '^?ionList'],
3737
//Run before anything else, so we can move it before other directives process
3838
//its location (eg ngIf relies on the location of the directive in the dom)
3939
priority: Number.MAX_VALUE,
@@ -48,7 +48,7 @@ IonicModule
4848
container.append($element);
4949
itemCtrl.$element.append(container).addClass('item-left-editable');
5050

51-
if (listCtrl.showDelete()) {
51+
if (listCtrl && listCtrl.showDelete()) {
5252
$animate.removeClass(container, 'ng-hide');
5353
}
5454
};

Diff for: js/angular/directive/itemReorderButton.js

+11-8
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,18 @@ var ITEM_TPL_REORDER_BUTTON =
1414
*
1515
* Can be dragged to reorder items in the list. Takes any ionicon class.
1616
*
17-
* When an item reorder is complete, the `on-reorder` callback given in the attribute is called
18-
* (see below).
17+
* 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.
1918
*
20-
* See {@link ionic.directive:ionList} for a complete example.
19+
* 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.
20+
*
21+
* Look at {@link ionic.directive:ionList} for more examples.
2122
*
2223
* @usage
2324
*
2425
* ```html
2526
* <ion-list ng-controller="MyCtrl">
2627
* <ion-item ng-repeat="item in items">
27-
* Item {{$index}}
28+
* Item {{item}}
2829
* <ion-reorder-button class="ion-navicon"
2930
* on-reorder="moveItem(item, $fromIndex, $toIndex)">
3031
* </ion-reorder>
@@ -46,19 +47,21 @@ var ITEM_TPL_REORDER_BUTTON =
4647
* Parameters given: $fromIndex, $toIndex.
4748
*/
4849
IonicModule
49-
.directive('ionReorderButton', ['$animate', function($animate) {
50+
.directive('ionReorderButton', ['$animate', '$parse', function($animate, $parse) {
5051
return {
5152
restrict: 'E',
52-
require: ['^ionItem', '^ionList'],
53+
require: ['^ionItem', '^?ionList'],
5354
priority: Number.MAX_VALUE,
5455
compile: function($element, $attr) {
5556
$attr.$set('class', ($attr['class'] || '') + ' button icon button-icon', true);
5657
$element[0].setAttribute('data-prevent-scroll', true);
5758
return function($scope, $element, $attr, ctrls) {
5859
var itemCtrl = ctrls[0];
5960
var listCtrl = ctrls[1];
61+
var onReorderFn = $parse($attr.onReorder);
62+
6063
$scope.$onReorder = function(oldIndex, newIndex) {
61-
$scope.$eval($attr.onReorder, {
64+
onReorderFn($scope, {
6265
$fromIndex: oldIndex,
6366
$toIndex: newIndex
6467
});
@@ -68,7 +71,7 @@ IonicModule
6871
container.append($element);
6972
itemCtrl.$element.append(container).addClass('item-right-editable');
7073

71-
if (listCtrl.showReorder()) {
74+
if (listCtrl && listCtrl.showReorder()) {
7275
$animate.removeClass(container, 'ng-hide');
7376
}
7477
};

Diff for: js/angular/directive/list.js

+4-5
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ function($animate, $timeout) {
105105
//Make sure onReorder is called in apply cycle,
106106
//but also make sure it has no conflicts by doing
107107
//$evalAsync
108-
itemScope.$evalAsync(function() {
108+
$timeout(function() {
109109
itemScope.$onReorder(oldIndex, newIndex);
110110
});
111111
}
@@ -115,18 +115,17 @@ function($animate, $timeout) {
115115
}
116116
});
117117

118-
if (angular.isDefined($attr.canSwipe)) {
118+
if (isDefined($attr.canSwipe)) {
119119
$scope.$watch('!!(' + $attr.canSwipe + ')', function(value) {
120120
listCtrl.canSwipeItems(value);
121121
});
122122
}
123-
124-
if (angular.isDefined($attr.showDelete)) {
123+
if (isDefined($attr.showDelete)) {
125124
$scope.$watch('!!(' + $attr.showDelete + ')', function(value) {
126125
listCtrl.showDelete(value);
127126
});
128127
}
129-
if (angular.isDefined($attr.showReorder)) {
128+
if (isDefined($attr.showReorder)) {
130129
$scope.$watch('!!(' + $attr.showReorder + ')', function(value) {
131130
listCtrl.showReorder(value);
132131
});

Diff for: js/views/listView.js

+26-21
Original file line numberDiff line numberDiff line change
@@ -254,17 +254,17 @@
254254
if (e.gesture.deltaY < 0 && pixelsPastTop > 0 && scrollY > 0) {
255255
this.scrollView.scrollBy(null, -pixelsPastTop);
256256
//Trigger another drag so the scrolling keeps going
257-
setTimeout(function() {
257+
ionic.requestAnimationFrame(function() {
258258
self.drag(e);
259-
}.bind(this));
259+
});
260260
}
261261
if (e.gesture.deltaY > 0 && pixelsPastBottom > 0) {
262262
if (scrollY < this.scrollView.getScrollMax().top) {
263263
this.scrollView.scrollBy(null, pixelsPastBottom);
264264
//Trigger another drag so the scrolling keeps going
265-
setTimeout(function() {
265+
ionic.requestAnimationFrame(function() {
266266
self.drag(e);
267-
}.bind(this));
267+
});
268268
}
269269
}
270270
}
@@ -280,32 +280,37 @@
280280

281281
this._currentDrag.currentY = scrollY + pageY - offset;
282282

283-
this._reorderItems();
283+
// this._reorderItems();
284284
}
285285
});
286286

287287
// When an item is dragged, we need to reorder any items for sorting purposes
288-
ReorderDrag.prototype._reorderItems = function() {
288+
ReorderDrag.prototype._getReorderIndex = function() {
289289
var self = this;
290290
var placeholder = this._currentDrag.placeholder;
291291
var siblings = Array.prototype.slice.call(this._currentDrag.placeholder.parentNode.children)
292292
.filter(function(el) {
293-
return el !== self.el;
293+
return el.nodeName === self.el.nodeName && el !== self.el;
294294
});
295295

296-
var index = siblings.indexOf(this._currentDrag.placeholder);
297-
var topSibling = siblings[Math.max(0, index - 1)];
298-
var bottomSibling = siblings[Math.min(siblings.length, index+1)];
299-
var thisOffsetTop = this._currentDrag.currentY;// + this._currentDrag.startOffsetTop;
300-
301-
if(topSibling && (thisOffsetTop < topSibling.offsetTop + topSibling.offsetHeight)) {
302-
ionic.DomUtil.swapNodes(this._currentDrag.placeholder, topSibling);
303-
return index - 1;
304-
} else if(bottomSibling && thisOffsetTop > (bottomSibling.offsetTop)) {
305-
ionic.DomUtil.swapNodes(bottomSibling, this._currentDrag.placeholder);
306-
return index + 1;
296+
var dragOffsetTop = this._currentDrag.currentY;
297+
var el;
298+
for (var i = 0, len = siblings.length; i < len; i++) {
299+
el = siblings[i];
300+
if (i === len - 1) {
301+
if (dragOffsetTop > el.offsetTop) {
302+
return i;
303+
}
304+
} else if (i === 0) {
305+
if (dragOffsetTop < el.offsetTop + el.offsetHeight) {
306+
return i;
307+
}
308+
} else if (dragOffsetTop > el.offsetTop - el.offsetHeight / 2 &&
309+
dragOffsetTop < el.offsetTop + el.offsetHeight * 1.5) {
310+
return i;
311+
}
307312
}
308-
313+
return this._currentDrag.startIndex;
309314
};
310315

311316
ReorderDrag.prototype.end = function(e, doneCallback) {
@@ -315,7 +320,7 @@
315320
}
316321

317322
var placeholder = this._currentDrag.placeholder;
318-
var finalPosition = ionic.DomUtil.getChildIndex(placeholder, placeholder.nodeName.toLowerCase());
323+
var finalIndex = this._getReorderIndex();
319324

320325
// Reposition the element
321326
this.el.classList.remove(ITEM_REORDERING_CLASS);
@@ -324,7 +329,7 @@
324329
placeholder.parentNode.insertBefore(this.el, placeholder);
325330
placeholder.parentNode.removeChild(placeholder);
326331

327-
this.onReorder && this.onReorder(this.el, this._currentDrag.startIndex, finalPosition);
332+
this.onReorder && this.onReorder(this.el, this._currentDrag.startIndex, finalIndex);
328333

329334
this._currentDrag = null;
330335
doneCallback && doneCallback();

Diff for: test/html/list.html

+5-58
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,6 @@ <h1 class="title">Subheader</h1>
5454
<div class="item item-divider">
5555
Type 1
5656
</div>
57-
<ion-item href="#">
58-
<button class="button button-energized">
59-
Hello
60-
</button>
61-
</ion-item>
62-
<ion-item href="hello">
63-
Woah!
64-
</ion-item>
6557
<ion-item ng-repeat="item in items"
6658
ng-click="alert(item.id)"
6759
class="item item-avatar-left item-icon-right"
@@ -106,10 +98,9 @@ <h2>Item {{ item.id }}</h2>
10698
};
10799

108100
$scope.moveItem = function(item, from, to) {
109-
console.log('beforeReorder', item, from, to, $scope.items.slice(0,5));
101+
console.log('beforeReorder', item.id, from, to, $scope.items.slice(0,5));
110102
$scope.items.splice(from, 1);
111103
$scope.items.splice(to, 0, item);
112-
console.log('afterReorder', $scope.items.slice(0,5));
113104
};
114105

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

128-
$scope.items = [
129-
{ id: 1 },
130-
{ id: 2 },
131-
{ id: 3 },
132-
{ id: 4 },
133-
{ id: 5 },
134-
{ id: 6 },
135-
{ id: 7 },
136-
{ id: 8 },
137-
{ id: 9 },
138-
{ id: 1 },
139-
{ id: 2 },
140-
{ id: 3 },
141-
{ id: 4 },
142-
{ id: 5 },
143-
{ id: 6 },
144-
{ id: 7 },
145-
{ id: 8 },
146-
{ id: 9 },
147-
{ id: 1 },
148-
{ id: 2 },
149-
{ id: 3 },
150-
{ id: 4 },
151-
{ id: 5 },
152-
{ id: 6 },
153-
{ id: 7 },
154-
{ id: 8 },
155-
{ id: 9 },
156-
{ id: 1 },
157-
{ id: 2 },
158-
{ id: 3 },
159-
{ id: 4 },
160-
{ id: 5 },
161-
{ id: 6 },
162-
{ id: 7 },
163-
{ id: 8 },
164-
{ id: 9 },
165-
{ id: 1 },
166-
{ id: 2 },
167-
{ id: 3 },
168-
{ id: 4 },
169-
{ id: 5 },
170-
{ id: 6 },
171-
{ id: 7 },
172-
{ id: 8 },
173-
{ id: 9 },
174-
{ id: 10 }
175-
];
119+
$scope.items = [];
120+
for (var i=0; i<100; i++) {
121+
$scope.items.push({id:i});
122+
}
176123

177124
});
178125
</script>

0 commit comments

Comments
 (0)