refactor(modal): use ngAnimate for animation

- Fix scope.$apply call not working previously because scope was already destroyed. Use $rootScope instead. - Move $destroy call nearer to the dom removal. - Remove fallback timer (emulateTime param) Fixes angular-ui#2585 Fixes angular-ui#2674 Fixes angular-ui#2536 Fixes angular-ui#2790 Fixes angular-ui#3182
chrisirhc · Mar 19, 2015 · 5d85752 · 5d85752
1 parent 431b9c7
commit 5d85752
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 33 deletions.
diff --git a/src/modal/modal.js b/src/modal/modal.js
@@ -1,4 +1,4 @@
-angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
+angular.module('ui.bootstrap.modal', [])
 
 /**
  * A helper, internal data structure that acts as a map but also allows getting / removing
@@ -152,8 +152,8 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
     };
   })
 
-  .factory('$modalStack', ['$transition', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
-    function ($transition, $timeout, $document, $compile, $rootScope, $$stackedMap) {
+  .factory('$modalStack', ['$animate', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
+    function ($animate, $timeout, $document, $compile, $rootScope, $$stackedMap) {
 
       var OPENED_MODAL_CLASS = 'modal-open';
 
@@ -187,8 +187,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
         openedWindows.remove(modalInstance);
 
         //remove window DOM element
-        removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, 300, function() {
-          modalWindow.modalScope.$destroy();
+        removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, function() {
           body.toggleClass(OPENED_MODAL_CLASS, openedWindows.length() > 0);
           checkRemoveBackdrop();
         });
@@ -198,28 +197,23 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
           //remove backdrop if no longer needed
           if (backdropDomEl && backdropIndex() == -1) {
             var backdropScopeRef = backdropScope;
-            removeAfterAnimate(backdropDomEl, backdropScope, 150, function () {
-              backdropScopeRef.$destroy();
+            removeAfterAnimate(backdropDomEl, backdropScope, function () {
               backdropScopeRef = null;
             });
             backdropDomEl = undefined;
             backdropScope = undefined;
           }
       }
 
-      function removeAfterAnimate(domEl, scope, emulateTime, done) {
+      function removeAfterAnimate(domEl, scope, done) {
         // Closing animation
         scope.animate = false;
 
-        var transitionEndEventName = $transition.transitionEndEventName;
-        if (transitionEndEventName) {
+        if ($animate.enabled()) {
           // transition out
-          var timeout = $timeout(afterAnimating, emulateTime);
-
-          domEl.bind(transitionEndEventName, function () {
-            $timeout.cancel(timeout);
-            afterAnimating();
-            scope.$apply();
+          // TODO: use .one when upgrading to >= 1.2.21
+          domEl.on('$animate:close', function closeFn() {
+            $rootScope.$evalAsync(afterAnimating);
           });
         } else {
           // Ensure this call is async
@@ -233,6 +227,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
           afterAnimating.done = true;
 
           domEl.remove();
+          scope.$destroy();
           if (done) {
             done();
           }

diff --git a/src/modal/test/modal.spec.js b/src/modal/test/modal.spec.js
@@ -8,14 +8,6 @@ describe('$modal', function () {
     element.trigger(e);
   };
 
-  var waitForBackdropAnimation = function () {
-    inject(function ($transition) {
-      if ($transition.transitionEndEventName) {
-        $timeout.flush();
-      }
-    });
-  };
-
   beforeEach(module('ui.bootstrap.modal'));
   beforeEach(module('template/modal/backdrop.html'));
   beforeEach(module('template/modal/window.html'));
@@ -106,16 +98,20 @@ describe('$modal', function () {
     return modal;
   }
 
-  function close(modal, result) {
+  function close(modal, result, noFlush) {
     var closed = modal.close(result);
-    $timeout.flush();
+    if (!noFlush) {
+      $timeout.flush();
+    }
     $rootScope.$digest();
     return closed;
   }
 
-  function dismiss(modal, reason) {
+  function dismiss(modal, reason, noFlush) {
     var closed = modal.dismiss(reason);
-    $timeout.flush();
+    if (!noFlush) {
+      $timeout.flush();
+    }
     $rootScope.$digest();
     return closed;
   }
@@ -134,7 +130,6 @@ describe('$modal', function () {
 
       expect($document).toHaveModalsOpen(0);
 
-      waitForBackdropAnimation();
       expect($document).not.toHaveBackdrop();
     });
 
@@ -150,7 +145,7 @@ describe('$modal', function () {
 
       expect($document).toHaveModalsOpen(0);
 
-      dismiss(modal, 'closing in test');
+      dismiss(modal, 'closing in test', true);
     });
 
     it('should not throw an exception on a second close', function () {
@@ -165,7 +160,7 @@ describe('$modal', function () {
 
       expect($document).toHaveModalsOpen(0);
 
-      close(modal, 'closing in test');
+      close(modal, 'closing in test', true);
     });
 
     it('should open a modal from templateUrl', function () {
@@ -181,7 +176,6 @@ describe('$modal', function () {
 
       expect($document).toHaveModalsOpen(0);
 
-      waitForBackdropAnimation();
       expect($document).not.toHaveBackdrop();
     });
 
@@ -489,7 +483,6 @@ describe('$modal', function () {
         expect(backdropEl).toHaveClass('in');
 
         dismiss(modal);
-        waitForBackdropAnimation();
 
         modal = open({ template: '<div>With backdrop</div>' });
         backdropEl = $document.find('body > div.modal-backdrop');