fix(timepicker): Make sure ng-model is pristine on initialization

[wesleycho]

• On initialization, it is possible for the timepicker ng-model to become dirty due to the update to the hours and minutes values when the model value is parsed. This change guarantees the model is pristine on first render.

Working Plunker here.

Fixes #3160.

[chrisirhc]

Actually, this isn't quite right either. updateHours and updateMinutes shouldn't be called when updating the ngModel, as they're in the ng-change expression. The ng-change expression should only fire when the user interacts with the input.

This is actually happening because of the change in type="text" handling in AngularJS 1.3 . We use type="text" fields with an model with number values.

We can either:

1. Change the hour and minute fields from type="text" to type="number". (but note that Chrome / Webkit might add ticker buttons on these fields, which imho is fine)
2. Store the hour and minute as strings. Probably a bad idea, but putting it out there anyway.

[wesleycho]

I didn't really like this either - what is happening is updateHours and updateMinutes are called in order to update validity, which is fine. The problem seems to be with the refresh function, where it is calling ngModel.$setViewValue - perhaps a more correct fix is to move that out of the refresh function then and check for all functions calling it?

[chrisirhc]

Hm I think you're misunderstanding me, these functions shouldn't be called in these cases.
There's a check where the model value != view value because entering hours = 10 gets checked with 10 === '10' which is false. This is because the numbers are being converted into strings.

In angular 1.3, the ngModel forces the format of text input fields to be string while before, it could be a number.

[wesleycho]

Oh, didn't realize that was happening - would adding a formatter that converts it into a string help any here?

[chrisirhc]

Yes, we could also change the timepicker.html template so that the input is a type="number" as I mentioned above.

[wesleycho]

I took a look at turning it into a number, but it turns out all of the timepicker unit tests assume support for string/date values.

The formatter approach might be the correct approach here - it turns out there is some logic in the render method that should actually be in the formatter.

I will write a unit test for this and open a new PR.

[wesleycho]

Superceded by #3427