feat(datepicker): allow date strings as the source for ng-model #9554
Conversation
| var parsedValue = angular.isDefined(value) ? Date.parse(value) : null; | ||
|
|
||
| if (parsedValue) { | ||
| value = self.dateLocale.parseDate(parsedValue); |
There was a problem hiding this comment.
Isn't this redundant with Date.parse(value) above? Also, mdDateLocaleProvider.parseDate should expect a string as well, and we're passing a DateNumber object here.
There was a problem hiding this comment.
The difference between Date.parse and new Date is that parse is a little more strict about what it accepts. E.g. the constructor will parse null, false, etc. into January 1st 1970, whereas parse will consider them invalid. Also parse returns the time since epoch, which is why it needs to be passed through parseDate.
As for the reason why it's not doing the logic in parseDate, it's because this should only be done when something, different from the directive, modifies the model.
There was a problem hiding this comment.
Okay, I see. If it isn't too much trouble, since you're making a change for L#407, perhaps you can renamed the variable as parsedNumber to explain that a number is expected to return. I didn't realize Date.parse returned a Number the first time I saw it. That makes more sense knowing that L#408 will convert the Number object into a Date object.
There was a problem hiding this comment.
Just did as a part of the change to the comparison.
crisbeto
force-pushed
the
9535/date-string
branch
3 times, most recently
from
85c3528
to
f1c92cb
Compare
|
@topherfangio - plz review. |
| 'Currently the model is a: ' + (typeof value)); | ||
| throw Error( | ||
| 'The ng-model for md-datepicker must be a Date instance or a value ' + | ||
| 'that can be parsed into a date. Currently the model is a: ' + typeof value |
There was a problem hiding this comment.
nitpick: Change Currently the model is a to Current model: because if it's an Array it should be an instead of a.
crisbeto
force-pushed
the
9535/date-string
branch
from
f1c92cb
to
6c905a3
Compare
crisbeto
force-pushed
the
9535/date-string
branch
from
6c905a3
to
ca6a987
Compare
|
@crisbeto - please confirm this PR is still valid/needed. And rebase to lastest in master. |
ThomasBurleson
added
needs: rebase
This change adds the ability to have a date string (e.g. "2016-01-02") as the source for the datepicker. Until now only Date objects were allowed. Fixes angular#6253. Fixes angular#9535.
crisbeto
force-pushed
the
9535/date-string
branch
from
ca6a987
to
18d0db9
Compare
googlebot
added
the
cla: yes
|
Rebased @ThomasBurleson. |
crisbeto
added
needs: review
ThomasBurleson
added
needs: presubmit
and removed
needs: review
|
LGTM |
This change adds the ability to have a date string (e.g. "2016-01-02") as the source for the datepicker. Until now only Date objects were allowed.
Fixes #6253.
Fixes #9535.