Skip to content

Commit 80a3dcd

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 c4d0e2a commit 80a3dcd

File tree

2 files changed

+51
-11
lines changed

2 files changed

+51
-11
lines changed

src/modal/modal.js

+42-11
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
@@ -78,7 +78,8 @@ angular.module('ui.bootstrap.modal', [])
7878
return {
7979
restrict: 'EA',
8080
scope: {
81-
index: '@'
81+
index: '@',
82+
animate: '='
8283
},
8384
replace: true,
8485
transclude: true,
@@ -105,8 +106,8 @@ angular.module('ui.bootstrap.modal', [])
105106
};
106107
}])
107108

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

111112
var OPENED_MODAL_CLASS = 'modal-open';
112113

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

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

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

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

155185
$document.bind('keydown', function (evt) {
@@ -185,6 +215,7 @@ angular.module('ui.bootstrap.modal', [])
185215
var angularDomEl = angular.element('<div modal-window></div>');
186216
angularDomEl.attr('window-class', modal.windowClass);
187217
angularDomEl.attr('index', openedWindows.length() - 1);
218+
angularDomEl.attr('animate', 'animate');
188219
angularDomEl.html(modal.content);
189220

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

src/modal/test/modal.spec.js

+9
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').click();
166+
$timeout.flush();
158167
$rootScope.$digest();
159168

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

0 commit comments

Comments
 (0)