-
Notifications
You must be signed in to change notification settings - Fork 6.7k
fix(dropdown): stop esc keydown event #5787
Conversation
Fixes angular-ui#5778 Closes angular-ui#5787 BREAKING CHANGE: Stops propagation of keydown event when escape key is pressed. Removes keydown event from the document and moves it to the dropdown element.
@wesleycho, not sure if switching the keydown event from document to element will have any ramifications I didn't think about. Escape and keyboard nav still seems to work fine for me in the linked plunker. The reason I did this was because the document event was catching after the modal escape key event had already fired so stopPropagation didn't do anything. Couldn't think of a nicer way to solve this than to move the event over to the element. |
@@ -26,8 +26,10 @@ describe('uib-dropdown', function() { | |||
|
|||
var triggerKeyDown = function (element, keyCode) { | |||
var e = $.Event('keydown'); | |||
spyOn(e, "stopPropagation"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use single quotes - also add .and.callThrough()
I think that switch is ok, judging by the code and playing around with the Plunker. I'm ok with this change. |
Fixes angular-ui#5778 Closes angular-ui#5787 BREAKING CHANGE: Stops propagation of keydown event when escape key is pressed. Removes keydown event from the document and moves it to the dropdown element.
@wesleycho, sounds good, thanks. I made the requested changes. |
LGTM if it passes the tests |
Fixes #5778
BREAKING CHANGE: Stops propagation of keydown event when escape key is pressed. Removes keydown event from the document and moves it to the dropdown element.
Plunker with fix