Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Return false from ui.Confirmation callback cancels dialog close #48

Open
wants to merge 4 commits into from

2 participants

@hyphyphyph

Of any relevance, I work with JPJoyal. :) We were discussing how to not close a confirmation dialog (for example, if the input of a form needs to be validated). The logical presumption was that you could return false from the callabck -- aka, have it react similarly to event callbacks. But nope.

Maybe an explicit design decision, so obviously feel free to reject this request if you no likey.

@tj
Owner
tj commented

a more flexible approach would be to allow overriding the default .ok click handler, for async validation etc

@hyphyphyph

Is this currently supported ?

@tj
Owner
tj commented

not currently in the API nope

@hyphyphyph

Great. In fact, I'll be needing support for that in a later part of my codebase, so I'll send another pull request once implemented.

@tj
Owner
tj commented

i cant think of a great API off-hand but I know a return short-circuit will be leaky, we could have both since this is tiny anyway but async would be nice

@hyphyphyph

Fair enough. Are you saying what's currently implemented here in this pull request is leaky ?

@tj
Owner
tj commented

just in the sense that it's not flexible, but it might compliment the async one fine depending on the API, if the API for the other one is nice then we wont need two

@hyphyphyph

What about a mild extension of providing a callback as it stands.

In addition to supporting

new ui.Dialog().show(function () { });

Also, optionally support:

new ui.Dialog().show({
  ok: function () {},
  cancel: function () {}
});

This would be backward compat, and fairly intuitive. In the case that either ok or cancel are not supplied, use the default.

@hyphyphyph

Actually, does it make more sense to just supply an optional 'async' second argument to show() ?

new ui.Confirmation().show(function () {}, true);
@hyphyphyph

Yyyeah, so I didn't realize this pull req stuff would update when I pushed my commit. Anyway, here's what I would propose for support more configurable behaviour of the Confirmation callback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
37 build/ui.js
@@ -117,7 +117,7 @@ var active;
exports.Dialog = Dialog;
/**
- * Return a new `Dialog` with the given
+ * Return a new `Dialog` with the given
* (optional) `title` and `msg`.
*
* @param {String} title or msg
@@ -330,9 +330,9 @@ Dialog.prototype.hide = function(ms){
// hide / remove
this.el.addClass('hide');
if (this._effect) {
- setTimeout(function(self){
+ setTimeout(function(){
self.remove();
- }, 500, this);
+ }, 500);
} else {
self.remove();
}
@@ -448,7 +448,7 @@ Overlay.prototype.hide = function(){
exports.Confirmation = Confirmation;
/**
- * Return a new `Confirmation` dialog with the given
+ * Return a new `Confirmation` dialog with the given
* `title` and `msg`.
*
* @param {String} title or msg
@@ -529,10 +529,11 @@ Confirmation.prototype.ok = function(text){
* @api public
*/
-Confirmation.prototype.show = function(fn){
+Confirmation.prototype.show = function(fn, async){
ui.Dialog.prototype.show.call(this);
this.el.find('.ok').focus();
this.callback = fn || function(){};
+ this.async = async || false;
return this;
};
@@ -562,15 +563,29 @@ Confirmation.prototype.render = function(options){
actions.find('.cancel').click(function(e){
e.preventDefault();
self.emit('cancel');
- self.callback(false);
+ if(self.async){
+ self.callback(false, function () {
+ self.hide();
+ });
+ }
+ else {
+ (self.callback(false) !== false && self.hide());
+ }
+ self.callback(false)
self.hide();
});
actions.find('.ok').click(function(e){
e.preventDefault();
self.emit('ok');
- self.callback(true);
- self.hide();
+ if(self.async){
+ self.callback(true, function(){
+ self.hide();
+ });
+ }
+ else{
+ (self.callback(true) !== false && self.hide());
+ }
});
};
@@ -1148,9 +1163,9 @@ Notification.prototype.hide = function(ms){
// hide / remove
this.el.addClass('hide');
if (this._effect) {
- setTimeout(function(self){
+ setTimeout(function(){
self.remove();
- }, 500, this);
+ }, 500);
} else {
self.remove();
}
@@ -1297,7 +1312,7 @@ exports.Menu = Menu;
*/
exports.menu = function(){
- return new Menu();
+ return new Menu;
};
/**
View
25 lib/components/confirmation/confirmation.js
@@ -6,7 +6,7 @@
exports.Confirmation = Confirmation;
/**
- * Return a new `Confirmation` dialog with the given
+ * Return a new `Confirmation` dialog with the given
* `title` and `msg`.
*
* @param {String} title or msg
@@ -87,10 +87,11 @@ Confirmation.prototype.ok = function(text){
* @api public
*/
-Confirmation.prototype.show = function(fn){
+Confirmation.prototype.show = function(fn, async){
ui.Dialog.prototype.show.call(this);
this.el.find('.ok').focus();
this.callback = fn || function(){};
+ this.async = async || false;
return this;
};
@@ -120,14 +121,26 @@ Confirmation.prototype.render = function(options){
actions.find('.cancel').click(function(e){
e.preventDefault();
self.emit('cancel');
- self.callback(false);
- self.hide();
+ if(self.async){
+ self.callback(false, function () {
+ self.hide();
+ });
+ }
+ else {
+ (self.callback(false) !== false && self.hide());
+ }
});
actions.find('.ok').click(function(e){
e.preventDefault();
self.emit('ok');
- self.callback(true);
- self.hide();
+ if(self.async){
+ self.callback(true, function(){
+ self.hide();
+ });
+ }
+ else{
+ (self.callback(true) !== false && self.hide());
+ }
});
};
View
2  lib/components/dialog/dialog.js
@@ -12,7 +12,7 @@ var active;
exports.Dialog = Dialog;
/**
- * Return a new `Dialog` with the given
+ * Return a new `Dialog` with the given
* (optional) `title` and `msg`.
*
* @param {String} title or msg
Something went wrong with that request. Please try again.