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

Commit 0d8820b

Browse files
committed
refactor(modal): use ngAnimate for animation
- Fix scope.$apply call not working previously because scope was already destroyed. Use $rootScope instead. - Move $destroy call nearer to the dom removal. - Remove fallback timer (emulateTime param) Fixes #2585 Fixes #2674 Fixes #2536 Fixes #2790 Fixes #3182 Closes #2724
1 parent 0bcd30c commit 0d8820b

File tree

2 files changed

+20
-35
lines changed

2 files changed

+20
-35
lines changed

Diff for: src/modal/modal.js

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

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

158-
.factory('$modalStack', ['$transition', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
159-
function ($transition, $timeout, $document, $compile, $rootScope, $$stackedMap) {
158+
.factory('$modalStack', ['$animate', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
159+
function ($animate, $timeout, $document, $compile, $rootScope, $$stackedMap) {
160160

161161
var OPENED_MODAL_CLASS = 'modal-open';
162162

@@ -190,8 +190,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
190190
openedWindows.remove(modalInstance);
191191

192192
//remove window DOM element
193-
removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, 300, function() {
194-
modalWindow.modalScope.$destroy();
193+
removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, function() {
195194
body.toggleClass(OPENED_MODAL_CLASS, openedWindows.length() > 0);
196195
checkRemoveBackdrop();
197196
});
@@ -201,28 +200,22 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
201200
//remove backdrop if no longer needed
202201
if (backdropDomEl && backdropIndex() == -1) {
203202
var backdropScopeRef = backdropScope;
204-
removeAfterAnimate(backdropDomEl, backdropScope, 150, function () {
205-
backdropScopeRef.$destroy();
203+
removeAfterAnimate(backdropDomEl, backdropScope, function () {
206204
backdropScopeRef = null;
207205
});
208206
backdropDomEl = undefined;
209207
backdropScope = undefined;
210208
}
211209
}
212210

213-
function removeAfterAnimate(domEl, scope, emulateTime, done) {
211+
function removeAfterAnimate(domEl, scope, done) {
214212
// Closing animation
215213
scope.animate = false;
216214

217-
var transitionEndEventName = $transition.transitionEndEventName;
218-
if (transitionEndEventName) {
215+
if ($animate.enabled()) {
219216
// transition out
220-
var timeout = $timeout(afterAnimating, emulateTime);
221-
222-
domEl.bind(transitionEndEventName, function () {
223-
$timeout.cancel(timeout);
224-
afterAnimating();
225-
scope.$apply();
217+
domEl.one('$animate:close', function closeFn() {
218+
$rootScope.$evalAsync(afterAnimating);
226219
});
227220
} else {
228221
// Ensure this call is async
@@ -236,6 +229,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
236229
afterAnimating.done = true;
237230

238231
domEl.remove();
232+
scope.$destroy();
239233
if (done) {
240234
done();
241235
}

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

+10-19
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,6 @@ describe('$modal', function () {
88
element.trigger(e);
99
};
1010

11-
var waitForBackdropAnimation = function () {
12-
inject(function ($transition) {
13-
if ($transition.transitionEndEventName) {
14-
$timeout.flush();
15-
}
16-
});
17-
};
18-
1911
beforeEach(module('ui.bootstrap.modal'));
2012
beforeEach(module('template/modal/backdrop.html'));
2113
beforeEach(module('template/modal/window.html'));
@@ -106,16 +98,20 @@ describe('$modal', function () {
10698
return modal;
10799
}
108100

109-
function close(modal, result) {
101+
function close(modal, result, noFlush) {
110102
var closed = modal.close(result);
111-
$timeout.flush();
103+
if (!noFlush) {
104+
$timeout.flush();
105+
}
112106
$rootScope.$digest();
113107
return closed;
114108
}
115109

116-
function dismiss(modal, reason) {
110+
function dismiss(modal, reason, noFlush) {
117111
var closed = modal.dismiss(reason);
118-
$timeout.flush();
112+
if (!noFlush) {
113+
$timeout.flush();
114+
}
119115
$rootScope.$digest();
120116
return closed;
121117
}
@@ -134,7 +130,6 @@ describe('$modal', function () {
134130

135131
expect($document).toHaveModalsOpen(0);
136132

137-
waitForBackdropAnimation();
138133
expect($document).not.toHaveBackdrop();
139134
});
140135

@@ -150,7 +145,7 @@ describe('$modal', function () {
150145

151146
expect($document).toHaveModalsOpen(0);
152147

153-
dismiss(modal, 'closing in test');
148+
dismiss(modal, 'closing in test', true);
154149
});
155150

156151
it('should not throw an exception on a second close', function () {
@@ -165,7 +160,7 @@ describe('$modal', function () {
165160

166161
expect($document).toHaveModalsOpen(0);
167162

168-
close(modal, 'closing in test');
163+
close(modal, 'closing in test', true);
169164
});
170165

171166
it('should open a modal from templateUrl', function () {
@@ -181,7 +176,6 @@ describe('$modal', function () {
181176

182177
expect($document).toHaveModalsOpen(0);
183178

184-
waitForBackdropAnimation();
185179
expect($document).not.toHaveBackdrop();
186180
});
187181

@@ -244,8 +238,6 @@ describe('$modal', function () {
244238
function openAndCloseModalWithAutofocusElement() {
245239
var modal = open({template: '<div><input type="text" id="auto-focus-element" autofocus></div>'});
246240

247-
waitForBackdropAnimation();
248-
249241
expect(angular.element('#auto-focus-element')).toHaveFocus();
250242

251243
close(modal, 'closed ok');
@@ -505,7 +497,6 @@ describe('$modal', function () {
505497
expect(backdropEl).toHaveClass('in');
506498

507499
dismiss(modal);
508-
waitForBackdropAnimation();
509500

510501
modal = open({ template: '<div>With backdrop</div>' });
511502
backdropEl = $document.find('body > div.modal-backdrop');

0 commit comments

Comments
 (0)