Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

Commit 1933488

Browse files
chrisirhcpkozlowski-opensource
authored andcommitted
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 #1341
1 parent f75f85a commit 1933488

File tree

2 files changed

+60
-13
lines changed

2 files changed

+60
-13
lines changed

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);

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)