Skip to content

Commit

Permalink
Merge pull request #54 from pgherveou/patch-3
Browse files Browse the repository at this point in the history 
handle confirmation closed through overlay click + unbind keydown
  • Loading branch information
@tj
tj committed Jul 11, 2012
2 parents f9903c8 + 3d882ef commit d75189a
Showing 1 changed file with 2 additions and 0 deletions.
2 changes: 2 additions & 0 deletions lib/components/dialog/dialog.js
View file
Expand Up @@ -148,6 +148,7 @@ Dialog.prototype.overlay = function(){
.overlay({ closable: true })
.on('hide', function(){
self.closedOverlay = true;
self.emit('close');
self.hide();
});
return this;
Expand Down Expand Up @@ -248,5 +249,6 @@ Dialog.prototype.hide = function(ms){
Dialog.prototype.remove = function(){
this.emit('hide');
this.el.remove();
$(document).unbind('keydown.dialog');
return this;
};

2 comments on commit d75189a

@pgherveou
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realised that there are scenario where the callback will be called twice
Sorry for the noise this will need one more pull request

if you click on the close button or the cancel button, callback is fired twice
so overlay should emit and overlayclose instead of close and we should catch this event as well to call the callback
This solution is ok for you ?

@tj
Copy link
Member Author

@tj tj commented on d75189a Jul 11, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overlay only has "show" and "hide" which is fine for that component, but in here yeah we can special-case if we need to

Please sign in to comment.