-
Notifications
You must be signed in to change notification settings - Fork 6.7k
fix(datepicker): Don't parse the date twice #3682
Conversation
Works for me. Thanks! |
Looks good and seems to be fixing some problems. Thanks for the PR! Will look into this soon to merge it. |
@realityking Can you please add some tests for this? |
@rvanbaalen Just to clarify, these tests would need protractor. Is that ok? |
My bad. Now I see your comment about Karma tests. Was running an hour of slightly brainless PR and Issue triaging 😉 |
No problem, but I still don't have an answer ;) Are protactor tests fine? |
+1 |
This change would have an effect on HTML5 date inputs I believe, since the $viewChangeListener is the only callback that gets fired that parses the date input. Can you produce a Plunker with an example of this in action for that situation? |
Any word on the status of this PR? |
I haven't had time to set up protractor tests, so at least that's still open. I'd be great if someone could create the test mentioned by @wesleycho as I'm pretty short on time right now. |
I can confirm that this fixes some parsing and validation issues (particularly #1289), specially when manually changing the date. For those using Bower who would like a hotfix, you may fetch AngularUI from here (minified version not updated): https://github.com/almeidap/angular-ui-bootstrap |
Fixes #3644, #3617
Potentially fixes #3643 as well, but I haven't tested that one yet.
Unit tests are missing, as I'm at a loss how to test these issues with karma. I'd appreciate some pointers in this regard. If it's fine to add protractor tests, I'd be happy to do so.