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

Commit

Permalink
fix(dialog): close dialog on location change
Browse files Browse the repository at this point in the history
Closes #335
  • Loading branch information
Monish Parajuli authored and pkozlowski-opensource committed Apr 24, 2013
1 parent 74beecd commit 474ce52
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/dialog/dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ dialogModule.provider("$dialog", function(){
e.preventDefault();
self.$scope.$apply();
};

this.handleLocationChange = function() {
self.close();
self.$scope.$apply();

This comment has been minimized.

Copy link
@lukasmlady

lukasmlady Apr 25, 2013

$digest is already in progress since we operate inside angular, so $apply isn't necessary.

This comment has been minimized.

Copy link
@pkozlowski-opensource

pkozlowski-opensource Apr 25, 2013

Member

@lukasmlady aha! good catch, don't know how I've missed this one! The worst thing is that tests didn't explode!!! Let me fix this straight away!

This comment has been minimized.

Copy link
@pkozlowski-opensource

pkozlowski-opensource Apr 25, 2013

Member

@lukasmlady it was fixed in 77e6acb, thnx once again for spotting this one!

This comment has been minimized.

Copy link
@lukasmlady

lukasmlady Apr 25, 2013

You are welcome!

};
}

// The `isOpen()` method returns wether the dialog is currently visible.
Expand Down Expand Up @@ -184,6 +189,8 @@ dialogModule.provider("$dialog", function(){
Dialog.prototype._bindEvents = function() {
if(this.options.keyboard){ body.bind('keydown', this.handledEscapeKey); }
if(this.options.backdrop && this.options.backdropClick){ this.backdropEl.bind('click', this.handleBackDropClick); }

this.$scope.$on('$locationChangeSuccess', this.handleLocationChange);
};

Dialog.prototype._unbindEvents = function() {
Expand Down
15 changes: 15 additions & 0 deletions src/dialog/test/dialog.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ describe('Given ui.bootstrap.dialog', function(){
var clearGlobalOptions = function(){
provider.options({});
};

var changeLocation = function() {
$rootScope.$broadcast('$locationChangeSuccess');
$rootScope.$apply();
};

var dialogShouldBeClosed = function(){
it('should not include a backdrop in the DOM', function(){
Expand Down Expand Up @@ -287,6 +292,16 @@ describe('Given ui.bootstrap.dialog', function(){
expect($document.find('body > div.modal > div.modal-header').length).toBe(1);
});
});

describe('When dialog is open and location changes', function(){
beforeEach(function(){
createDialog({template:template});
openDialog();
changeLocation();
});

dialogShouldBeClosed();
});

describe('when opening it with a template containing white-space', function(){

Expand Down
6 changes: 6 additions & 0 deletions src/modal/test/modal.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,5 +169,11 @@ describe('Give ui.boostrap.modal', function() {
$scope.$digest();
expect($scope.modalShown).not.toBeTruthy();
});

it('should update the model if the location change is successful', function() {
$rootScope.$broadcast('$locationChangeSuccess');
$scope.$digest();
expect($scope.modalShown).not.toBeTruthy();
});
});
});

0 comments on commit 474ce52

Please sign in to comment.