Skip to content
This repository was archived by the owner on Sep 8, 2020. It is now read-only.

Commit c0679e3

Browse files
thgreasithgreasi
committed
Merge thgreasi/memleak branch
2 parents 6a7579f + e7439e5 commit c0679e3

File tree

4 files changed

+162
-4
lines changed

4 files changed

+162
-4
lines changed

‎src/sortable.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ angular.module('ui.sortable', [])
3333
return (/left|right/).test(item.css('float')) || (/inline|table-cell/).test(item.css('display'));
3434
}
3535

36+
function afterStop(e, ui) {
37+
ui.item.sortable._destroy();
38+
}
39+
3640
var opts = {};
3741

3842
// directive specific options
@@ -95,7 +99,12 @@ angular.module('ui.sortable', [])
9599
return !!ui.item.sortable._isCustomHelperUsed;
96100
},
97101
_isCanceled: false,
98-
_isCustomHelperUsed: ui.item.sortable._isCustomHelperUsed
102+
_isCustomHelperUsed: ui.item.sortable._isCustomHelperUsed,
103+
_destroy: function () {
104+
angular.forEach(ui.item.sortable, function(value, key) {
105+
ui.item.sortable[key] = undefined;
106+
});
107+
}
99108
};
100109
};
101110

@@ -261,6 +270,8 @@ angular.module('ui.sortable', [])
261270
// call apply after stop
262271
value = combineCallbacks(
263272
value, function() { scope.$apply(); });
273+
274+
value = combineCallbacks(value, afterStop);
264275
}
265276
// wrap the callback
266277
value = combineCallbacks(callbacks[key], value);
@@ -276,6 +287,9 @@ angular.module('ui.sortable', [])
276287

277288
angular.forEach(callbacks, function(value, key) {
278289
opts[key] = combineCallbacks(value, opts[key]);
290+
if( key === 'stop' ){
291+
opts[key] = combineCallbacks(opts[key], afterStop);
292+
}
279293
});
280294

