This repository has been archived by the owner on Apr 12, 2024. It is now read-only.
fix(ngRepeat): trigger move animation #15072
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1488,7 +1488,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'); | ||
| })); | ||
|
|
@@ -1566,6 +1572,107 @@ 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 |
||
| it('should maintain the order when the track by expression evaluates to an integer', | ||
| inject(function($compile, $rootScope, $animate, $document, $sniffer, $timeout) { | ||
| if (!$sniffer.transitions) return; | ||
|
|
||
| var item; | ||
| var ss = createMockStyleSheet($document); | ||
|
|
||
| var items = [ | ||
| {id: 1, name: 'A'}, | ||
| {id: 2, name: 'B'}, | ||
| {id: 4, name: 'C'}, | ||
| {id: 3, name: 'D'} | ||
| ]; | ||
|
|
||
| try { | ||
|
|
||
| $animate.enabled(true); | ||
|
|
||
| ss.addRule('.animate-me div', | ||
| 'transition:1s linear all;'); | ||
|
|
||
| element = $compile(html('<div class="animate-me">' + | ||
| '<div ng-repeat="item in items track by item.id">{{ item.name }}</div>' + | ||
| '</div>'))($rootScope); | ||
|
|
||
| $rootScope.items = [items[0], items[1], items[2]]; | ||
| $rootScope.$digest(); | ||
| expect(element.text()).toBe('ABC'); | ||
|
|
||
| $rootScope.items.push(items[3]); | ||
| $rootScope.$digest(); | ||
|
|
||
| expect(element.text()).toBe('ABCD'); // the original order should be preserved | ||
| $animate.flush(); | ||
| $timeout.flush(1500); // 1s * 1.5 closing buffer | ||
| expect(element.text()).toBe('ABCD'); | ||
|
|
||
| $rootScope.items = [items[0], items[1], items[3]]; | ||
| $rootScope.$digest(); | ||
|
|
||
| // The leaving item should maintain it's position until it is removed | ||
|
There was a problem hiding this comment. it's -> its (I know, I added this test ;)) |
||
| expect(element.text()).toBe('ABCD'); | ||
| $animate.flush(); | ||
| $timeout.flush(1500); // 1s * 1.5 closing buffer | ||
| expect(element.text()).toBe('ABD'); | ||
|
|
||
| } finally { | ||
| ss.destroy(); | ||
| } | ||
| }) | ||
| ); | ||
| }); | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
getBlockStart function is no longer used.