Skip to content

Commit 3dc6ab6

Browse files
committed
fix(collectionRepeat): properly delete items when setting size to 0
Refactored to create a `changeValidator` object that will properly tell you if a resize or a data change requires a refresh. Closes #3299.
1 parent a301483 commit 3dc6ab6

File tree

3 files changed

+58
-23
lines changed

3 files changed

+58
-23
lines changed

js/angular/directive/collectionRepeat.js

+45-20
Original file line numberDiff line numberDiff line change
@@ -138,30 +138,37 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r
138138

139139
var afterItemsContainer = initAfterItemsContainer();
140140

141+
var changeValidator = makeChangeValidator();
141142
initDimensions();
142143

143144
// Dimensions are refreshed on resize or data change.
144-
angular.element($window).on('resize', validateResize);
145145
scrollCtrl.$element.on('scroll.resize', refreshDimensions);
146-
var unlistenToExposeAside = $rootScope.$on('$ionicExposeAside', validateResize);
146+
147+
angular.element($window).on('resize', onResize);
148+
var unlistenToExposeAside = $rootScope.$on('$ionicExposeAside', onResize);
147149
$timeout(refreshDimensions, 0, false);
148150

151+
function onResize() {
152+
if (changeValidator.resizeRequiresRefresh(scrollView.__clientWidth, scrollView.__clientHeight)) {
153+
refreshDimensions();
154+
}
155+
}
156+
149157
scope.$watchCollection(listGetter, function(newValue) {
150158
data = newValue || (newValue = []);
151159
if (!angular.isArray(newValue)) {
152160
throw new Error("collection-repeat expected an array for '" + listExpr + "', " +
153161
"but got a " + typeof value);
154162
}
155-
156163
// Wait for this digest to end before refreshing everything.
157164
scope.$$postDigest(function() {
158-
getRepeatManager().refreshData(newValue);
159-
refreshDimensions();
165+
getRepeatManager().setData(data);
166+
if (changeValidator.dataChangeRequiresRefresh(data)) refreshDimensions();
160167
});
161168
});
162169

163170
scope.$on('$destroy', function() {
164-
angular.element($window).off('resize', validateResize);
171+
angular.element($window).off('resize', onResize);
165172
unlistenToExposeAside();
166173
scrollCtrl.$element && scrollCtrl.$element.off('scroll.resize', refreshDimensions);
167174

@@ -174,6 +181,32 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r
174181
repeatManager = null;
175182
});
176183

184+
function makeChangeValidator() {
185+
var self;
186+
return (self = {
187+
dataLength: 0,
188+
width: 0,
189+
height: 0,
190+
resizeRequiresRefresh: function(newWidth, newHeight) {
191+
var requiresRefresh = self.dataLength &&
192+
newWidth && newWidth !== self.width &&
193+
newHeight && newHeight !== self.height;
194+
195+
self.width = newWidth;
196+
self.height = newHeight;
197+
198+
return !!requiresRefresh;
199+
},
200+
dataChangeRequiresRefresh: function(newData) {
201+
var requiresRefresh = newData.length > 0 || newData.length < self.dataLength;
202+
203+
self.dataLength = newData.length;
204+
205+
return !!requiresRefresh;
206+
}
207+
});
208+
}
209+
177210
function getRepeatManager() {
178211
return repeatManager || (repeatManager = new $ionicCollectionManager({
179212
afterItemsNode: afterItemsContainer[0],
@@ -242,23 +275,14 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r
242275
}
243276
}
244277

245-
// Make sure this resize actually changed the size of the screen
246-
function validateResize() {
247-
var h = scrollView.__clientHeight, w = scrollView.__clientWidth;
248-
if (w && h && (validateResize.height !== h || validateResize.width !== w)) {
249-
validateResize.height = h;
250-
validateResize.width = w;
251-
refreshDimensions();
252-
}
253-
}
254278
function refreshDimensions() {
255-
if (!data.length) return;
279+
var hasData = data.length > 0;
256280

257-
if (heightData.computed || widthData.computed) {
281+
if (hasData && (heightData.computed || widthData.computed)) {
258282
computeStyleDimensions();
259283
}
260284

261-
if (heightData.computed) {
285+
if (hasData && heightData.computed) {
262286
heightData.value = computedStyleDimensions.height;
263287
if (!heightData.value) {
264288
throw new Error('collection-repeat tried to compute the height of repeated elements "' +
@@ -269,7 +293,8 @@ function CollectionRepeatDirective($ionicCollectionManager, $parse, $window, $$r
269293
// If it's a constant with a getter (eg percent), we just refresh .value after resize
270294
heightData.value = heightData.getValue();
271295
}
272-
if (widthData.computed) {
296+
297+
if (hasData && widthData.computed) {
273298
widthData.value = computedStyleDimensions.width;
274299
if (!widthData.value) {
275300
throw new Error('collection-repeat tried to compute the width of repeated elements "' +
@@ -527,7 +552,7 @@ function RepeatManagerFactory($rootScope, $window, $$rAF) {
527552
}
528553
};
529554

530-
this.refreshData = function(newData) {
555+
this.setData = function(newData) {
531556
data = newData;
532557
(view.onRefreshData || angular.noop)();
533558
isDataReady = true;

scss/_util.scss

+1
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@
276276
}
277277
.collection-repeat-container {
278278
position: relative;
279+
z-index: 1; //make sure it's above the after-container
279280
}
280281
.collection-repeat-after-container {
281282
z-index: 0;

test/html/collection-repeat/basic-list.html

+12-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
<ion-header-bar class="bar-positive">
1616
<button class="button" ng-click="$root.showDelete = !$root.showDelete">Toggle Delete</button>
1717
<h1 class="title">Basic List: Static Dimensions</h1>
18+
<button class="button" ng-click="clearList()">Clear List</button>
1819
</ion-header-bar>
1920
<ion-content>
2021
<h4 style="margin: 80px;">I have 80px margin</h4>
@@ -40,10 +41,18 @@ <h2>Stuff after list</h2>
4041
image: 'http://placekitten.com/'+(100+50%i)+'/'+(100+50%i)
4142
});
4243
}
44+
init();
4345

44-
$timeout(function() {
45-
for (var i = 0; i < 100; i++) addImage();
46-
}, 1000);
46+
$scope.clearList = function() {
47+
$scope.items.length = 0;
48+
init();
49+
};
50+
51+
function init() {
52+
$timeout(function() {
53+
for (var i = 0; i < 100; i++) addImage();
54+
}, 1000);
55+
}
4756
}
4857
</script>
4958
<style>

0 commit comments

Comments
 (0)