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

refactor(ngAnimateSwap): remove unused dependency and add tests #16565

Closed
wants to merge 2 commits into from
Closed

refactor(ngAnimateSwap): remove unused dependency and add tests #16565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a245483
refactor(ngAnimateSwap): remove usused dependency
gkalpak May 15, 2018
431dc5e
refactor(ngAnimateSwap): simplify scope creation add tests regarding …
gkalpak May 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/ngAnimate/ngAnimateSwap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
* </file>
* </example>
*/
var ngAnimateSwapDirective = ['$animate', '$rootScope', function($animate, $rootScope) {
var ngAnimateSwapDirective = ['$animate', function($animate) {
return {
restrict: 'A',
transclude: 'element',
Expand All @@ -104,10 +104,10 @@ var ngAnimateSwapDirective = ['$animate', '$rootScope', function($animate, $root
previousScope = null;
}
if (value || value === 0) {
previousScope = scope.$new();
$transclude(previousScope, function(element) {
previousElement = element;
$animate.enter(element, null, $element);
$transclude(function(clone, childScope) {
previousElement = clone;
previousScope = childScope;
$animate.enter(clone, null, $element);
});
}
});
Expand Down
47 changes: 47 additions & 0 deletions test/ngAnimate/ngAnimateSwapSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,53 @@ describe('ngAnimateSwap', function() {
expect(two).toBeTruthy();
}));

it('should create a new (non-isolate) scope for each inserted clone', inject(function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that this test proves that the new scopes are non-isolate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't an isolate scope be directly derived from the rootScope and make this fail: expect(scopeOne.$parent).toBe(parentScope)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - an isolate scope still has a parent but it doesn't inherit from its parent. So

$parent.foo = 'bar';
$isoChild = $parent.$new(true);
assert($isoChild.foo !== 'bar');
assert($isoChild.$parent.foo === 'bar');

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops 😁

var parentScope = $rootScope.$new();
parentScope.foo = 'bar';

element = $compile('<div><div ng-animate-swap="value">{{ value }}</div></div>')(parentScope);

$rootScope.$apply('value = 1');
var scopeOne = element.find('div').eq(0).scope();
expect(scopeOne.foo).toBe('bar');

$rootScope.$apply('value = 2');
var scopeTwo = element.find('div').eq(0).scope();
expect(scopeTwo.foo).toBe('bar');

expect(scopeOne).not.toBe(scopeTwo);
}));

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

$rootScope.$apply('value = 1');
var scopeOne = element.find('div').eq(0).scope();
expect(scopeOne.$$destroyed).toBe(false);

// Swapping the old element with a new one.
$rootScope.$apply('value = 2');
expect(scopeOne.$$destroyed).toBe(true);

var scopeTwo = element.find('div').eq(0).scope();
expect(scopeTwo.$$destroyed).toBe(false);

// Removing the old element (without inserting a new one).
$rootScope.$apply('value = null');
expect(scopeTwo.$$destroyed).toBe(true);
}));

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

$rootScope.$apply('value = 1');
var scopeOne = element.find('div').eq(0).scope();
expect(scopeOne.$$destroyed).toBe(false);

$rootScope.$apply('value = 2');
expect(scopeOne.$$destroyed).toBe(true);
}));


describe('animations', function() {
it('should trigger a leave animation followed by an enter animation upon swap',
Expand Down