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

ngAnimate addClass/removeClass: Callback after addClass, as well as after transitionend #5053

Closed
caitp opened this issue Nov 20, 2013 · 13 comments

Comments

@caitp
Copy link
Contributor

caitp commented Nov 20, 2013

So, there are a few things --- In some cases, adding a class needs to be paired with an actual DOM operation (which can't take place before adding the class, this would lead to an instant jump) --- So having a sort of before transition, after adding transition class callback would be a pretty big deal.

Following this, on most browsers you won't see a transitionend event fired immediately (because no transition will have occurred if the class is added before a DOM operation such as changing the height or position or something of the animated element) happens --- So, if a transitionend were to happen immediately, the first instance should be ignored (if supplied with a "DOM manipulation"/pre-transition callback)

Finally, the proper transitionend/post-duration callback should not trigger the doneCallback until after it's actually completed (or alternatively, why not have $animate.addClass/removeClass/enter/leave() return a promise?

I am not talking about "module animations" registered via module.animation() here, because in some cases these are not practical to use.

Clearly there are some breaking changes listed, although I think these changes could be written in a way which would provide API compatibility with 1.2.0. I may try to get something like this working after matsko@'s ngAnimate fixes are merged.

(These views come from attempting to add some ngAnimate functionality to angular-ui/bootstrap, which has typically required listening for transitionend or waiting for a delay/duration manually)

@matsko
Copy link
Contributor

matsko commented Nov 21, 2013

Adding extra callback fns to $animate.event(...) should do the job. Here is a breakdown of how $animate works right now.

  1. Promises: Yes there are plans to replace the callbacks with promises, but this won't happen anytime soon until the promises API is detached from the digest cycle (DOM Promises with a polyfill for Angular 2.0). The reason why we can't have them now is because it would mean a digest trigger at the end of the animation (which is expensive when you have a bunch of them running at the same time).

  2. Before / After callbacks: There is no callback that gets fired RIGHT before the animation starts, and other developers have asked for it. It would require an API change or perhaps extending the callback param to be key/value object.

Something like:

$animate.enter(element, parent, after, {
  before : function() { }, //before the DOM operation
  after : function() { }, //just after the DOM operation
  close : function(wasCancelled) { } //when the animation is complete
});

the before and after callbacks should give you the hooks to place the class in to avoid any jumping. And this wouldn't cause any breaking changes in 1.2.

Keep in mind that the callbacks would work in conjunction with JS animations as well (so there wouldn't be a callback specific to transitions, but if you're only using transitions then it would be specific to that).

  1. transitionend: So to get this straight you're listening on transitionend to emulate a beforeTransition event? Or are you trying to figure out when the transition ends when the actual animation is done? When the first CSS class is added (something like .ng-enter or .class-add) transitions and keyframe animations are blocked so there will never be a transitionend event. You'll only see it once the animation actually ends or gets cancelled.

@caitp
Copy link
Contributor Author

caitp commented Nov 21, 2013

Thanks for the reply

So yeah, the issue is that without being able to perform any additional DOM manipulation after addClass (but before a reflow, say), the close callback ends up firing pretty much ASAP, and you end up with a jumpy effect. It's not always possible to perform additional DOM manipulation before addClass (for the same reason --- jumpy effect and immediate callback). The "workaround" is to add classes, perform the DOM manipulation in the close callback, and wait an arbitrary length of time (or bind to transitionend, or what have you) --- but these things all kind of suck (and potentially break older browsers who are never going to fire transitionend).

So, I think the "object of callbacks" thing is not terrible (in terms of not breaking the API) and would solve this problem for me (and apparently others), so I guess that's a good thing. I saw the other animation fixes merge today, so if I have time I can see if this solution actually solves the problem tomorrow

@matsko
Copy link
Contributor

matsko commented Nov 28, 2013

@caitp I'm scrapping the solution to treat the callback param as a collection of callbacks. Instead what $animate will do is fire callback functions on the element.

$animate.enter(element, parent);

element.on('$animate:start', function(data) {
  //data.className
  //data.event
});

element.on('$animate:close', function(data) {
  //data.className
  //data.event
  //data.cancelled
});

This way you can listen on animation events in other directives as well as child directives.

This is almost ready and should be done by tomorrow or Friday.

@caitp
Copy link
Contributor Author

caitp commented Nov 28, 2013

I’m not sure I like the idea of using triggered events without something like jQuery.fn.one() to automatically unbind
it.

I can see how it would be simpler to implement, but is it really a good idea?

Caitlin Potter
snowball@defpixel.com

On Nov 28, 2013, at 12:18 AM, Matias Niemelä notifications@github.com wrote:

@caitp I'm scrapping the solution to treat the callback param as a collection of callbacks. Instead what $animate will do is fire callback functions on the element.

$animate.enter(element, parent);

element.on('$animate:start', function(data) {
//data.className
//data.event
});

element.on('$animate:end', function(data) {
//data.className
//data.event
});
This way you can listen on animation events in other directives as well as child directives.

This is almost ready and should be done by tomorrow or Friday.


Reply to this email directly or view it on GitHub.

@matsko
Copy link
Contributor

matsko commented Nov 28, 2013

Yeah I thought about that too. I'm also planning on putting in one() for jqLite.

@matsko
Copy link
Contributor

matsko commented Nov 28, 2013

Alright so jqLite.one() is accepted by the Angular team. Both should be ready by Friday.

@nissoh
Copy link

nissoh commented Jan 6, 2014

i'm not sure if this is related.
i can't seem to catch the done callback on this particular animation.

http://plnkr.co/edit/7Jdmq1Q1zesPk6qZee0Q?p=preview

@matsko
Copy link
Contributor

matsko commented Jan 8, 2014

@nissoh very strange indeed. Can you possibly open a new issue and reference me in there please?

@bbshopadmin
Copy link

@nissoh did you find a way to catch the done callback?

@bbshopadmin
Copy link

@matsko i posted a related question regarding callbacks here: #5685

@matsko
Copy link
Contributor

matsko commented Jan 9, 2014

@bbshopadmin this what we're planning on doing: 3830

@matsko
Copy link
Contributor

matsko commented Jan 22, 2014

Once $animate:close gets added to the other existing callbacks in 1.2.10 then we can close this.

matsko added a commit to matsko/angular.js that referenced this issue Jan 22, 2014
@matsko matsko closed this as completed in ca6b7d0 Jan 24, 2014
juliemr pushed a commit to juliemr/angular.js that referenced this issue Jan 28, 2014
@yahyazini
Copy link

Thank you so much @matsko this has helped a lot!

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

Successfully merging a pull request may close this issue.

5 participants