fix($animate): insert elements at the start of the parent container instead of at the end #6663
Conversation
|
Matias! |
| @@ -128,7 +128,7 @@ var $AnimateProvider = ['$provide', function($provide) { | |||
| if (!parent || !parent[0]) { | |||
| parent = after.parent(); | |||
There was a problem hiding this comment.
semi unrelated, but does this actually work?
if (after) {
...
} else {
...
parent = after.parent();looks to me like after could never exist when you are here?
I think it might be left over from when this was using the native js insert fuction.
also there is a comment there that might need to be updated from append to prepend
* @param {DOMElement} parent the parent element which will append the element as
* a child (if the after element is not present)There was a problem hiding this comment.
semi unrelated, but does this actually work?
No, an issue has been raised about this before, currently $animate.enter() will throw if either the parent or after are not supplied. That entire branch should probably be removed, or at least clarified to explain that failure is due to both parent and after being undefined.
There was a problem hiding this comment.
Are you refering to this issue? #6275
There was a problem hiding this comment.
I've refactored enter. I don't think we need any special error to be thrown if both parent + after are missing since there could be a deeper error incase an empty jqLite/jQuery object wrapper is provided anyway, or if a comment node is in place.
|
Also closes #6274 |
| element.append(jqLite('<div> ' + i + '</div>')); | ||
| } | ||
|
|
||
| var child = $compile('<div>first</div>')($rootScope); |
There was a problem hiding this comment.
what's the purpose of compilation here? it seems unnecessary.
|
no matter what, this PR needs better commit message explaining what this actually means for the developer and possibly even a note in api docs. |
|
the commit message has not been updated as I request ;-) |
|
imagine that you are a developer that knows only very little about $animate, the commit message makes it hard for me to understand what to do. can you make the text more clear and add an example like: |
|
perhaps it could be worded something like: BREAKING CHANGE: $animate will no longer default the after parameter to the last element of the parent container. When after is not specified the new element will be inserted before the first element of the parent container. To update existing code change any instances of $animate.enter() or $animate.move() from: $animate.enter(element, parent);to $animate.enter(element, parent, angular.element(parent[0].lastChild)); |
…nstead of at the end With 1.2.x, $animate.enter and $animate.move both insert the element at the end of the provided parent container element when only the parent element is provided. If an after element is provided then they will place the inserted element after that one. This works fine, but there is no way to place an item at the top of the provided parent container using both these functions. Closes angular#4934 Closes angular#6275 BREAKING CHANGE: $animate will no longer default the after parameter to the last element of the parent container. Instead, when after is not specified, the new element will be inserted as the first child of the parent container. To update existing code change any instances of $animate.enter() or $animate.move() from: `$animate.enter(element, parent);` to: `$animate.enter(element, parent, angular.element(parent[0].lastChild));`
|
MERGED |
|
Landed as 1cb8584 |
With 1.2.x,
$animate.enterand$animate.moveboth insert the element at the end of the providedparent container element when only the
parentelement is provided. If anafterelement is providedthen they will place the inserted element after that one. This works fine, but there is no way to
place an item at the top of the provided parent container using these two APIs.
With this change, if the
afterargument is not specified for either$animate.enteror$animate.move,the new child element will be inserted into the first position of the parent container element.
Closes #4934
Closes #6275
BREAKING CHANGE: $animate will no longer default the after parameter to the last element of the parent
container. Instead, when after is not specified, the new element will be inserted as the first child of
the parent container.
To update existing code, change all instances of
$animate.enter()or$animate.move()from:$animate.enter(element, parent);to:
$animate.enter(element, parent, angular.element(parent[0].lastChild));