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

feat(ngAnimate): add between callbacks #3830

Closed
wants to merge 1 commit into from

Conversation

mgcrea
Copy link
Contributor

@mgcrea mgcrea commented Sep 1, 2013

We're currently working on the migration of AngularStrap/AngularUI-bootstrap to Bootstrap 3.0 + AngularJS 1.2.0.

We need theses callbacks to be able to properly handle element positioning (ie. tooltips, popovers) while still relying on the $animate.enter/$animate.leave syntax to leverage CSS animations.

Not sure unit testing is required for such a basic patch, will gladly add them if you want.

cc @matsko

@ghost ghost assigned matsko Sep 18, 2013
@matsko
Copy link
Contributor

matsko commented Oct 14, 2013

@mgcrea we're thinking of adding: before / during / after callbacks to the JS animations. This might favour your idea in this issue. Working on a prototype this week. I'll keep you posted.

@mgcrea
Copy link
Contributor Author

mgcrea commented Oct 14, 2013

@matsko I really think this should not be limited to JS animations, I do need theses callbacks for CSS animations, there is no room for JS animations on mobile.

I'm really looking to integrate animations like theses to the modal component. Also it would really be awesome if this could be made into 1.2.0 as it quite postpone any bootstrap rewrite for AngularJS 1.2.

@mgcrea
Copy link
Contributor Author

mgcrea commented Nov 4, 2013

@matsko any news on this? Any chance it will make it into the 1.2.0 release? If you want to check out how this would be used for a tooltip plugin you can look at this specific line.

@matsko
Copy link
Contributor

matsko commented Nov 4, 2013

@mgcrea strangely enough I'm just now thinking of the naming convention for this. I'm adding beforeRemoveClass and beforeAddClass to the callbacks list. Those will be async (you can do an animation can call done before the class is added). Then addClass and removeClass will fire after the class change operation has occurred.

With enter and move I also want to add the callbacks, but I may not want to use beforeEnter and beforeMove since they would not be async (no done method ... the DOM operation happens right away). Maybe having preEnter or preMove. Leave is untouched.

Either way it's not a blocking issue and this final fix before 1.2 should provide you with hooks that will fire before the DOM operation. Although it won't be within the directive, but as extra callbacks within the JS animation.

@mgcrea
Copy link
Contributor Author

mgcrea commented Nov 4, 2013

@matsko, I do like this alternative naming approach which is more explicit.

Although it won't be within the directive, but as extra callbacks within the JS animation.

I'm not sure I understand this correctly, does this mean that $animate.enter won't have an extra callback (as in this PR) that I can use in a blocking-way, but that I will have to define a JS-fallback-animation with the same name as my CSS animations that is doing nothing except that applying proper placement in a callback?

Do you have a branch/plunker where I could test this implementation?

Thanks!

@matsko
Copy link
Contributor

matsko commented Nov 4, 2013

So it would be something like this.

myModule.animation('.tooltip', function() {
  return {
    beforeEnter : function(element) {
       element.addClass('tooltip');
    },
    enter : function(element, done) {
      //your enter animation
      //or you can skip this and just rely on the default CSS animation
    }
  };
});

No plunkr yet, but I should have one later today.

@wilsonjackson
Copy link

Just curious, and maybe this isn't related enough to this particular issue, but is there a possibility of also allowing an animation to specify a callback that gets invoked when all animations are done?

A specific use case would be a js animation that sets a static height, but still uses CSS transitions to animate that height. Ideally it should be able to determine when the transition is complete (or all transitions/animations are complete) and reset that static height to "auto".

@matsko
Copy link
Contributor

matsko commented Nov 22, 2013

@mgcrea please read: #5053 (comment)

@matsko
Copy link
Contributor

matsko commented Nov 28, 2013

Hey guys.

Going to be using DOM events instead of multiple callbacks.

$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.

@matsko
Copy link
Contributor

matsko commented Dec 4, 2013

Closing this for now. Please let me know if you can think of a good way to improve the new DOM events.

@bbshopadmin
Copy link

@matsko any estimation when the events are ready?

@mgcrea
Copy link
Contributor Author

mgcrea commented Jan 9, 2014

@bbshopadmin @matsko, Got it working pretty easily for AngularStrap:

Basically, this commit postponed animation triggering to the the next $digest completion, enabling DOM operations to be run between the DOM insertion & before the animation starts.

You can have a look at the tooltip.js code here, looked like this order of events produced the best results (may have to run another bath of tests against the current master).

Thanks @matsko, I've been looking forward to produce directives leveraging ngAnimate for the past year, and it's really great to finally be able to build things with it.

@matsko
Copy link
Contributor

matsko commented Jan 9, 2014

Hey guys. We should have these for 1.2.9.

@mgcrea
Copy link
Contributor Author

mgcrea commented Jan 22, 2014

Hey guys, somehow what I wrote last time has proven to be quite wrong: I checked on it recently & I do get the tooltip repositioning visible on slow/mobile devices so It was clearly not being postponed. So I'm not sure on what release I worked on at the time I concluded this, but anyway, it's not a proper fix for custom positioning issues on appearing elements.

What's great however is that the v1.2.9 release with requestAnimationFrame solved all my issues since the repaint is now stacked and happen only once, this code does the trick:

          $animate.enter(tipElement, parent, after, function() {});
          scope.$$phase || scope.$digest();
          requestAnimationFrame($tooltip.$applyPlacement);

Note that without the $digest, the animation will start before the repositioning takes place, so somehow triggering a digest must delay the animation enough (thanks to $$postDigest I guess) so that it gets stacked on the next repaint thanks to requestAnimationFrame (had to add a safe option because on some pretty basic service use-cases it could trigger an already-in-progress error).

You can test the difference (an iPad/iPhone may be ideal to reproduce it):

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

Successfully merging this pull request may close these issues.

4 participants