Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes all instances of $animate.enter() to follow angular 1.3.x semantics without breaking 1.2.x #170

Merged
merged 2 commits into from
Feb 17, 2015

Conversation

assisrafael
Copy link
Contributor

According to the angular migration guide the $animate.enter() method behavior has changed. This change causes flickering for some Angular 1.3.x apps. I was not able to isolate a small scenario.

@chieffancypants
Copy link
Owner

Nice! Thanks!

chieffancypants added a commit that referenced this pull request Feb 17, 2015
Changes all instances of $animate.enter() to follow angular 1.3.x semantics without breaking 1.2.x
@chieffancypants chieffancypants merged commit 7ce8a1f into chieffancypants:master Feb 17, 2015
@chieffancypants chieffancypants added this to the next version milestone Feb 17, 2015
@joaovieira
Copy link
Contributor

@chieffancypants @assisrafael this does not take into account if parent does not have children. In these situations, the bar is never appended.

According to angular's $AnimateProvider.$get.enter code:

enter: function(element, parent, after, options) {
        applyStyles(element, options);
        after ? after.after(element)
              : parent.prepend(element);
        return asyncPromise();
      }

If parent doesn't have children, the after element will be defined but empty.

@chieffancypants
Copy link
Owner

@joaovieira what do you suggest?

@joaovieira
Copy link
Contributor

@chieffancypants I had another look... Although the migration document suggests to use angular.element() with the lastChild, it is actually not needed, since $animate.enter() already prepares this for us:
https://github.com/angular/bower-angular-animate/blob/v1.3.15/angular-animate.js#L950
https://github.com/angular/bower-angular-animate/blob/v1.3.15/angular-animate.js#L471

Also, if there's no children in the parent element:
$parent[0].lastChild -> null
angular.element($parent[0].lastChild) -> []

Because of this, using simply:

$animate.enter(loadingBarContainer, $parent, $parent[0].lastChild);

Will do. If the lastChild is null, it remains null until the end and the bar is prepended in the parent, if lastChild exists the bar is prepended after it. But if it's [] it does nothing :)

I can create the PR with the test case if needed.

@joaovieira
Copy link
Contributor

Sorry, seems like that won't work with the default $animate (funny that the ngAnimateMock only delegates to the default and doesn't really behave like the real implementation...). I'd then suggest doing the same check ngAnimate does?

$animate.enter(loadingBarContainer, $parent, $parent[0].lastChild && angular.element($parent[0].lastChild));

@assisrafael
Copy link
Contributor Author

I think you should start by writing a test case that breaks. It will help in your search for the proper solution.

@joaovieira
Copy link
Contributor

PR #218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants