refactor(modal): use $animate for animation

[chrisirhc]

• Use decorator to customize animations
• Need to sort of wrap $animate because $animate doesn't support animations that call other animations (.leave can't call .removeClass and expect the removeClass animation to execute)
• Fix tests
• Remove unused dependencies (transition)
• Remove change in demo

The demo change is included just for convenience.

[Foxandxss]

Yeeeeah, that is what I wanted to see.

Two things I noticed. There is no need to load ui.bootstrap.transition and both modalBackdrop and modalWindow still inject the $timeout.

[pkozlowski-opensource]

Wow, it starts to look interesting. I still wonder if it would be somehow possible to keep the in class out of the JavaScript code... but I might be idealistic here...

[chrisirhc]

@pkozlowski-opensource , unfortunately, if we want to allow users to be able to customize/replace the animations, they have to be in the JS code.

There's also no way of telling which animation has ended without checking the event's data (className, and event name), so it's either the in class is in both the template and the JS code, or just in the JS code.

I think if we can restrict class names to be only in animation code (perhaps only used by "animator" services), I think it won't be so bad since AngularJS 1.2 animations use classes quite extensively.

[Foxandxss]

I am not so sure if I understood the problem so maybe what I am going to say have no sense, but here we go:

If the class needs to be hardcoded. Can't we just configure it on a provider? So if someone decide to use don't know, foundation and they need the in class to be foo he can just configure ui-bootstrap and set that the animation class is foo. Simple as that.

Maybe I am mixing things, but brainstorming is free :P

[chrisirhc]

@Foxandxss I've modified the animator so that one can override just the visibleClass.

[Foxandxss]

@chrisirhc I like the end result. Can you explain to me (for learning purposes) why $decorate it? A provider won't work in that case?

[chrisirhc]

@Foxandxss sure, I used $decorate here so that the animations can be overridden. This implementation sets up a default animation behavior.

The intention is that users can read this implementation and be able to do the same $decorate to replace this animation with their own animation methods.

[Narretz]

Hi, I got a question about this. I am currently experimenting with using the modal as the basis for an off-canvas menu. For this I need to fire additional animations on different DOM elements at the same time the modal is opening. Would something like this be possible with this code?

[chrisirhc]

@Narretz , yes, you can do all the animations you need on the element and any sub-elements and call the done function provided via the arguments when you're done.
Right now, the default animation is to add the visibleClass to the element.

[Narretz]

Okay, but what I actually need to do is sync animations on the body and the modal, so no descendants. I need to test this then.

I merged your changes into current master and put everything in a plunker to test. There's one problem: when I first open the modal, the visibleClass does not get set: click on the 'open' button, modal is not visible. Hit ESC to close, click 'open' again -> modal shows.

I logged the element in 'afterEnter', and it appears to be a fragment / disconnected DOM tree.

It appears to be a problem with the templateCache: in the first run, the template is an ajax request, and in subsequent 'openings' it is cached already.

http://plnkr.co/edit/mIfavH64Z21132YrEh6K?p=preview

In my app, the class doesn't get added at all, so there might be another problem.

[Narretz]

Update:

it works when you put the calls to the modalAnimator into the directive link functions (backdrop and window). Haven't tested yet if that breaks something else, but it makes more sense to me.

well, apparently it doesn't always work ... need to investigate

edit: okay, so apparently when I include ngAnimate in my app module's dependencies, it doesn't work. Without it does. Does that make any sense?

[Narretz]

Ok, I have misunderstood how the integration works. From what I can say, $animate in the modal actually does not use the enter() and leave() classes and the css animations, but it simply handles adding and removing the modal element, while the transition / animations come from bootstrap, at least in the default implementation.

[chrisirhc]

@Narretz , that's right, it doesn't use the enter and leave classes because these classes don't exist on the bootstrap stylesheet unless you add them manually to your custom stylesheet.

If you want to customize the animation, you need to override the default implementation of modalAnimator using a decorator just like how the default animation is provided. In the default animation implementation, it uses the in class that is provided by the bootstrap CSS.

Is that clear? Does that work for your case of custom animations?

[Narretz]

Well, I have test if I can use $animate like I need.
Regardless, there's still the issue (at least I see it in the plunker) that the modal isn't displayed correctly when the page first loads and you first open it (in the default implementation).

There is also another thing: in removeModalWindow(), the check for removing the backdrop is called in the modal window leave callbck:

    modalAnimator.leave(modalWindow.modalDomEl, function () {
      modalWindow.modalScope.$destroy();
      body.toggleClass(OPENED_MODAL_CLASS, openedWindows.length() > 0);
      checkRemoveBackdrop();
    });

But I think it should be called after / before it, so if the backdrop needs to be removed, both animations happen roughly at the same time. This is important if you want to change the animations in the $delegate.

[ashaffer]

+1 for acceptance on this PR. I'd really like to be able to disable all my animations in my e2e tests, and ui-bootstraps implementation is currently preventing this.

[porjo]

+1

I simply want to disable the modal slide-in effect. This solution worked for me, but looks like a bit of hack: http://stackoverflow.com/a/23373622

[Narretz]

I played around with this a little more, and it works quite well (with the fixes I mentioned earlier): There's just one problem: It currently doesn't work very well to $animate.addClass an element inside the modal when you $animate.enter the modal window, because structural animations block all other animations inside them.

There was some talk about explicitly enabling non-blocking animations here: angular/angular.js#7105

[chrisirhc]

Closing this in favor of #2724. The new PR won't make use of $animate.enter and hence won't block animations in the modal. It's also minimal changes to the current code.