281295
} else {

‎test/sortable.e2e.callbacks.spec.js

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@ describe('uiSortable', function() {
66
beforeEach(module('ui.sortable'));
77
beforeEach(module('ui.sortable.testHelper'));
88

9-
var EXTRA_DY_PERCENTAGE, listContent;
9+
var EXTRA_DY_PERCENTAGE, listContent, hasUndefinedProperties;
1010

1111
beforeEach(inject(function (sortableTestHelper) {
1212
EXTRA_DY_PERCENTAGE = sortableTestHelper.EXTRA_DY_PERCENTAGE;
1313
listContent = sortableTestHelper.listContent;
14+
hasUndefinedProperties = sortableTestHelper.hasUndefinedProperties;
1415
}));
1516

1617
describe('Callbacks related', function() {
@@ -188,6 +189,61 @@ describe('uiSortable', function() {
188189
});
189190
});
190191

192+
it('should properly free ui.item.sortable object', function() {
193+
inject(function($compile, $rootScope) {
194+
var element, uiItem, uiItemSortable_Destroy;
195+
element = $compile('<ul ui-sortable="opts" ng-model="items"><li ng-repeat="item in items" id="s-{{$index}}">{{ item }}</li></ul>')($rootScope);
196+
$rootScope.$apply(function() {
197+
$rootScope.opts = {
198+
start: function (e, ui) {
199+
uiItem = ui.item;
200+
spyOn(ui.item.sortable, '_destroy').andCallThrough();
201+
uiItemSortable_Destroy = ui.item.sortable._destroy;
202+
},
203+
update: function(e, ui) {
204+
if (ui.item.scope().item === 'Two') {
205+
ui.item.sortable.cancel();
206+
}
207+
}
208+
};
209+
$rootScope.items = ['One', 'Two', 'Three'];
210+
});
211+
212+
host.append(element);
213+
214+
var li = element.find(':eq(1)');
215+
var dy = (1 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
216+
li.simulate('drag', { dy: dy });
217+
expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
218+
expect($rootScope.items).toEqual(listContent(element));
219+
expect(uiItemSortable_Destroy).toHaveBeenCalled();
220+
expect(hasUndefinedProperties(uiItem.sortable)).toBe(true);
221+
uiItem = uiItemSortable_Destroy = undefined;
222+
223+
224+
li = element.find(':eq(0)');
225+
dy = (2 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
226+
li.simulate('drag', { dy: dy });
227+
expect($rootScope.items).toEqual(['Two', 'Three', 'One']);
228+
expect($rootScope.items).toEqual(listContent(element));
229+
expect(uiItemSortable_Destroy).toHaveBeenCalled();
230+
expect(hasUndefinedProperties(uiItem.sortable)).toBe(true);
231+
uiItem = uiItemSortable_Destroy = undefined;
232+
233+
234+
li = element.find(':eq(2)');
235+
dy = -(2 + EXTRA_DY_PERCENTAGE) * li.outerHeight();
236+
li.simulate('drag', { dy: dy });
237+
expect($rootScope.items).toEqual(['One', 'Two', 'Three']);
238+
expect($rootScope.items).toEqual(listContent(element));
239+
expect(uiItemSortable_Destroy).toHaveBeenCalled();
240+
expect(hasUndefinedProperties(uiItem.sortable)).toBe(true);
241+
uiItem = uiItemSortable_Destroy = undefined;
242+
243+
$(element).remove();
244+
});
245+
});
246+
191247
});
192248

193249
});

‎test/sortable.e2e.multi.spec.js

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@ describe('uiSortable', function() {
66
beforeEach(module('ui.sortable'));
77
beforeEach(module('ui.sortable.testHelper'));
88

9-
var EXTRA_DY_PERCENTAGE, listContent, listInnerContent, simulateElementDrag;
9+
var EXTRA_DY_PERCENTAGE, listContent, listInnerContent, simulateElementDrag, hasUndefinedProperties;
1010

1111
beforeEach(inject(function (sortableTestHelper) {
1212
EXTRA_DY_PERCENTAGE = sortableTestHelper.EXTRA_DY_PERCENTAGE;
1313
listContent = sortableTestHelper.listContent;
1414
listInnerContent = sortableTestHelper.listInnerContent;
1515
simulateElementDrag = sortableTestHelper.simulateElementDrag;
16+
hasUndefinedProperties = sortableTestHelper.hasUndefinedProperties;
1617
}));
1718

1819
describe('Multiple sortables related', function() {
@@ -470,6 +471,83 @@ describe('uiSortable', function() {
470471
});
471472
});
472473

474+
it('should properly free ui.item.sortable object', function() {
475+
inject(function($compile, $rootScope) {
476+
var elementTop, elementBottom, uiItem, uiItemSortable_Destroy;
477+
elementTop = $compile('<ul ui-sortable="opts" class="cross-sortable" ng-model="itemsTop"><li ng-repeat="item in itemsTop" id="s-top-{{$index}}">{{ item }}</li></ul>')($rootScope);
478+
elementBottom = $compile('<ul ui-sortable="opts" class="cross-sortable" ng-model="itemsBottom"><li ng-repeat="item in itemsBottom" id="s-bottom-{{$index}}">{{ item }}</li></ul>')($rootScope);
479+
$rootScope.$apply(function() {
480+
$rootScope.itemsTop = ['Top One', 'Top Two', 'Top Three'];
481+
$rootScope.itemsBottom = ['Bottom One', 'Bottom Two', 'Bottom Three'];
482+
$rootScope.opts = {
483+
connectWith: '.cross-sortable',
484+
start: function (e, ui) {
485+
uiItem = ui.item;
486+
spyOn(ui.item.sortable, '_destroy').andCallThrough();
487+
uiItemSortable_Destroy = ui.item.sortable._destroy;
488+
},
489+
update: function(e, ui) {
490+
uiItem.sortable = ui.item.sortable;
491+
if (ui.item.scope() &&
492+
(typeof ui.item.scope().item === 'string') &&
493+
ui.item.scope().item.indexOf('Two') >= 0) {
494+
ui.item.sortable.cancel();
495+
}
496+
}
497+
};
498+
});
499+
500+
host.append(elementTop).append(elementBottom).append('<div class="clear"></div>');
501+
502+
var li1 = elementTop.find(':eq(1)');
503+
var li2 = elementBottom.find(':eq(0)');
504+
simulateElementDrag(li1, li2, 'below');
505+
expect($rootScope.itemsTop).toEqual(['Top One', 'Top Two', 'Top Three']);
506+
expect($rootScope.itemsBottom).toEqual(['Bottom One', 'Bottom Two', 'Bottom Three']);
507+
expect($rootScope.itemsTop).toEqual(listContent(elementTop));
508+
expect($rootScope.itemsBottom).toEqual(listContent(elementBottom));
509+
expect(uiItemSortable_Destroy).toHaveBeenCalled();
510+
expect(hasUndefinedProperties(uiItem.sortable)).toBe(true);
511+
uiItem = uiItemSortable_Destroy = undefined;
512+
513+
li1 = elementBottom.find(':eq(1)');
514+
li2 = elementTop.find(':eq(1)');
515+
simulateElementDrag(li1, li2, { place: 'above', extradx: -20, extrady: -10 });
516+
expect($rootScope.itemsTop).toEqual(['Top One', 'Top Two', 'Top Three']);
517+
expect($rootScope.itemsBottom).toEqual(['Bottom One', 'Bottom Two', 'Bottom Three']);
518+
expect($rootScope.itemsTop).toEqual(listContent(elementTop));
519+
expect($rootScope.itemsBottom).toEqual(listContent(elementBottom));
520+
expect(uiItemSortable_Destroy).toHaveBeenCalled();
521+
expect(hasUndefinedProperties(uiItem.sortable)).toBe(true);
522+
uiItem = uiItemSortable_Destroy = undefined;
523+
524+
li1 = elementTop.find(':eq(0)');
525+
li2 = elementBottom.find(':eq(0)');
526+
simulateElementDrag(li1, li2, 'below');
527+
expect($rootScope.itemsTop).toEqual(['Top Two', 'Top Three']);
528+
expect($rootScope.itemsBottom).toEqual(['Bottom One', 'Top One', 'Bottom Two', 'Bottom Three']);
529+
expect($rootScope.itemsTop).toEqual(listContent(elementTop));
530+
expect($rootScope.itemsBottom).toEqual(listContent(elementBottom));
531+
expect(uiItemSortable_Destroy).toHaveBeenCalled();
532+
expect(hasUndefinedProperties(uiItem.sortable)).toBe(true);
533+
uiItem = uiItemSortable_Destroy = undefined;
534+
535+
li1 = elementBottom.find(':eq(1)');
536+
li2 = elementTop.find(':eq(1)');
537+
simulateElementDrag(li1, li2, { place: 'above', extradx: -20, extrady: -10 });
538+
expect($rootScope.itemsTop).toEqual(['Top Two', 'Top One', 'Top Three']);
539+
expect($rootScope.itemsBottom).toEqual(['Bottom One', 'Bottom Two', 'Bottom Three']);
540+
expect($rootScope.itemsTop).toEqual(listContent(elementTop));
541+
expect($rootScope.itemsBottom).toEqual(listContent(elementBottom));
542+
expect(uiItemSortable_Destroy).toHaveBeenCalled();
543+
expect(hasUndefinedProperties(uiItem.sortable)).toBe(true);
544+
uiItem = uiItemSortable_Destroy = undefined;
545+
546+
$(elementTop).remove();
547+
$(elementBottom).remove();
548+
});
549+
});
550+
473551
});
474552

475553
});

‎test/sortable.test-helper.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,20 @@ angular.module('ui.sortable.testHelper', [])
6262
draggedElement.simulate('drag', dragOptions);
6363
}
6464

65+
function hasUndefinedProperties(testObject) {
66+
return testObject && Object.keys(testObject)
67+
.filter(function(key) {
68+
return testObject.hasOwnProperty(key) &&
69+
testObject[key] !== undefined;
70+
})
71+
.length === 0;
72+
}
73+
6574
return {
6675
EXTRA_DY_PERCENTAGE: EXTRA_DY_PERCENTAGE,
6776
listContent: listContent,
6877
listInnerContent: listInnerContent,
69-
simulateElementDrag: simulateElementDrag
78+
simulateElementDrag: simulateElementDrag,
79+
hasUndefinedProperties: hasUndefinedProperties
7080
};
7181
});

0 commit comments

Comments
 (0)