Skip to content

Commit 9baf219

Browse files
ajoslinajoslin
committed
fix(popup): synchronously add/remove popups from stack, no matter the animation state
Closes #3131
1 parent 71e8971 commit 9baf219

File tree

2 files changed

+101
-113
lines changed

2 files changed

+101
-113
lines changed

Diff for: ‎js/angular/service/popup.js

+94-106
Original file line numberDiff line numberDiff line change
@@ -281,84 +281,79 @@ function($ionicTemplateLoader, $ionicBackdrop, $q, $timeout, $rootScope, $ionicB
281281
buttons: []
282282
}, options || {});
283283

284-
var popupPromise = $ionicTemplateLoader.compile({
285-
template: POPUP_TPL,
286-
scope: options.scope && options.scope.$new(),
287-
appendTo: $ionicBody.get()
284+
var self = {};
285+
self.scope = (options.scope || $rootScope).$new();
286+
self.element = jqLite(POPUP_TPL);
287+
self.responseDeferred = $q.defer();
288+
289+
$ionicBody.get().appendChild(self.element[0]);
290+
$compile(self.element)(self.scope);
291+
292+
extend(self.scope, {
293+
title: options.title,
294+
buttons: options.buttons,
295+
subTitle: options.subTitle,
296+
cssClass: options.cssClass,
297+
$buttonTapped: function(button, event) {
298+
var result = (button.onTap || noop)(event);
299+
event = event.originalEvent || event; //jquery events
300+
301+
if (!event.defaultPrevented) {
302+
self.responseDeferred.resolve(result);
303+
}
304+
}
288305
});
289-
var contentPromise = options.templateUrl ?
290-
$ionicTemplateLoader.load(options.templateUrl) :
291-
$q.when(options.template || options.content || '');
292306

293-
return $q.all([popupPromise, contentPromise]).then(function(results) {
294-
var self = results[0];
295-
var content = results[1];
296-
var responseDeferred = $q.defer();
297-
298-
self.responseDeferred = responseDeferred;
299-
300-
//Can't ng-bind-html for popup-body because it can be insecure html
301-
//(eg an input in case of prompt)
302-
var body = jqLite(self.element[0].querySelector('.popup-body'));
303-
if (content) {
304-
body.html(content);
305-
$compile(body.contents())(self.scope);
307+
$q.when(
308+
options.templateUrl ?
309+
$ionicTemplateLoader.load(options.templateUrl) :
310+
(options.template || options.content || '')
311+
).then(function(template) {
312+
var popupBody = jqLite(self.element[0].querySelector('.popup-body'));
313+
if (template) {
314+
popupBody.html(template);
315+
$compile(popupBody.contents())(self.scope);
306316
} else {
307-
body.remove();
317+
popupBody.remove();
308318
}
319+
});
309320

310-
extend(self.scope, {
311-
title: options.title,
312-
buttons: options.buttons,
313-
subTitle: options.subTitle,
314-
cssClass: options.cssClass,
315-
$buttonTapped: function(button, event) {
316-
var result = (button.onTap || noop)(event);
317-
event = event.originalEvent || event; //jquery events
318-
319-
if (!event.defaultPrevented) {
320-
responseDeferred.resolve(result);
321-
}
322-
}
323-
});
324-
325-
self.show = function() {
326-
if (self.isShown || self.removed) return;
321+
self.show = function() {
322+
if (self.isShown || self.removed) return;
327323

328-
self.isShown = true;
329-
ionic.requestAnimationFrame(function() {
330-
//if hidden while waiting for raf, don't show
331-
if (!self.isShown) return;
324+
self.isShown = true;
325+
ionic.requestAnimationFrame(function() {
326+
//if hidden while waiting for raf, don't show
327+
if (!self.isShown) return;
332328

333-
self.element.removeClass('popup-hidden');
334-
self.element.addClass('popup-showing active');
335-
focusInput(self.element);
336-
});
337-
};
329+
self.element.removeClass('popup-hidden');
330+
self.element.addClass('popup-showing active');
331+
focusInput(self.element);
332+
});
333+
};
338334

339-
self.hide = function(callback) {
340-
callback = callback || noop;
341-
if (!self.isShown) return callback();
335+
self.hide = function(callback) {
336+
callback = callback || noop;
337+
if (!self.isShown) return callback();
342338

343-
self.isShown = false;
344-
self.element.removeClass('active');
345-
self.element.addClass('popup-hidden');
346-
$timeout(callback, 250);
347-
};
339+
self.isShown = false;
340+
self.element.removeClass('active');
341+
self.element.addClass('popup-hidden');
342+
$timeout(callback, 250, false);
343+
};
348344

349-
self.remove = function() {
350-
if (self.removed) return;
345+
self.remove = function() {
346+
if (self.removed) return;
351347

352-
self.hide(function() {
353-
self.element.remove();
354-
self.scope.$destroy();
355-
});
348+
self.hide(function() {
349+
self.element.remove();
350+
self.scope.$destroy();
351+
});
356352

357-
self.removed = true;
358-
};
353+
self.removed = true;
354+
};
359355

360-
return self;
361-
});
356+
return self;
362357
}
363358

364359
function onHardwareBackButton() {
@@ -367,69 +362,62 @@ function($ionicTemplateLoader, $ionicBackdrop, $q, $timeout, $rootScope, $ionicB
367362
}
368363

369364
function showPopup(options) {
370-
var resultDeferred;
371-
var popupPromise = $ionicPopup._createPopup(options);
365+
var popup = $ionicPopup._createPopup(options);
366+
var showDelay = 0;
372367

373368
if (popupStack.length > 0) {
374369
popupStack[popupStack.length - 1].hide();
375-
resultDeferred = $timeout(doShowPopup, config.stackPushDelay);
370+
showDelay = config.stackPushDelay;
376371
} else {
377372
//Add popup-open & backdrop if this is first popup
378373
$ionicBody.addClass('popup-open');
379-
console.log("RETAIN");
380374
$ionicBackdrop.retain();
381375
//only show the backdrop on the first popup
382376
$ionicPopup._backButtonActionDone = $ionicPlatform.registerBackButtonAction(
383377
onHardwareBackButton,
384378
IONIC_BACK_PRIORITY.popup
385379
);
386-
resultDeferred = doShowPopup();
387380
}
388381

389-
resultDeferred.close = function popupClose(result) {
390-
popupPromise.then(function(popup) {
391-
if (!popup.removed) popup.responseDeferred.resolve(result);
392-
});
382+
// Expose a 'close' method on the returned promise
383+
popup.responseDeferred.promise.close = function popupClose(result) {
384+
if (!popup.removed) popup.responseDeferred.resolve(result);
393385
};
386+
//DEPRECATED: notify the promise with an object with a close method
387+
popup.responseDeferred.notify({ close: popup.responseDeferred.close });
394388

395-
return resultDeferred;
389+
doShow();
396390

397-
function doShowPopup() {
398-
return popupPromise.then(function(popup) {
399-
popupStack.push(popup);
400-
popup.show();
391+
return popup.responseDeferred.promise;
401392

402-
//DEPRECATED: notify the promise with an object with a close method
403-
popup.responseDeferred.notify({
404-
close: resultDeferred.close
405-
});
393+
function doShow() {
394+
popupStack.push(popup);
395+
$timeout(popup.show, showDelay, false);
406396

407-
return popup.responseDeferred.promise.then(function(result) {
408-
var index = popupStack.indexOf(popup);
409-
if (index !== -1) {
410-
popupStack.splice(index, 1);
411-
}
412-
popup.remove();
413-
$ionicBackdrop.release();
414-
415-
if (popupStack.length > 0) {
416-
popupStack[popupStack.length - 1].show();
417-
} else {
418-
//Remove popup-open & backdrop if this is last popup
419-
$timeout(function() {
420-
// wait to remove this due to a 300ms delay native
421-
// click which would trigging whatever was underneath this
422-
if (!popupStack.length) {
423-
$ionicBody.removeClass('popup-open');
424-
}
425-
}, 400, false);
397+
popup.responseDeferred.promise.then(function(result) {
398+
var index = popupStack.indexOf(popup);
399+
if (index !== -1) {
400+
popupStack.splice(index, 1);
401+
}
426402

427-
($ionicPopup._backButtonActionDone || noop)();
428-
}
403+
if (popupStack.length > 0) {
404+
popupStack[popupStack.length - 1].show();
405+
} else {
406+
$ionicBackdrop.release();
407+
//Remove popup-open & backdrop if this is last popup
408+
$timeout(function() {
409+
// wait to remove this due to a 300ms delay native
410+
// click which would trigging whatever was underneath this
411+
if (!popupStack.length) {
412+
$ionicBody.removeClass('popup-open');
413+
}
414+
}, 400, false);
415+
($ionicPopup._backButtonActionDone || noop)();
416+
}
429417

430-
return result;
431-
});
418+
popup.remove();
432419

420+
return result;
433421
});
434422

435423
}

Diff for: ‎test/unit/angular/service/popup.unit.js

+7-7
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ describe('$ionicPopup service', function() {
204204
show: jasmine.createSpy('show'),
205205
responseDeferred: $q.defer()
206206
};
207-
spyOn($ionicPopup, '_createPopup').andReturn($q.when(fakePopup));
207+
spyOn($ionicPopup, '_createPopup').andReturn(fakePopup);
208208
expect($ionicPopup._popupStack.length).toBe(0);
209209
$ionicPopup.show();
210210
expect(fakePopup.show).not.toHaveBeenCalled();
@@ -215,9 +215,9 @@ describe('$ionicPopup service', function() {
215215
}));
216216

217217
it('should have close function which resolves promise with argument', inject(function($ionicPopup, $q, $rootScope) {
218-
var popup = TestUtil.unwrapPromise($ionicPopup._createPopup());
219-
spyOn($ionicPopup, '_createPopup').andReturn($q.when(popup));
220-
var result = $ionicPopup.show();
218+
var popup = TestUtil.unwrapPromise($ionicPopup._createPopup({template: 'foo'}));
219+
spyOn($ionicPopup, '_createPopup').andReturn(popup);
220+
var result = $ionicPopup.show();
221221
spyOn(popup.responseDeferred, 'resolve');
222222
result.close('foobar');
223223
$rootScope.$apply();
@@ -230,7 +230,7 @@ describe('$ionicPopup service', function() {
230230
remove: jasmine.createSpy('remove'),
231231
responseDeferred: $q.defer()
232232
};
233-
spyOn($ionicPopup, '_createPopup').andReturn($q.when(fakePopup));
233+
spyOn($ionicPopup, '_createPopup').andReturn(fakePopup);
234234
var result = $ionicPopup.show();
235235
$timeout.flush();
236236
expect(fakePopup.remove).not.toHaveBeenCalled();
@@ -251,7 +251,7 @@ describe('$ionicPopup service', function() {
251251
show: jasmine.createSpy('show'),
252252
hide: jasmine.createSpy('hide')
253253
};
254-
spyOn($ionicPopup, '_createPopup').andReturn($q.when(fakePopup));
254+
spyOn($ionicPopup, '_createPopup').andReturn(fakePopup);
255255
$ionicPopup._popupStack.unshift(previousPopup);
256256
$ionicPopup.show();
257257
fakePopup.responseDeferred.resolve();
@@ -268,7 +268,7 @@ describe('$ionicPopup service', function() {
268268
var backDoneSpy = jasmine.createSpy('backDone');
269269
spyOn($ionicPlatform, 'registerBackButtonAction').andReturn(backDoneSpy);
270270
spyOn($ionicBackdrop, 'release');
271-
spyOn($ionicPopup, '_createPopup').andReturn($q.when(fakePopup));
271+
spyOn($ionicPopup, '_createPopup').andReturn(fakePopup);
272272
$ionicPopup.show();
273273
fakePopup.responseDeferred.resolve();
274274
$timeout.flush();

0 commit comments

Comments
 (0)