add support for keyboard option

[falexandrou]

No description provided.

[makeusabrew]

Hi @falexandrou, @tarlepp,

Apologies for looking at this after such a long delay. Just diffing master from the last release and noticed it.

Can either of you remember / explain what this actually adds to the library? Bootbox already explicitly handles the escape key itself; this is necessary so it can trigger the relevant callback (if supplied).

Using the Bootstrap keyboard option, which is described simply as "Closes the modal when escape key is pressed" on their own website will not trigger the necessary Bootbox logic. And since it only handles the escape key, I can't see what it adds or why it was necessary in the first place.

Any ideas? I'm afraid I'll have to revert it if not. Sorry.

[falexandrou]

Hi @makeusabrew

I was not sure how the escape key was handled and saw the explicit keyboard: false option, so i thought it was a bug. The thing is that bootstrap already handles keyboard actions and there are events for when the modal is being closed or is closed. Couldn't we trigger Bootbox logic that way then?

[makeusabrew]

I'm unsure how you came to the conclusion it was a bug; the escape button
has been handled properly since Bootbox was first released years ago.

From memory, no, we can't just hook into Bootstrap's events because we
receive them too late - the dialog has already closed (or is in the process
of closing) which doesn't let us trap the escape event and handle it
ourselves (we might not even want to hide the dialog depending on our
callback).

I'll revert this commit later today I'm afraid.

Thanks,

Nick

On Thu, Jan 8, 2015 at 9:08 AM, Fotis Alexandrou notifications@github.com
wrote:

Hi @makeusabrew https://github.com/makeusabrew

I was not sure how the escape key was handled and saw the explicit keyboard:
false option, so i thought it was a bug. The thing is that bootstrap
already handles keyboard actions and there are events for when the modal is
being closed or is closed. Couldn't we trigger Bootbox logic that way then?

—
Reply to this email directly or view it on GitHub
#319 (comment).

[falexandrou]

@makeusabrew It didn't work for me, so i thought it was a bug. I'll try to reproduce the issue but I don't have the time right now.

You can prevent the dialog from closing if you use e.preventDefault() in the hide event callback. I'm sorry I just like sticking to the defaults, or whatever provided by the original component, so I thought this would be a good fit. IMHO: Sticking to what bootstrap provides, is better than adding custom callbacks

If you think you should revert it, please go ahead.

[makeusabrew]

The functionality is there and has been there for the past three years - it
works, it lets us handle the behaviour explicitly, it's tested, and I
absolutely 100% promise you I wouldn't have added it unless it was
necessary. The code to handle the escape key amounts to only four lines
(715-719). I'll only accept a PR which implements it the 'proper' way if it
amounts to less lines, exhibits the same behaviour, and is 100% tested.

If the escape key functionality doesn't work for you, that's a bug and a
different issue :). Please can you let me know if the very first 'Run
example' button doesn't let you close the dialog with the escape key on the
demo page please? http://bootboxjs.com/ - if not, please raise a separate
bug.

Thanks,

Nick

On Thu, Jan 8, 2015 at 10:39 AM, Fotis Alexandrou notifications@github.com
wrote:

@makeusabrew https://github.com/makeusabrew It didn't work for me, so i
thought it was a bug. I'll try to reproduce the issue but I don't have the
time right now.

You can prevent the dialog from closing if you use e.preventDefault() in
the hide event callback. I'm sorry I just like sticking to the defaults,
or whatever provided by the original component, so I thought this would be
a good fit. IMHO: Sticking to what bootstrap provides, is better than
adding custom callbacks

If you think you should revert it, please go ahead.

—
Reply to this email directly or view it on GitHub
#319 (comment).

[falexandrou]

@makeusabrew I'll try to reproduce the issue i had in the first place then

[jtakalai]

Hi, in your examples, I could close the modal by pressing ESC only in the first examples (alert, confirm, prompt, prompt with default value) but in the other examples (custom*) ESC key would do nothing.

Browser is Version 47.0.2526.111 on Mac OS X 10.11.2

Would be nice to be able to add handler functions for keys in one of the following ways:

• bootbox.dialog( ..., keys: { 13: "ok", 27: "cancel" }, buttons: { ok: ..., cancel: ...})
• bootbox.dialog( ..., keys: { 13: function() { ...} })
• bootbox.dialog( ..., buttons: { ok: { label: "OK", key: 13 } })