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

Commit

Permalink
fix(dialog, interimElement): element removals occur after dialog clos…
Browse files Browse the repository at this point in the history 
…e animation finishes.

only alerts `hide()` on escape or clickOutside; confirm and custom `cancel()`
improve use of `.finally()` in promise chaining
remove backdrop and dialog-container AFTER dialog close animation finishes
stacked interimElements now cancel

#3782
  • Loading branch information
@ThomasBurleson
ThomasBurleson committed Jul 17, 2015
1 parent dbafaa1 commit 7bbfd1f
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 22 deletions.
19 changes: 13 additions & 6 deletions src/components/dialog/dialog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,15 +489,15 @@ function MdDialogProvider($$interimElementProvider) {
* Remove function for all dialogs
*/
function onRemove(scope, element, options) {
angular.element($document[0].body).removeClass('md-dialog-is-showing');

options.deactivateListeners();
options.unlockScreenReader();
options.hideBackdrop();

return dialogPopOut(element, options)
.then(function () {
.finally(function () {
angular.element($document[0].body).removeClass('md-dialog-is-showing');
options.hideBackdrop();
element.remove();

options.origin.focus();
});
}
Expand Down Expand Up @@ -531,6 +531,13 @@ function MdDialogProvider($$interimElementProvider) {
*/
function activateListeners(element, options) {
var removeListeners = [ ];
var smartClose = function() {
// Only 'confirm' dialogs have a cancel button... escape/clickOutside will
// cancel or fallback to hide.
var closeFn = ( options.$type == 'alert' ) ? $mdDialog.hide : $mdDialog.cancel;

$mdUtil.nextTick( closeFn, true );
};

if (options.escapeToClose) {
var target = options.parent;
Expand All @@ -539,7 +546,7 @@ function MdDialogProvider($$interimElementProvider) {
ev.stopPropagation();
ev.preventDefault();

$mdUtil.nextTick($mdDialog.cancel);
smartClose();
}
};

Expand All @@ -561,7 +568,7 @@ function MdDialogProvider($$interimElementProvider) {
ev.stopPropagation();
ev.preventDefault();

$mdUtil.nextTick($mdDialog.cancel);
smartClose();
}
};

Expand Down
115 changes: 113 additions & 2 deletions src/components/dialog/dialog.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,40 @@ describe('$mdDialog', function() {

expect($document.activeElement).toBe(parent[0].querySelector('md-dialog-content'));
}));

it('should remove `md-dialog-container` on click and remove', inject(function($mdDialog, $rootScope, $timeout) {
jasmine.mockElementFocus(this);
var container, parent = angular.element('<div>');

$mdDialog.show(
$mdDialog.alert({
template:
'<md-dialog>' +
'<md-dialog-content tabIndex="0">' +
'<p>Muppets are the best</p>' +
'</md-dialog-content>' +
'</md-dialog>',
parent: parent,
clickOutsideToClose: true
})
);

$rootScope.$apply();
triggerTransitionEnd( parent.find('md-dialog') );

container = angular.element(parent[0].querySelector('.md-dialog-container'));
container.triggerHandler({
type: 'click',
target: container[0]
});

$timeout.flush();
triggerTransitionEnd( parent.find('md-dialog') );

container = angular.element(parent[0].querySelector('.md-dialog-container'));
expect(container.length).toBe(0);
}));

});

describe('#confirm()', function() {
Expand Down Expand Up @@ -167,14 +201,87 @@ describe('$mdDialog', function() {
'<button class="dialog-close">Close</button>' +
'</div>' +
'</md-dialog>',
parent: parent
parent: parent,
});

$rootScope.$apply();
triggerTransitionEnd( parent.find('md-dialog') );

expect($document.activeElement).toBe(parent[0].querySelector('.dialog-close'));
}));

it('should remove `md-dialog-container` after click outside', inject(function($mdDialog, $rootScope, $timeout) {
jasmine.mockElementFocus(this);
var container, parent = angular.element('<div>');

$mdDialog.show(
$mdDialog.confirm({
template:
'<md-dialog>' +
'<md-dialog-content tabIndex="0">' +
'<p>Muppets are the best</p>' +
'</md-dialog-content>' +
'</md-dialog>',
parent: parent,
clickOutsideToClose: true,
ok : 'OK',
cancel : 'CANCEL'
})
);

$rootScope.$apply();
triggerTransitionEnd( parent.find('md-dialog') );

container = angular.element(parent[0].querySelector('.md-dialog-container'));
container.triggerHandler({
type: 'click',
target: container[0]
});

$timeout.flush();
triggerTransitionEnd( parent.find('md-dialog') );

container = angular.element(parent[0].querySelector('.md-dialog-container'));
expect(container.length).toBe(0);
}));

