Skip to content

Commit 2670fb8

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 2670fb8

File tree

2 files changed

+51
-11
lines changed

2 files changed

+51
-11
lines changed

src/modal/modal.js

Lines changed: 42 additions & 11 deletions
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
@@ -87,7 +87,8 @@ angular.module('ui.bootstrap.modal', [])
8787
return {
8888
restrict: 'EA',
8989
scope: {
90-
index: '@'
90+
index: '@',
91+
animate: '='
9192
},
9293
replace: true,
9394
transclude: true,
@@ -106,8 +107,8 @@ angular.module('ui.bootstrap.modal', [])
106107
};
107108
}])
108109

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

112113
var OPENED_MODAL_CLASS = 'modal-open';
113114

@@ -140,17 +141,46 @@ angular.module('ui.bootstrap.modal', [])
140141
openedWindows.remove(modalInstance);
141142

142143
//remove window DOM element
143-
modalWindow.modalDomEl.remove();
144+
removeAfterAnimating(modalWindow.modalDomEl, modalWindow.modalScope, function () {
145+
//remove backdrop if no longer needed
146+
if (backdropDomEl && backdropIndex() == -1) {
147+
removeAfterAnimating(backdropDomEl, backdropScope);
148+
backdropDomEl = undefined;
149+
}
150+
});
144151
body.toggleClass(OPENED_MODAL_CLASS, openedWindows.length() > 0);
152+
}
153+
154+
function removeAfterAnimating(domEl, scope, done) {
155+
// Closing animation
156+
scope.animate = false;
145157

146-
//remove backdrop if no longer needed
147-
if (backdropDomEl && backdropIndex() == -1) {
148-
backdropDomEl.remove();
149-
backdropDomEl = undefined;
158+
var transitionEndEventName = $transition.transitionEndEventName;
159+
if (transitionEndEventName) {
160+
// transition out
161+
var timeout = $timeout(afterAnimating, 500);
162+
163+
domEl.bind(transitionEndEventName, function () {
164+
$timeout.cancel(timeout);
165+
afterAnimating();
166+
scope.$apply();
167+
});
168+
} else {
169+
// Ensure this call is async
170+
$timeout(afterAnimating, 0);
150171
}
151172

152-
//destroy scope
153-
modalWindow.modalScope.$destroy();
173+
function afterAnimating() {
174+
if (afterAnimating.done) {
175+
return;
176+
}
177+
afterAnimating.done = true;
178+
179+
domEl.remove();
180+
if (done) {
181+
done();
182+
}
183+
}
154184
}
155185

156186
$document.bind('keydown', function (evt) {
@@ -186,6 +216,7 @@ angular.module('ui.bootstrap.modal', [])
186216
var angularDomEl = angular.element('<div modal-window></div>');
187217
angularDomEl.attr('window-class', modal.windowClass);
188218
angularDomEl.attr('index', openedWindows.length() - 1);
219+
angularDomEl.attr('animate', 'animate');
189220
angularDomEl.html(modal.content);
190221

191222
var modalDomEl = $compile(angularDomEl)(modal.scope);

src/modal/test/modal.spec.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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

@@ -120,6 +121,9 @@ describe('$modal', function () {
120121
dismiss(modal, 'closing in test');
121122

122123
expect($document).toHaveModalsOpen(0);
124+
125+
// Backdrop animation
126+
$timeout.flush();
123127
expect($document).not.toHaveBackdrop();
124128
});
125129

@@ -135,6 +139,9 @@ describe('$modal', function () {
135139
dismiss(modal, 'closing in test');
136140

137141
expect($document).toHaveModalsOpen(0);
142+
143+
// Backdrop animation
144+
$timeout.flush();
138145
expect($document).not.toHaveBackdrop();
139146
});
140147

@@ -144,6 +151,7 @@ describe('$modal', function () {
144151
expect($document).toHaveModalsOpen(1);
145152

146153
triggerKeyDown($document, 27);
154+
$timeout.flush();
147155
$rootScope.$digest();
148156

149157
expect($document).toHaveModalsOpen(0);
@@ -155,6 +163,7 @@ describe('$modal', function () {
155163
expect($document).toHaveModalsOpen(1);
156164

157165
$document.find('body > div.modal-backdrop').click();
166+
$timeout.flush();
158167
$rootScope.$digest();
159168

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

0 commit comments

Comments
 (0)