Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

fix(modal): skipping ESC handling for form inputs; fixes #2544 #3551

Closed
wants to merge 4 commits into from
Closed

Conversation

stryju
Copy link
Contributor

@stryju stryju commented Apr 16, 2015

also added scripts section in package.json to speed up figuring out which task is for testing ;-)

@chrisirhc
Copy link
Contributor

This breaks expected behavior as I mentioned in #2544 . Discussing there.

@wesleycho
Copy link
Contributor

Hey @stryju, can you make the requested change(s) in #2544? Happy to merge it then.

@stryju
Copy link
Contributor Author

stryju commented Aug 4, 2015

done

@@ -251,6 +251,10 @@ angular.module('ui.bootstrap.modal', [])
}

$document.bind('keydown', function (evt) {
if (evt.isPropagationStopped() || /^(?:input|textarea|select)$/i.test(evt.target.nodeName)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, true!
also, forgot to remove the form input check

on it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

went with evt.isDefaultPrevented()

@wesleycho
Copy link
Contributor

This LGTM, based on the comments in the issue.

@wesleycho wesleycho closed this in a05b9c1 Aug 6, 2015
@wesleycho
Copy link
Contributor

I had to do a few minor tweaks since there have been updates since this PR, but got the tests passing & did some minor cleanup.

Thanks!

@stryju
Copy link
Contributor Author

stryju commented Aug 6, 2015

🍻

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants