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

Commit 5d85752

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
1 parent 431b9c7 commit 5d85752

File tree

2 files changed

+21
-33
lines changed

2 files changed

+21
-33
lines changed

Diff for: src/modal/modal.js

+11-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
@@ -152,8 +152,8 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
152152
};
153153
})
154154

155-
.factory('$modalStack', ['$transition', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
156-
function ($transition, $timeout, $document, $compile, $rootScope, $$stackedMap) {
155+
.factory('$modalStack', ['$animate', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
156+
function ($animate, $timeout, $document, $compile, $rootScope, $$stackedMap) {
157157

158158
var OPENED_MODAL_CLASS = 'modal-open';
159159

@@ -187,8 +187,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
187187
openedWindows.remove(modalInstance);
188188

189189
//remove window DOM element
190-
removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, 300, function() {
191-
modalWindow.modalScope.$destroy();
190+
removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, function() {
192191
body.toggleClass(OPENED_MODAL_CLASS, openedWindows.length() > 0);
193192
checkRemoveBackdrop();
194193
});
@@ -198,28 +197,23 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
198197
//remove backdrop if no longer needed
199198
if (backdropDomEl && backdropIndex() == -1) {
200199
var backdropScopeRef = backdropScope;
201-
removeAfterAnimate(backdropDomEl, backdropScope, 150, function () {
202-
backdropScopeRef.$destroy();
200+
removeAfterAnimate(backdropDomEl, backdropScope, function () {
203201
backdropScopeRef = null;
204202
});
205203
backdropDomEl = undefined;
206204
backdropScope = undefined;
207205
}
208206
}
209207

210-
function removeAfterAnimate(domEl, scope, emulateTime, done) {
208+
function removeAfterAnimate(domEl, scope, done) {
211209
// Closing animation
212210
scope.animate = false;
213211

214-
var transitionEndEventName = $transition.transitionEndEventName;
215-
if (transitionEndEventName) {
212+
if ($animate.enabled()) {
216213
// transition out
217-
var timeout = $timeout(afterAnimating, emulateTime);
218-
219-
domEl.bind(transitionEndEventName, function () {
220-
$timeout.cancel(timeout);
221-
afterAnimating();
222-
scope.$apply();
214+
// TODO: use .one when upgrading to >= 1.2.21
215+
domEl.on('$animate:close', function closeFn() {
216+
$rootScope.$evalAsync(afterAnimating);
223217
});
224218
} else {
225219
// Ensure this call is async
@@ -233,6 +227,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
233227
afterAnimating.done = true;
234228

235229
domEl.remove();
230+
scope.$destroy();
236231
if (done) {
237232
done();
238233
}

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

+10-17
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

@@ -489,7 +483,6 @@ describe('$modal', function () {
489483
expect(backdropEl).toHaveClass('in');
490484

491485
dismiss(modal);
492-
waitForBackdropAnimation();
493486

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

0 commit comments

Comments
 (0)