uikit icon indicating copy to clipboard operation
uikit copied to clipboard

Return false from ui.Confirmation callback cancels dialog close

Open hyphyphyph opened this issue 12 years ago • 10 comments

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.

hyphyphyph avatar Jun 04 '12 20:06 hyphyphyph

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

tj avatar Jun 04 '12 20:06 tj

Is this currently supported ?

hyphyphyph avatar Jun 04 '12 20:06 hyphyphyph

not currently in the API nope

tj avatar Jun 04 '12 20:06 tj

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.

hyphyphyph avatar Jun 04 '12 20:06 hyphyphyph

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

tj avatar Jun 04 '12 20:06 tj

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

hyphyphyph avatar Jun 04 '12 20:06 hyphyphyph

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

tj avatar Jun 04 '12 20:06 tj

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 avatar Jun 04 '12 20:06 hyphyphyph

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

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

hyphyphyph avatar Jun 04 '12 21:06 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.

hyphyphyph avatar Jun 05 '12 18:06 hyphyphyph