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

Commit c9b0bfe

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

File tree

2 files changed

+116
-112
lines changed

2 files changed

+116
-112
lines changed

‎src/ng/directive/ngSwitch.js

+8-7
Original file line numberDiff line numberDiff line change
@@ -141,22 +141,23 @@ 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) {
148148
var i, ii;
149-
for (i = 0, ii = previousElements.length; i < ii; ++i) {
150-
previousElements[i].remove();
149+
for (i = 0, ii = previousLeaveAnimations.length; i < ii; ++i) {
150+
$animate.cancel(previousLeaveAnimations[i]);
151151
}
152-
previousElements.length = 0;
152+
previousLeaveAnimations.length = 0;
153153

154154
for (i = 0, ii = selectedScopes.length; i < ii; ++i) {
155155
var selected = getBlockNodes(selectedElements[i].clone);
156156
selectedScopes[i].$destroy();
157-
previousElements[i] = selected;
158-
$animate.leave(selected).then(function() {
159-
previousElements.splice(i, 1);
157+
158+
var promise = previousLeaveAnimations[i] = $animate.leave(selected);
159+
promise.then(function() {
160+
previousLeaveAnimations.splice(i, 1);
160161
});
161162
}
162163

‎test/ng/directive/ngSwitchSpec.js

+108-105
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,122 @@ 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+
expect(destroyed).toBe(true);
374+
});
432375
});
433376
});
434377

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

0 commit comments

Comments
 (0)