it('should remove `md-dialog-container` after ESCAPE key', inject(function($mdDialog, $rootScope, $timeout, $mdConstant) {
jasmine.mockElementFocus(this);
var container, parent = angular.element('<div>');
var response;

$mdDialog.show(
$mdDialog.confirm({
template:
'<md-dialog>' +
'<md-dialog-content tabIndex="0">' +
'<p>Muppets are the best</p>' +
'</md-dialog-content>' +
'</md-dialog>',
parent: parent,
clickOutsideToClose: true,
escapeToClose: true,
ok : 'OK',
cancel : 'CANCEL'
})
).catch(function(reason){
response = reason;
});

$rootScope.$apply();
triggerTransitionEnd( parent.find('md-dialog') );

parent.triggerHandler({type: 'keyup',
keyCode: $mdConstant.KEY_CODE.ESCAPE
});
$timeout.flush();
triggerTransitionEnd( parent.find('md-dialog') );

container = angular.element(parent[0].querySelector('.md-dialog-container'));

expect(container.length).toBe(0);
expect(response).toBe(false);
}));
});

describe('#build()', function() {
Expand Down Expand Up @@ -331,7 +438,11 @@ describe('$mdDialog', function() {

var container = angular.element(parent[0].querySelector('.md-dialog-container'));

container.triggerHandler('click');
container.triggerHandler({
type: 'click',
target: container[0]
});

$timeout.flush();
$animate.triggerCallbacks();

Expand Down
5 changes: 3 additions & 2 deletions src/core/services/interimElement/interimElement.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ function InterimElementProvider() {
*/
function show(options) {
if (stack.length) {
return service.cancel().then(function() {
return service.cancel().finally(function() {
return show(options);
});
} else {
Expand Down Expand Up @@ -314,6 +314,7 @@ function InterimElementProvider() {
.remove()
.then(function() {
interimElement.deferred.reject(reason);
return interimElement.deferred.promise;
});
}

Expand Down Expand Up @@ -417,7 +418,7 @@ function InterimElementProvider() {
// Trigger onRemoving callback *before* the remove operation starts
(options.onRemoving || angular.noop)(options.scope, element);

return $q.when(ret).then(function() {
return $q.when(ret).finally(function() {
if (!options.preserveScope) options.scope.$destroy();
removeDone = true;
});
Expand Down
23 changes: 12 additions & 11 deletions src/core/services/interimElement/interimElement.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ describe('$$interimElement service', function() {
it('should not call onShow or onRemove on failing to load templates', function() {
createInterimProvider('interimTest');
inject(function($q, $rootScope, $rootElement, interimTest, $httpBackend, $animate) {
$compilerSpy.and.callFake(function() {
var deferred = $q.defer();
deferred.reject();
return deferred.promise;
$compilerSpy.and.callFake(function(reason) {
return $q(function(resolve,reject){
reject(reason || false);
});
});
$httpBackend.when('GET', '/fail-url.html').respond(500, '');
var onShowCalled = false, onHideCalled = false;
Expand All @@ -59,8 +59,9 @@ describe('$$interimElement service', function() {
onRemove: function() { onHideCalled = true; }
});
$animate.triggerCallbacks();
interimTest.hide();
interimTest.cancel();
$animate.triggerCallbacks();

expect(onShowCalled).toBe(false);
expect(onHideCalled).toBe(false);
});
Expand Down Expand Up @@ -501,13 +502,13 @@ describe('$$interimElement service', function() {

$compilerSpy.and.callFake(function(opts) {
var el = $compile(opts.template);
var deferred = $q.defer();
deferred.resolve({
link: el,
locals: {}
return $q(function(resolve){
resolve({
link: el,
locals: {}
});
!$rootScope.$$phase && $rootScope.$apply();
});
!$rootScope.$$phase && $rootScope.$apply();
return deferred.promise;
});
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/util/animate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function AnimateDomUtils($mdUtil, $$rAF, $q, $timeout, $mdConstant) {
* Announce completion or failure via promise handlers
*/
waitTransitionEnd: function (element, opts) {
var TIMEOUT = 10000; // fallback is 10 secs
var TIMEOUT = 3000; // fallback is 3 secs

return $q(function(resolve, reject){
opts = opts || { };
Expand Down

0 comments on commit 7bbfd1f

Please sign in to comment.