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

Commit bce6533

Browse files
committed
fix(ngAnimateSwap): make it compatible with ngIf on the same element
Previously, both `ngAnimateSwap` and `ngIf` had a priority of 600, which meant that (while both are [terminal directives][1]) they were executed on top of each other (essentially messing each other's comment node). This commit fixes it, by giving `ngAnimateSwap` a priority of 550, which is lower than `ngIf` but still higher than other directives. For reference, here is a list of built-in directive per priority: ``` -400: ngInclude, ngView -1: ngRef 1: ngMessage, ngMessageDefault, ngMessageExp, ngModel, select 10: ngModelOptions 99: ngHref, ngSrc, ngSrcset 100: attr interpolation, ngChecked, ngDisabled, ngList, ngMax, ngMaxlength, ngMin, ngMinlength, ngModel (aria), ngMultiple, ngOpen, ngPattern, ngProp*, ngReadonly, ngRequired, ngSelected, ngStep, ngValue, option 400: ngInclude, ngView 450: ngInit 500: ngController 600: ngAnimateSwap, ngIf 1000: ngNonBindable, ngRepeat 1200: ngSwitchDefault, ngSwitchWhen ``` [1]: https://docs.angularjs.org/api/ng/service/$compile#-terminal- Fixes #16616
1 parent 0a5e58d commit bce6533

File tree

2 files changed

+50
-30
lines changed

2 files changed

+50
-30
lines changed

src/ngAnimate/ngAnimateSwap.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ var ngAnimateSwapDirective = ['$animate', function($animate) {
9292
restrict: 'A',
9393
transclude: 'element',
9494
terminal: true,
95-
priority: 600, // we use 600 here to ensure that the directive is caught before others
95+
priority: 550, // We use 550 here to ensure that the directive is caught before others,
96+
// but after `ngIf` (at priority 600).
9697
link: function(scope, $element, attrs, ctrl, $transclude) {
9798
var previousElement, previousScope;
9899
scope.$watchCollection(attrs.ngAnimateSwap || attrs['for'], function(value) {

test/ngAnimate/ngAnimateSwapSpec.js

+48-29
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ describe('ngAnimateSwap', function() {
2020
}));
2121

2222

23-
it('should render a new container when the expression changes', inject(function() {
23+
it('should render a new container when the expression changes', function() {
2424
element = $compile('<div><div ng-animate-swap="exp">{{ exp }}</div></div>')($rootScope);
2525
$rootScope.$digest();
2626

@@ -40,9 +40,9 @@ describe('ngAnimateSwap', function() {
4040
expect(third.textContent).toBe('super');
4141
expect(third).not.toEqual(second);
4242
expect(second.parentNode).toBeFalsy();
43-
}));
43+
});
4444

45-
it('should render a new container only when the expression property changes', inject(function() {
45+
it('should render a new container only when the expression property changes', function() {
4646
element = $compile('<div><div ng-animate-swap="exp.prop">{{ exp.value }}</div></div>')($rootScope);
4747
$rootScope.exp = {
4848
prop: 'hello',
@@ -66,9 +66,9 @@ describe('ngAnimateSwap', function() {
6666
var three = element.find('div')[0];
6767
expect(three.textContent).toBe('planet');
6868
expect(three).not.toBe(two);
69-
}));
69+
});
7070

71-
it('should watch the expression as a collection', inject(function() {
71+
it('should watch the expression as a collection', function() {
7272
element = $compile('<div><div ng-animate-swap="exp">{{ exp.a }} {{ exp.b }} {{ exp.c }}</div></div>')($rootScope);
7373
$rootScope.exp = {
7474
a: 1,
@@ -99,26 +99,24 @@ describe('ngAnimateSwap', function() {
9999
var four = element.find('div')[0];
100100
expect(four.textContent.trim()).toBe('4');
101101
expect(four).not.toEqual(three);
102-
}));
102+
});
103103

104104
they('should consider $prop as a falsy value', [false, undefined, null], function(value) {
105-
inject(function() {
106-
element = $compile('<div><div ng-animate-swap="value">{{ value }}</div></div>')($rootScope);
107-
$rootScope.value = true;
108-
$rootScope.$digest();
105+
element = $compile('<div><div ng-animate-swap="value">{{ value }}</div></div>')($rootScope);
106+
$rootScope.value = true;
107+
$rootScope.$digest();
109108

110-
var one = element.find('div')[0];
111-
expect(one).toBeTruthy();
109+
var one = element.find('div')[0];
110+
expect(one).toBeTruthy();
112111

113-
$rootScope.value = value;
114-
$rootScope.$digest();
112+
$rootScope.value = value;
113+
$rootScope.$digest();
115114

116-
var two = element.find('div')[0];
117-
expect(two).toBeFalsy();
118-
});
115+
var two = element.find('div')[0];
116+
expect(two).toBeFalsy();
119117
});
120118

121-
it('should consider "0" as a truthy value', inject(function() {
119+
it('should consider "0" as a truthy value', function() {
122120
element = $compile('<div><div ng-animate-swap="value">{{ value }}</div></div>')($rootScope);
123121
$rootScope.$digest();
124122

@@ -130,9 +128,9 @@ describe('ngAnimateSwap', function() {
130128

131129
var two = element.find('div')[0];
132130
expect(two).toBeTruthy();
133-
}));
131+
});
134132

135-
it('should create a new (non-isolate) scope for each inserted clone', inject(function() {
133+
it('should create a new (non-isolate) scope for each inserted clone', function() {
136134
var parentScope = $rootScope.$new();
137135
parentScope.foo = 'bar';
138136

@@ -147,9 +145,9 @@ describe('ngAnimateSwap', function() {
147145
expect(scopeTwo.foo).toBe('bar');
148146

149147
expect(scopeOne).not.toBe(scopeTwo);
150-
}));
148+
});
151149

152-
it('should destroy the previous scope when removing the element', inject(function() {
150+
it('should destroy the previous scope when removing the element', function() {
153151
element = $compile('<div><div ng-animate-swap="value">{{ value }}</div></div>')($rootScope);
154152

155153
$rootScope.$apply('value = 1');
@@ -166,9 +164,9 @@ describe('ngAnimateSwap', function() {
166164
// Removing the old element (without inserting a new one).
167165
$rootScope.$apply('value = null');
168166
expect(scopeTwo.$$destroyed).toBe(true);
169-
}));
167+
});
170168

171-
it('should destroy the previous scope when swapping elements', inject(function() {
169+
it('should destroy the previous scope when swapping elements', function() {
172170
element = $compile('<div><div ng-animate-swap="value">{{ value }}</div></div>')($rootScope);
173171

174172
$rootScope.$apply('value = 1');
@@ -177,13 +175,34 @@ describe('ngAnimateSwap', function() {
177175

178176
$rootScope.$apply('value = 2');
179177
expect(scopeOne.$$destroyed).toBe(true);
180-
}));
178+
});
181179

180+
it('should work with `ngIf` on the same element', function() {
181+
var tmpl = '<div><div ng-animate-swap="exp" ng-if="true">{{ exp }}</div></div>';
182+
element = $compile(tmpl)($rootScope);
183+
$rootScope.$digest();
182184

183-
describe('animations', function() {
184-
it('should trigger a leave animation followed by an enter animation upon swap',
185-
inject(function() {
185+
var first = element.find('div')[0];
186+
expect(first).toBeFalsy();
187+
188+
$rootScope.exp = 'yes';
189+
$rootScope.$digest();
190+
191+
var second = element.find('div')[0];
192+
expect(second.textContent).toBe('yes');
193+
194+
$rootScope.exp = 'super';
195+
$rootScope.$digest();
186196

197+
var third = element.find('div')[0];
198+
expect(third.textContent).toBe('super');
199+
expect(third).not.toEqual(second);
200+
expect(second.parentNode).toBeFalsy();
201+
});
202+
203+
204+
describe('animations', function() {
205+
it('should trigger a leave animation followed by an enter animation upon swap',function() {
187206
element = $compile('<div><div ng-animate-swap="exp">{{ exp }}</div></div>')($rootScope);
188207
$rootScope.exp = 1;
189208
$rootScope.$digest();
@@ -208,6 +227,6 @@ describe('ngAnimateSwap', function() {
208227
var forth = $animate.queue.shift();
209228
expect(forth.event).toBe('leave');
210229
expect($animate.queue.length).toBe(0);
211-
}));
230+
});
212231
});
213232
});

0 commit comments

Comments
 (0)