[modal] ESC key capture too aggressive

[stryju]

https://github.com/angular-ui/bootstrap/blob/master/src/modal/modal.js#L224-L232

it captures that globally without checking if event.target is an user input (<input type="text|number|checkbox|...">, <textarea>, <select>)

[dvidsilva]

so this can't be prevented or overwritten?

[wesleycho]

For reference, the current lines are https://github.com/angular-ui/bootstrap/blob/master/src/modal/modal.js#L253-L265 .

[chrisirhc]

Also, for reference, Twitter Bootstrap has the escape key listener only on the modal element itself rather than on the $document.

[stryju]

there you go

[chrisirhc]

Re-looking at this, what's the actual use case where this is breaking things? Why would a user need to type escape while inputting a form?

Changing this may break expected behavior (and deviates from Twitter Bootstrap's modal behavior). Typically, if a form control requires the use of the Escape key to perform some action, it should do event.stopPropagation() to prevent other components, such as the modal from capturing the escape key.

[stryju]

well, when i'm typing something in an input and i press escape (input still :focus), i expect the value to be gone, not closing of the "window" (modal, in that case)

[stryju]

IMO it would be better if developer explicitly triggered blur on the form elements when they're "empty" so the next esc key press would close the modal

[chrisirhc]

Hm what kind of an input field is this? I don't think this is the default behavior in browsers. It's not the behavior in the input text boxes in GitHub for example. If a developer triggers blur, the developer should also stop the event from propagating or preventDefault.

[chrisirhc]

I realized we can also check whether the event has default prevented to avoid closing the modal if the isDefaultPrevented returns true. That way, developers can signal that the keyboard event has been canceled and avoid closing the modal when event.preventDefault has been called.

[stryju]

hm - true

[chrisirhc]

If you could modify your PR or create a new PR for that change, I'll be happy to get it merged 😄

[stryju]

ok, so that bugged me for a while...

1. input[type="search"] is cleared (webkit/blink)
2. input[type="date"] deactivates/collapses, when in active/expanded state
3. select deactivates/collapses, when in active/expanded state

http://jsbin.com/quyavunemo/1/