Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid display of date with modelType of type String #22

Closed
nicolasgarnil opened this issue Sep 27, 2016 · 7 comments
Closed

Invalid display of date with modelType of type String #22

nicolasgarnil opened this issue Sep 27, 2016 · 7 comments

Comments

@nicolasgarnil
Copy link

The date is being parsed with the display format instead of the model type format.

Have a look at the following plunker. The first input ("Date 1") should display "01/09/2016" instead of "09/01/2016". The second input ("Date 2") is displayed correctly because moment interpretes it as an invalid format and it fallbacks to the modelType format, which is accepted.

In order to fix this, modelType must precede attrs.dateTimeInput in the inputFormats construction. If I do this, all specs pass, including this special case.

@dalelotts
Copy link
Owner

dalelotts commented Sep 27, 2016

Can you add the passing case also?

@nicolasgarnil
Copy link
Author

The following case fails without inverting the order of modelType and attrs.dateTimeInput :

it('accept valid date string in the model', function () {
  $rootScope.date = '11/12/2016'

  var element = $compile('<input data-date-time-input="DD/MM/YYYY" data-ng-model="date" data-model-type="MM/DD/YYYY">')($rootScope)
  $rootScope.$digest()

  expect(element.val()).toBe('12/11/2016')
})

It passes with the following change:

var inputFormats = [modelType, attrs.dateTimeInput].concat(scope.dateFormats).concat([moment.ISO_8601]).filter(unique)

I don't submit a PR because I may be missing something but it makes sense to me that the model type format should be preferred over the display format to parse the date.

@dalelotts
Copy link
Owner

Model and display are completely separate in Angular (or at least they can be and are in this case) The data coming from the model MUST be in the specified model format (using strings for dates is a bad idea IMO, but it can be done if you know how to handle it.)

The display format is used to transform the model into the display format - input formats also play a role. In short, dates are not simple.

Also, putting code in the issue does not help me debug. Please add it to the plunker.

@dalelotts
Copy link
Owner

Ah, I knew I would regret supporting string formats... 👎

@dalelotts
Copy link
Owner

I'm sure you will find someone that thinks the input format should be preferred for the model format. The documentation directive does not specify the order in which formats are preferred.

After reading a bit more of the code and some angular documentation I'm pretty sure the current behavior is correct. Parsers are used to sanitize / convert the $viewValue (user input). Since the parsers are handling user input, the current code make sense to me - ie display format, model format, then input formats...well, one could possibly convince me that the model format should be the LAST format in the list, not the second.

Anyway, store dates the model and you won't have this problem. =)

@dalelotts
Copy link
Owner

I'm still thinking about this...I think your right that there is a defect...I'm not going to be able to get to this any time soon though.

@dalelotts
Copy link
Owner

Okay, I have it sorted... Nice catch! Expect a new release soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants