Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

Commit

Permalink
fix(dialog): make sure dialog only destroys itself.
Browse files Browse the repository at this point in the history
Dialog destruction is async and deferred. As such, there's a risk of another dialog having been created by the time the current dialog's "destroy()" is called.

*  Added tests for testing dialog double opening.
*  Fix a bug which prevents a dialog from being closed if it's opened while another dialog is shown.

Closes #5157.
  • Loading branch information
erikogenvik authored and ThomasBurleson committed Oct 29, 2015
1 parent 4205be7 commit e8cfce2
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/components/dialog/dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function MdDialogDirective($$rAF, $mdTheming, $mdDialog) {
}

scope.$on('$destroy', function() {
$mdDialog.destroy();
$mdDialog.destroy(element);
});

/**
Expand Down
60 changes: 59 additions & 1 deletion src/components/dialog/dialog.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,65 @@ describe('$mdDialog', function() {
expect(parent[0].querySelectorAll('md-dialog.two').length).toBe(1);
}));

it('should hide dialog', inject(function($mdDialog, $rootScope, $animate) {
var parent = angular.element('<div>');
$mdDialog.show({
template: '<md-dialog class="one">',
parent: parent
});
runAnimation();

$mdDialog.hide();
runAnimation();

expect(parent[0].querySelectorAll('md-dialog.one').length).toBe(0);
}));

it('should allow opening new dialog after existing without corruption', inject(function($mdDialog, $rootScope, $animate) {
var parent = angular.element('<div>');
$mdDialog.show({
template: '<md-dialog class="one">',
parent: parent
});
runAnimation();
$mdDialog.hide();
runAnimation();

$mdDialog.show({
template: '<md-dialog class="two">',
parent: parent
});
runAnimation();
$mdDialog.hide();
runAnimation();

expect(parent[0].querySelectorAll('md-dialog.one').length).toBe(0);
expect(parent[0].querySelectorAll('md-dialog.two').length).toBe(0);
}));

it('should allow opening new dialog from existing without corruption', inject(function($mdDialog, $rootScope, $animate) {
var parent = angular.element('<div>');
$mdDialog.show({
template: '<md-dialog class="one">',
parent: parent
});
runAnimation();

$mdDialog.show({
template: '<md-dialog class="two">',
parent: parent
});
//First run is for the old dialog being hidden.
runAnimation();
//Second run is for the new dialog being shown.
runAnimation();
$mdDialog.hide();
runAnimation();

expect(parent[0].querySelectorAll('md-dialog.one').length).toBe(0);
expect(parent[0].querySelectorAll('md-dialog.two').length).toBe(0);
}));

it('should have the dialog role', inject(function($mdDialog, $rootScope) {
var template = '<md-dialog>Hello</md-dialog>';
var parent = angular.element('<div>');
Expand Down Expand Up @@ -1141,4 +1200,3 @@ describe('$mdDialog with custom interpolation symbols', function() {
expect(buttons.eq(1).text()).toBe('OK');
}));
});

29 changes: 23 additions & 6 deletions src/core/services/interimElement/interimElement.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,14 +370,29 @@ function InterimElementProvider() {

/*
* Special method to quick-remove the interim element without animations
* Note: interim elements are in "interim containers"
*/
function destroy() {
var interim = stack.shift();
function destroy(target) {
var interim = !target ? stack.shift() : null;
var cntr = angular.element(target).length ? angular.element(target)[0].parentNode : null;

if (cntr) {
// Try to find the interim element in the stack which corresponds to the supplied DOM element.
var filtered = stack.filter(function(entry) {
var currNode = entry.options.element[0];
return (currNode === cntr);
});

return interim ? interim.remove(SHOW_CANCELLED, false, {'$destroy':true}) :
$q.when(SHOW_CANCELLED);
}
// Note: this function might be called when the element already has been removed, in which
// case we won't find any matches. That's ok.
if (filtered.length > 0) {
interim = filtered[0];
stack.splice(stack.indexOf(interim), 1);
}
}

return interim ? interim.remove(SHOW_CANCELLED, false, {'$destroy':true}) : $q.when(SHOW_CANCELLED);
}

/*
* Internal Interim Element Object
Expand Down Expand Up @@ -438,7 +453,9 @@ function InterimElementProvider() {

if ( options.$destroy === true ) {

return hideElement(options.element, options);
return hideElement(options.element, options).then(function(){
(isCancelled && rejectAll(response)) || resolveAll(response);
});

} else {

Expand Down

0 comments on commit e8cfce2

Please sign in to comment.