-
Notifications
You must be signed in to change notification settings - Fork 6.7k
fix(datepicker): format values received from external model changes #2943
fix(datepicker): format values received from external model changes #2943
Conversation
When `ngModel.$render` receives a value updated externally (that is, not as a response to a datepicker event, but some other model update) it does not actually apply configured formatting of any kind. It is assumed that the value is a string, but if it is an instance of `Date` then it is implicitly cast to a string and the element's value is set to that. This fix corrects the behavior by parsing it and applying local filters before assiging the element's value.
Thank you for your contribution. This PR is missing tests that verify the addition of this missing functionality. |
Thanks Chris. I took a cursory stab at creating a plunkr but couldn't repro the issue in that environment. It's possible I'm doing something wrong -- here's what I'm seeing: But, I could be doing something wrong because in this stripped down Plunkr it works just fine: http://plnkr.co/edit/ONQyPbP3ksitNixbagcp?p=preview |
Are you using AngularJS 1.3 ? This looks like a 1.3 bug that has been reported before (#2659). |
Yeah that's what it was, I tried including 1.2 and it worked as expected. Would this help as a fix for that issue? |
This fixes the issue for me. Would love to see it merged. |
Would love to see this merged as well! |
BTW, here's an example of the bug on jsFiddle |
FWIW, @DaveWM has a workaround: #2659 (comment) |
As a side note... if ng-minlength is added to the input field it breaks the datepicker. Anybody have any ideas on what might be the cause of that? |
Why hasn't this been merged yet? |
@jessedhillon are you able to add a test for this PR? |
Yeah, I'll try to get to it this weekend |
Question: isn't this issue covered by the test at line 1316 in |
Should be yes, I assume the test still passes? |
Should it be merged now? |
Yes - there's a Cherry Picking technique involved which I and other new maintainers are going to learn about in tonight's meeting. I'm hoping that several PR's will start to get 'merged' over the coming weeks. |
Sorry for the delay in response. Yes, it's passing the test in my branch. Looks like you already figured it out though! 👍 |
Should be fixed in 5f9afe5 Closing for now. Thanks to all |
…-ui#2659 Closes angular-ui#3293 Closes angular-ui#3279 Closes angular-ui#2440 Closes angular-ui#2932 Closes angular-ui#3074 Closes angular-ui#2943 Closes angular-ui#2733 Fixes angular-ui#3047 Fixes angular-ui#2659 Fixes angular-ui#2681
When
ngModel.$render
receives a value updated externally (that is, not as a response to a datepicker event, but some other model update) it does not actually apply configured formatting of any kind. It is assumed that the value is a string, but if it is an instance ofDate
then it is implicitly cast to a string and the element's value is set to that. This fix corrects the behavior by parsing it and applying local filters before assigning the element's value.