-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix(md-calendar): boundKeyHandler preventDefault on other input elements #9746
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
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.
Thank you for the PR. I actually did something similar in #9641, although it shouldn't hurt fixing it until #9641 is merged. Your tests seem to be failing, because they're still dispatching key events to the body, instead of the element. See https://github.com/angular/material/blob/master/src/components/datepicker/js/calendar.spec.js#L116
// otherwise apply on the calendar element only. | ||
|
||
if (this.$element.parent().hasClass('md-datepicker-calendar')) { | ||
handleKeyElement = angular.element(document.body); |
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.
It might be a little more readable if you defined the handleKeyEvent
in an else
block. Also you can refer to this.$element
as just $element
:
if ($element.parent().hasClass('md-datepicker-calendar')) {
handleKeyElement = angular.element(document.body);
} else {
handleKeyElement = $element;
}
@@ -113,7 +113,14 @@ describe('md-calendar', function() { | |||
function dispatchKeyEvent(keyCode, opt_modifiers) { | |||
var mod = opt_modifiers || {}; | |||
|
|||
angular.element(document.body).triggerHandler({ | |||
var dispatchElement; | |||
if (angular.element(element).parent().hasClass('md-datepicker-calendar')) { |
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.
You don't need this check. The calendar here will always be on it's own (without the datepicker).
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.
LGTM @ThomasBurleson. @NgaiKaKit could you try signing the CLA again? Otherwise the PR can't be merged.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. |
If use the md-calendar directly in the body without datepicker, boundKeyHandler will disable some keyEvent on other input elements in the page. So only apply the handleKeyEvent on the document.body when the md-calendar is inside datepicker. Otherwise, apply on the calendar element only.