fix(ngRepeat): trigger move animation #15072
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1487,7 +1487,13 @@ describe('ngRepeat animations', function() { | |
| $rootScope.items = ['1','3']; | ||
| $rootScope.$digest(); | ||
|
|
||
| while ($animate.queue.length) { | ||
| item = $animate.queue.shift(); | ||
| if (item.event === 'leave') { | ||
| break; | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. A comment to explain why this is needed would be helpful. There was a problem hiding this comment. I am going to revert this code. The ngRepeat tests assume the animation queue is in a certain order. During refactoring of ngRepeat, the order was different in this test so I altered the test. My latest version of ngRepeat, c969ae2, passes the original version of this test. There was a problem hiding this comment. Should tests assume that animation events are queued in a certain order? If not, then perhaps the queue should be tested like this: // Test each queue item's event type
while ($animate.queue.length) {
item = $animate.queue.shift();
switch (item.element.text()) {
case '2':
expect(item.event).toBe('leave');
break;
case '3':
expect(item.event).toBe('move');
break;
default:
expect(item).toBeUndefined();
}
}Maybe we should add a Jasmine matcher in the "ngRepeat Animations" block that compares item.event and item.text() values between two arrays? Should this be new GitHub issue? There was a problem hiding this comment. Apparently, the animation queue order IS different than it used to be.
As far as I can tell, tests should not assume the order of events in the queue. So I propose the following matcher be added to the 'ngRepeat animations' group of tests: beforeEach(function() {
jasmine.addMatchers({
toContainQueueItem: function() {
return {
compare: generateCompare(false),
negativeCompare: generateCompare(true)
};
function generateCompare(isNot) {
/**
* Matcher that checks that the expected element text is in the actual Array of
* animation queue items and that the event matches.
* @param {array} actual
* @param {string} expectedElementText
* @param {string} expectedEvent optional if isNot = true
* @returns {{pass: boolean, message: message}}
*/
return function(actual, expectedElementText, expectedEvent) {
if (expectedEvent === undefined) {
expectedEvent = '';
}
var actualLength = actual.length;
var i;
var item;
var message = valueFn(
'Expected animation queue to ' + (isNot ? 'not ' : '') + 'contain an item where the element\'s text is "'
+ expectedElementText + '"' + (isNot ? '' : ' and the event is "' + expectedEvent + '"')
);
var pass = isNot;
for (i = 0; i < actualLength; i++) {
item = actual[i];
if (item.element.text() === expectedElementText) {
pass = item.event === expectedEvent;
break;
}
}
return {'pass': pass, 'message': message};
};
}
}
});
});The tests should use the matcher like this: expect($animate.queue).not.toContainQueueItem('1');
expect($animate.queue).toContainQueueItem('2', 'leave');
expect($animate.queue).toContainQueueItem('3', 'move');
$animate.queue = [];What does everyone think? |
||
|
|
||
| expect(item.event).toBe('leave'); | ||
| expect(item.element.text()).toBe('2'); | ||
| })); | ||
|
|
@@ -1565,6 +1571,56 @@ describe('ngRepeat animations', function() { | |
| item = $animate.queue.shift(); | ||
| expect(item.event).toBe('move'); | ||
| expect(item.element.text()).toBe('3'); | ||
|
|
||
| item = $animate.queue.shift(); | ||
| expect(item.event).toBe('move'); | ||
| expect(item.element.text()).toBe('1'); | ||
| }) | ||
| ); | ||
|
|
||
| it('should fire off the move animation for filtered items', | ||
|
There was a problem hiding this comment. This description could be more clear. Perhaps: Or something like that. There was a problem hiding this comment. How about this description? |
||
| inject(function($compile, $rootScope, $animate) { | ||
|
|
||
| var item; | ||
|
|
||
| element = $compile(html( | ||
| '<div>' + | ||
| '<div ng-repeat="item in items | filter:search">' + | ||
| '{{ item }}' + | ||
| '</div>' + | ||
| '</div>' | ||
| ))($rootScope); | ||
|
|
||
| $rootScope.items = ['1','2','3']; | ||
| $rootScope.search = ''; | ||
| $rootScope.$digest(); | ||
|
|
||
| item = $animate.queue.shift(); | ||
| expect(item.event).toBe('enter'); | ||
| expect(item.element.text()).toBe('1'); | ||
|
|
||
| item = $animate.queue.shift(); | ||
| expect(item.event).toBe('enter'); | ||
| expect(item.element.text()).toBe('2'); | ||
|
|
||
| item = $animate.queue.shift(); | ||
| expect(item.event).toBe('enter'); | ||
| expect(item.element.text()).toBe('3'); | ||
|
|
||
| $rootScope.search = '3'; | ||
| $rootScope.$digest(); | ||
|
|
||
| item = $animate.queue.shift(); | ||
| expect(item.event).toBe('move'); | ||
| expect(item.element.text()).toBe('3'); | ||
|
|
||
| item = $animate.queue.shift(); | ||
| expect(item.event).toBe('leave'); | ||
| expect(item.element.text()).toBe('1'); | ||
|
|
||
| item = $animate.queue.shift(); | ||
| expect(item.event).toBe('leave'); | ||
| expect(item.element.text()).toBe('2'); | ||
| }) | ||
| ); | ||
|
There was a problem hiding this comment. Please put a newline between the two |
||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please remove this commented-out code?