Skip to content

Commit 2acc925

Browse files
committed
fix(modal): Support close animation
Tests now have to wait for animation before checking for open modals. Tests should actually check for the 'in' class. Setting up both the transition end handler and a timeout ensures that the modal is always closed even if there was an issue with the transition. This was the implementation used by Twitter Bootstrap modal JS code. Note that unbinding the transition end event within the event handler isn't necessary, and, worse is currently buggy in jqLite (see angular/angular.js#5109 ). Note that the scope is already destroyed when the dom is removed so the $destroy call isn't needed. Closes angular-ui#1341
1 parent 0b39994 commit 2acc925

File tree

2 files changed

+30
-8
lines changed

2 files changed

+30
-8
lines changed

src/modal/modal.js

+26-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
angular.module('ui.bootstrap.modal', [])
1+
angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
22

33
/**
44
* A helper, internal data structure that acts as a map but also allows getting / removing
@@ -106,8 +106,8 @@ angular.module('ui.bootstrap.modal', [])
106106
};
107107
}])
108108

109-
.factory('$modalStack', ['$document', '$compile', '$rootScope', '$$stackedMap',
110-
function ($document, $compile, $rootScope, $$stackedMap) {
109+
.factory('$modalStack', ['$transition', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
110+
function ($transition, $timeout, $document, $compile, $rootScope, $$stackedMap) {
111111

112112
var OPENED_MODAL_CLASS = 'modal-open';
113113

@@ -140,17 +140,36 @@ angular.module('ui.bootstrap.modal', [])
140140
openedWindows.remove(modalInstance);
141141

142142
//remove window DOM element
143-
modalWindow.modalDomEl.remove();
143+
removeAfterAnimating(modalWindow.modalDomEl, modalWindow.modalScope);
144144
body.toggleClass(OPENED_MODAL_CLASS, openedWindows.length() > 0);
145145

146146
//remove backdrop if no longer needed
147147
if (backdropDomEl && backdropIndex() == -1) {
148-
backdropDomEl.remove();
148+
removeAfterAnimating(backdropDomEl, backdropScope);
149149
backdropDomEl = undefined;
150150
}
151+
}
151152

152-
//destroy scope
153-
modalWindow.modalScope.$destroy();
153+
function removeAfterAnimating(domEl, scope) {
154+
// Closing animation
155+
$timeout(function () {
156+
scope.animate = false;
157+
});
158+
159+
var transitionEndEventName = $transition.transitionEndEventName;
160+
if (transitionEndEventName) {
161+
// transition out
162+
var timeout = $timeout(function () {
163+
domEl.remove();
164+
}, 500);
165+
166+
domEl.bind(transitionEndEventName, function () {
167+
$timeout.cancel(timeout);
168+
domEl.remove();
169+
});
170+
} else {
171+
domEl.remove();
172+
}
154173
}
155174

156175
$document.bind('keydown', function (evt) {

src/modal/test/modal.spec.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
describe('$modal', function () {
1+
ddescribe('$modal', function () {
22
var $rootScope, $document, $compile, $templateCache, $timeout, $q;
33
var $modal, $modalProvider;
44

@@ -104,6 +104,7 @@ describe('$modal', function () {
104104

105105
function dismiss(modal, reason) {
106106
modal.dismiss(reason);
107+
$timeout.flush();
107108
$rootScope.$digest();
108109
}
109110

@@ -144,6 +145,7 @@ describe('$modal', function () {
144145
expect($document).toHaveModalsOpen(1);
145146

146147
triggerKeyDown($document, 27);
148+
$timeout.flush();
147149
$rootScope.$digest();
148150

149151
expect($document).toHaveModalsOpen(0);
@@ -155,6 +157,7 @@ describe('$modal', function () {
155157
expect($document).toHaveModalsOpen(1);
156158

157159
$document.find('body > div.modal-backdrop').click();
160+
$timeout.flush();
158161
$rootScope.$digest();
159162

160163
expect($document).toHaveModalsOpen(0);

0 commit comments

Comments
 (0)