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

Commit cd30758

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
1 parent 6aaeb10 commit cd30758

File tree

2 files changed

+21
-33
lines changed

2 files changed

+21
-33
lines changed

src/modal/modal.js

Lines changed: 11 additions & 16 deletions
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
@@ -131,8 +131,8 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
131131
};
132132
})
133133

134-
.factory('$modalStack', ['$transition', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
135-
function ($transition, $timeout, $document, $compile, $rootScope, $$stackedMap) {
134+
.factory('$modalStack', ['$animate', '$timeout', '$document', '$compile', '$rootScope', '$$stackedMap',
135+
function ($animate, $timeout, $document, $compile, $rootScope, $$stackedMap) {
136136

137137
var OPENED_MODAL_CLASS = 'modal-open';
138138

@@ -166,8 +166,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
166166
openedWindows.remove(modalInstance);
167167

168168
//remove window DOM element
169-
removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, 300, function() {
170-
modalWindow.modalScope.$destroy();
169+
removeAfterAnimate(modalWindow.modalDomEl, modalWindow.modalScope, function() {
171170
body.toggleClass(OPENED_MODAL_CLASS, openedWindows.length() > 0);
172171
checkRemoveBackdrop();
173172
});
@@ -177,28 +176,23 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
177176
//remove backdrop if no longer needed
178177
if (backdropDomEl && backdropIndex() == -1) {
179178
var backdropScopeRef = backdropScope;
180-
removeAfterAnimate(backdropDomEl, backdropScope, 150, function () {
181-
backdropScopeRef.$destroy();
179+
removeAfterAnimate(backdropDomEl, backdropScope, function () {
182180
backdropScopeRef = null;
183181
});
184182
backdropDomEl = undefined;
185183
backdropScope = undefined;
186184
}
187185
}
188186

189-
function removeAfterAnimate(domEl, scope, emulateTime, done) {
187+
function removeAfterAnimate(domEl, scope, done) {
190188
// Closing animation
191189
scope.animate = false;
192190

193-
var transitionEndEventName = $transition.transitionEndEventName;
194-
if (transitionEndEventName) {
191+
if ($animate.enabled()) {
195192
// transition out
196-
var timeout = $timeout(afterAnimating, emulateTime);
197-
198-
domEl.bind(transitionEndEventName, function () {
199-
$timeout.cancel(timeout);
200-
afterAnimating();
201-
scope.$apply();
193+
// TODO: use .one when upgrading to >= 1.2.21
194+
domEl.on('$animate:close', function closeFn() {
195+
$rootScope.$evalAsync(afterAnimating);
202196
});
203197
} else {
204198
// Ensure this call is async
@@ -212,6 +206,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.transition'])
212206
afterAnimating.done = true;
213207

214208
domEl.remove();
209+
scope.$destroy();
215210
if (done) {
216211
done();
217212
}

src/modal/test/modal.spec.js

Lines changed: 10 additions & 17 deletions
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,15 +98,19 @@ describe('$modal', function () {
10698
return modal;
10799
}
108100

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

115-
function dismiss(modal, reason) {
109+
function dismiss(modal, reason, noFlush) {
116110
modal.dismiss(reason);
117-
$timeout.flush();
111+
if (!noFlush) {
112+
$timeout.flush();
113+
}
118114
$rootScope.$digest();
119115
}
120116

@@ -132,7 +128,6 @@ describe('$modal', function () {
132128

133129
expect($document).toHaveModalsOpen(0);
134130

135-
waitForBackdropAnimation();
136131
expect($document).not.toHaveBackdrop();
137132
});
138133

@@ -148,7 +143,7 @@ describe('$modal', function () {
148143

149144
expect($document).toHaveModalsOpen(0);
150145

151-
dismiss(modal, 'closing in test');
146+
dismiss(modal, 'closing in test', true);
152147
});
153148

154149
it('should not throw an exception on a second close', function () {
@@ -163,7 +158,7 @@ describe('$modal', function () {
163158

164159
expect($document).toHaveModalsOpen(0);
165160

166-
close(modal, 'closing in test');
161+
close(modal, 'closing in test', true);
167162
});
168163

169164
it('should open a modal from templateUrl', function () {
@@ -179,7 +174,6 @@ describe('$modal', function () {
179174

180175
expect($document).toHaveModalsOpen(0);
181176

182-
waitForBackdropAnimation();
183177
expect($document).not.toHaveBackdrop();
184178
});
185179

@@ -487,7 +481,6 @@ describe('$modal', function () {
487481
expect(backdropEl).toHaveClass('in');
488482

489483
dismiss(modal);
490-
waitForBackdropAnimation();
491484

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

0 commit comments

Comments
 (0)