Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hyphyphyph
Copy link

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
Copy link
Member

tj commented Jun 4, 2012

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

@hyphyphyph
Copy link
Author

Is this currently supported ?

@tj
Copy link
Member

tj commented Jun 4, 2012

not currently in the API nope

@hyphyphyph
Copy link
Author

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
Copy link
Member

tj commented Jun 4, 2012

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
Copy link
Author

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

@tj
Copy link
Member

tj commented Jun 4, 2012

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
Copy link
Author

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
Copy link
Author

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

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

@hyphyphyph
Copy link
Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants