Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit bd07dc4

Browse files
matskomatsko
committed
fix(ngSwitch): avoid removing DOM nodes twice within watch operation
Closes #8662
1 parent 1eda183 commit bd07dc4

File tree

2 files changed

+122
-113
lines changed

2 files changed

+122
-113
lines changed

‎src/ng/directive/ngSwitch.js

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,22 +141,26 @@ var ngSwitchDirective = ['$animate', function($animate) {
141141
var watchExpr = attr.ngSwitch || attr.on,
142142
selectedTranscludes = [],
143143
selectedElements = [],
144-
previousElements = [],
144+
previousLeaveAnimations = [],
145145
selectedScopes = [];
146146

147147
scope.$watch(watchExpr, function ngSwitchWatchAction(value) {
148-
var i, ii;
149-
for (i = 0, ii = previousElements.length; i < ii; ++i) {
150-
previousElements[i].remove();
148+
var i, ii, promise;
149+
for (i = 0, ii = previousLeaveAnimations.length; i < ii; ++i) {
150+
promise = previousLeaveAnimations[i];
151+
if (promise) {
152+
$animate.cancel(promise);
153+
}
151154
}
152-
previousElements.length = 0;
155+
previousLeaveAnimations.length = 0;
153156

154157
for (i = 0, ii = selectedScopes.length; i < ii; ++i) {
155158
var selected = getBlockNodes(selectedElements[i].clone);
156159
selectedScopes[i].$destroy();
157-
previousElements[i] = selected;
158-
$animate.leave(selected).then(function() {
159-
previousElements.splice(i, 1);
160+
161+
promise = previousLeaveAnimations[i] = $animate.leave(selected);
162+
promise.then(function() {
163+
previousLeaveAnimations.splice(i, 1);
160164
});
161165
}
162166

‎test/ng/directive/ngSwitchSpec.js

Lines changed: 110 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ describe('ngSwitch', function() {
314314
}));
315315
});
316316

317-
describe('ngSwitch animations', function() {
317+
describe('ngSwitch animation', function() {
318318
var body, element, $rootElement;
319319

320320
function html(content) {
@@ -323,8 +323,6 @@ describe('ngSwitch animations', function() {
323323
return element;
324324
}
325325

326-
beforeEach(module('ngAnimateMock'));
327-
328326
beforeEach(module(function() {
329327
// we need to run animation on attached elements;
330328
return function(_$rootElement_) {
@@ -339,117 +337,124 @@ describe('ngSwitch animations', function() {
339337
dealoc(element);
340338
});
341339

342-
it('should fire off the enter animation',
343-
inject(function($compile, $rootScope, $animate) {
344-
var item;
345-
var $scope = $rootScope.$new();
346-
element = $compile(html(
347-
'<div ng-switch on="val">' +
348-
'<div ng-switch-when="one">one</div>' +
349-
'<div ng-switch-when="two">two</div>' +
350-
'<div ng-switch-when="three">three</div>' +
351-
'</div>'
352-
))($scope);
353-
354-
$rootScope.$digest(); // re-enable the animations;
355-
$scope.val = 'one';
356-
$scope.$digest();
357-
358-
item = $animate.queue.shift();
359-
expect(item.event).toBe('enter');
360-
expect(item.element.text()).toBe('one');
361-
})
362-
);
363-
364-
365-
it('should fire off the leave animation',
366-
inject(function($compile, $rootScope, $animate) {
367-
var item;
368-
var $scope = $rootScope.$new();
369-
element = $compile(html(
370-
'<div ng-switch on="val">' +
371-
'<div ng-switch-when="one">one</div>' +
372-
'<div ng-switch-when="two">two</div>' +
373-
'<div ng-switch-when="three">three</div>' +
374-
'</div>'
375-
))($scope);
376-
377-
$rootScope.$digest(); // re-enable the animations;
378-
$scope.val = 'two';
379-
$scope.$digest();
380-
381-
item = $animate.queue.shift();
382-
expect(item.event).toBe('enter');
383-
expect(item.element.text()).toBe('two');
384-
385-
$scope.val = 'three';
386-
$scope.$digest();
387-
388-
item = $animate.queue.shift();
389-
expect(item.event).toBe('leave');
390-
expect(item.element.text()).toBe('two');
391-
392-
item = $animate.queue.shift();
393-
expect(item.event).toBe('enter');
394-
expect(item.element.text()).toBe('three');
395-
})
396-
);
397-
398-
it('should destroy the previous leave animation if a new one takes place', function() {
399-
module(function($provide) {
400-
$provide.decorator('$animate', function($delegate, $$q) {
401-
var emptyPromise = $$q.defer().promise;
402-
$delegate.leave = function() {
403-
return emptyPromise;
404-
};
405-
return $delegate;
406-
});
407-
});
408-
inject(function ($compile, $rootScope, $animate, $templateCache) {
409-
var item;
410-
var $scope = $rootScope.$new();
411-
element = $compile(html(
412-
'<div ng-switch="inc">' +
413-
'<div ng-switch-when="one">one</div>' +
414-
'<div ng-switch-when="two">two</div>' +
415-
'</div>'
416-
))($scope);
417-
418-
$scope.$apply('inc = "one"');
419-
420-
var destroyed, inner = element.children(0);
421-
inner.on('$destroy', function() {
422-
destroyed = true;
340+
describe('behavior', function() {
341+
it('should destroy the previous leave animation if a new one takes place', function() {
342+
module('ngAnimate');
343+
module(function($animateProvider) {
344+
$animateProvider.register('.long-leave', function() {
345+
return {
346+
leave : function(element, done) {
347+
//do nothing at all
348+
}
349+
};
350+
});
423351
});
352+
inject(function ($compile, $rootScope, $animate, $templateCache) {
353+
var item;
354+
var $scope = $rootScope.$new();
355+
element = $compile(html(
356+
'<div ng-switch="inc">' +
357+
'<div ng-switch-when="one">one</div>' +
358+
'<div ng-switch-when="two">two</div>' +
359+
'</div>'
360+
))($scope);
361+
362+
$scope.$apply('inc = "one"');
424363

425-
$scope.$apply('inc = "two"');
364+
var destroyed, inner = element.children(0);
365+
inner.on('$destroy', function() {
366+
destroyed = true;
367+
});
426368

427-
$scope.$apply('inc = "one"');
369+
$scope.$apply('inc = "two"');
428370

429-
$scope.$apply('inc = "two"');
371+
$scope.$apply('inc = "one"');
430372

431-
expect(destroyed).toBe(true);
373+
$scope.$apply('inc = "two"');
374+
375+
expect(destroyed).toBe(true);
376+
});
432377
});
433378
});
434379

435-
it('should work with svg elements when the svg container is transcluded', function() {
436-
module(function($compileProvider) {
437-
$compileProvider.directive('svgContainer', function() {
438-
return {
439-
template: '<svg ng-transclude></svg>',
440-
replace: true,
441-
transclude: true
442-
};
380+
describe('events', function() {
381+
beforeEach(module('ngAnimateMock'));
382+
383+
it('should fire off the enter animation',
384+
inject(function($compile, $rootScope, $animate) {
385+
var item;
386+
var $scope = $rootScope.$new();
387+
element = $compile(html(
388+
'<div ng-switch on="val">' +
389+
'<div ng-switch-when="one">one</div>' +
390+
'<div ng-switch-when="two">two</div>' +
391+
'<div ng-switch-when="three">three</div>' +
392+
'</div>'
393+
))($scope);
394+
395+
$rootScope.$digest(); // re-enable the animations;
396+
$scope.val = 'one';
397+
$scope.$digest();
398+
399+
item = $animate.queue.shift();
400+
expect(item.event).toBe('enter');
401+
expect(item.element.text()).toBe('one');
402+
})
403+
);
404+
405+
406+
it('should fire off the leave animation',
407+
inject(function($compile, $rootScope, $animate) {
408+
var item;
409+
var $scope = $rootScope.$new();
410+
element = $compile(html(
411+
'<div ng-switch on="val">' +
412+
'<div ng-switch-when="one">one</div>' +
413+
'<div ng-switch-when="two">two</div>' +
414+
'<div ng-switch-when="three">three</div>' +
415+
'</div>'
416+
))($scope);
417+
418+
$rootScope.$digest(); // re-enable the animations;
419+
$scope.val = 'two';
420+
$scope.$digest();
421+
422+
item = $animate.queue.shift();
423+
expect(item.event).toBe('enter');
424+
expect(item.element.text()).toBe('two');
425+
426+
$scope.val = 'three';
427+
$scope.$digest();
428+
429+
item = $animate.queue.shift();
430+
expect(item.event).toBe('leave');
431+
expect(item.element.text()).toBe('two');
432+
433+
item = $animate.queue.shift();
434+
expect(item.event).toBe('enter');
435+
expect(item.element.text()).toBe('three');
436+
})
437+
);
438+
439+
it('should work with svg elements when the svg container is transcluded', function() {
440+
module(function($compileProvider) {
441+
$compileProvider.directive('svgContainer', function() {
442+
return {
443+
template: '<svg ng-transclude></svg>',
444+
replace: true,
445+
transclude: true
446+
};
447+
});
448+
});
449+
inject(function($compile, $rootScope) {
450+
element = $compile('<svg-container ng-switch="inc"><circle ng-switch-when="one"></circle>' +
451+
'</svg-container>')($rootScope);
452+
$rootScope.inc = 'one';
453+
$rootScope.$apply();
454+
455+
var circle = element.find('circle');
456+
expect(circle[0].toString()).toMatch(/SVG/);
443457
});
444-
});
445-
inject(function($compile, $rootScope) {
446-
element = $compile('<svg-container ng-switch="inc"><circle ng-switch-when="one"></circle>' +
447-
'</svg-container>')($rootScope);
448-
$rootScope.inc = 'one';
449-
$rootScope.$apply();
450-
451-
var circle = element.find('circle');
452-
expect(circle[0].toString()).toMatch(/SVG/);
453458
});
454459
});
455460
});

0 commit comments

Comments
 (0)