refactor(datepicker): implement ng-if on datepicker-popup #3666
Conversation
|
+1 |
|
Looks like some solid work @RobJacobs. I'll go ahead and merge this in asap. |
| @@ -161,9 +161,7 @@ angular.module('ui.bootstrap.datepicker', ['ui.bootstrap.dateparser', 'ui.bootst | |||
| $scope.keys = { 13:'enter', 32:'space', 33:'pageup', 34:'pagedown', 35:'end', 36:'home', 37:'left', 38:'up', 39:'right', 40:'down' }; | |||
|
|
|||
| var focusElement = function() { | |||
| $timeout(function() { | |||
There was a problem hiding this comment.
Was this change required to pass tests? This seems like a behavior change for the keydown actions below.
There was a problem hiding this comment.
Just saw #3705 . It would be good to either separate out this change to fix that particular issue, or to specify in this commit message the reason for this change that's unrelated to implemented ng-if .
There was a problem hiding this comment.
@chrisirhc This PR does not include #3705 suggested changes. I moved the $timeout wrapping the focus() call to the isOpen watch and wrapped the datepicker.focus broadcast with it. This was necessary as the datepicker popup now needs a digest cycle to render with ng-if and will provide a better indication of when the datepicker is ready for focus. Otherwise self.element[0] was not in the DOM yet.
I would like to move the document click binding into that same timeout, but will submit another PR for that.
There was a problem hiding this comment.
I like this change - this is only an issue for the popup, so the $timeout should only be in the popup itself.
|
This LGTM. |
Implementing ng-if on the datepicker-popup directive for performance.
Closes #1915