angular-ui / bootstrap

PLEASE READ THE PROJECT STATUS BELOW. Native AngularJS (Angular) directives for Bootstrap. Smaller footprint (20kB gzipped), no 3rd party JS dependencies (jQuery, bootstrap JS) required. Please read the README.md file before submitting an issue!
http://angular-ui.github.io/bootstrap/
MIT License
14.29k stars 6.73k forks source link

Modal doesn't use ng-enter/ng-leave animations #4183

Closed dpogue closed 8 years ago

dpogue commented 9 years ago

This current modal animation code seems significantly overcomplicated, and doesn't work with the standard Angular way of handling elements being added/removed from the page.

I suspect all of the additional complexity is for compatibility with vanilla bootstrap CSS, but it makes it difficult for people who want to use the modal JS code with their own custom stylesheets.

wesleycho commented 9 years ago

I'm confused by this assertion - the $modal service uses ngAnimate's $animateCss/$animate under the hood to add & remove CSS classes. The use of ngAnimate can be seen here, where it properly adds the evented classes.

We need more info on exactly what is wrong here.

dpogue commented 9 years ago

The modal isn't added to the page using $animate.enter (so .ng-enter animations never play), and the leave event with $animateCss doesn't cause .ng-leave animations to play.

Example CSS (that fails to animate):

.modal {
    transition: transform 0.5s, opacity 0.4s;
    will-change: transform, opacity;
}

.modal.ng-enter {
    opacity: 0;
    transform: translate(0, -50%);
}

.modal.ng-enter.ng-enter-active {
    opacity: 1;
    transform: translate(0, 0);
}

.modal.ng-leave {
    opacity: 1;
    transform: translate(0, 0);
}

.modal.ng-leave.ng-leave-active {
    opacity: 0;
    transform: translate(0, -50%);
}
wesleycho commented 9 years ago

The library does not include CSS, so that cannot be a solution.

I highly suspect that this feature request would be near impossible to support given how Bootstrap's transitions work with the modal & the CSS that goes along with it. It should be enough to hook into the -add-active and -remove-active class suffixes that $animateCss/$animate are adding.

wesleycho commented 8 years ago

Looks like I reference the wrong issue from the commit - this should be addressed by 742132a2c84149a84ca8a299ec2a40875e7448df.