Skip to content

Commit 86037ce

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 e1c9dc0 commit 86037ce

File tree

2 files changed

+60
-13
lines changed

2 files changed

+60
-13
lines changed

Diff for: src/modal/modal.js

+48-13
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

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

142143
//remove window DOM element
143-
modalWindow.modalDomEl.remove();
144+
removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, 300, checkRemoveBackdrop);
144145
body.toggleClass(OPENED_MODAL_CLASS, openedWindows.length() > 0);
146+
}
147+
148+
function checkRemoveBackdrop() {
149+
//remove backdrop if no longer needed
150+
if (backdropDomEl && backdropIndex() == -1) {
151+
var backdropScopeRef = backdropScope;
152+
removeAfterAnimate(backdropDomEl, backdropScope, 150, function () {
153+
backdropScopeRef.$destroy();
154+
backdropScopeRef = null;
155+
});
156+
backdropDomEl = undefined;
157+
backdropScope = undefined;
158+
}
159+
}
145160

146-
//remove backdrop if no longer needed
147-
if (backdropDomEl && backdropIndex() == -1) {
148-
backdropDomEl.remove();
149-
backdropDomEl = undefined;
161+
function removeAfterAnimate(domEl, scope, emulateTime, done) {
162+
// Closing animation
163+
scope.animate = false;
150164

151-
backdropScope.$destroy();
152-
backdropScope = undefined;
165+
var transitionEndEventName = $transition.transitionEndEventName;
166+
if (transitionEndEventName) {
167+
// transition out
168+
var timeout = $timeout(afterAnimating, emulateTime);
169+
170+
domEl.bind(transitionEndEventName, function () {
171+
$timeout.cancel(timeout);
172+
afterAnimating();
173+
scope.$apply();
174+
});
175+
} else {
176+
// Ensure this call is async
177+
$timeout(afterAnimating, 0);
153178
}
154179

155-
//destroy scope
156-
modalWindow.modalScope.$destroy();
180+
function afterAnimating() {
181+
if (afterAnimating.done) {
182+
return;
183+
}
184+
afterAnimating.done = true;
185+
186+
domEl.remove();
187+
if (done) {
188+
done();
189+
}
190+
}
157191
}
158192

159193
$document.bind('keydown', function (evt) {
@@ -191,6 +225,7 @@ angular.module('ui.bootstrap.modal', [])
191225
var angularDomEl = angular.element('<div modal-window></div>');
192226
angularDomEl.attr('window-class', modal.windowClass);
193227
angularDomEl.attr('index', openedWindows.length() - 1);
228+
angularDomEl.attr('animate', 'animate');
194229
angularDomEl.html(modal.content);
195230

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

Diff for: src/modal/test/modal.spec.js

+12
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);
@@ -386,6 +395,9 @@ describe('$modal', function () {
386395
expect(backdropEl).toHaveClass('in');
387396

388397
dismiss(modal);
398+
// Backdrop animation
399+
$timeout.flush();
400+
389401
modal = open({ template: '<div>With backdrop</div>' });
390402
backdropEl = $document.find('body > div.modal-backdrop');
391403
expect(backdropEl).not.toHaveClass('in');

0 commit comments

Comments
 (0)