Datepicker min max refactor

[stevecavanagh]

Add validation to the input associated with the datepicker-popup. Cleaned up some code formatting and eliminated some duplicate code.

Fixes: #1688

[jamesjwarren]

Nice 😄

[stevecavanagh]

Thanks @RIAstar ! I'll look into this and create the necessary tests around this change.

[stevecavanagh]

@RIAstar, Thanks for identifying the issue and the fix. In my test, I used format (dd.MM.yyyy) since this format was described in an existing test. The test failed prior to your fix. Thanks!

[john-ring]

Thanks for the validation code - this is exactly what I was looking for! It seems like the validation isn't quite right though - with max-date="2015-02-18T17:13:58.847Z" and min-date="2013-02-18T17:13:58.847Z", the error object for the date "02/19/2105" is {"parse":true,"minDate":true,"maxDate":true}
The minDate should pass validation yet it doesn't.

[stevecavanagh]

@JaggyGT, Thanks for uncovering this problem. I'll create a new test to cover this scenario and then change code as needed.

[stevecavanagh]

@JaggyGT , I can't seem to replicate this problem. Can you please provide a Plunker? I will be submitting a PR because using the parseDate function is doing unnecessary work. I believe using dateParser.parse should be sufficient.

[stevecavanagh]

@antoinepairet Please let me know if you'd like a new PR with squashed commits. Note: I revised my tests to reflect changes made by: 23936f9. Also, I tested the datepicker in the demo with both Angular 1.2.x and 1.3.x after incorporating #3099

[wesleycho]

This is a lot of parsers and formatters to be adding to the arrays. Ideally we should only be adding one parser and formatter which executes all of the modifications we desire.

[wesleycho]

There seems to be some history pollution here with the merge commits appearing in the history. Can you open a new PR with a cleaned up history?

[stevecavanagh]

Will do this evening.

---- Wesley Cho wrote ----

There seems to be some history pollution here with the merge commit appearing in the history. Can you open a new PR with a cleaned up history?

—
Reply to this email directly or view it on GitHub.

[stevecavanagh]

It looks like I may need to adjust this PR based on some of the recent commits. I will probably not have a new PR available until 3/29 or so.

---- Steve Cavanagh wrote ----

Will do this evening.

[saintnikopol]

Directly setting date like '01.01.2090' into input fied
is corretly validated on date limit ('maxDate')

but setting date like '01.01.20909' is not validated on date limit.

possible cause is that formatters not called
ngModel.$formatters.unshift(minLimitFormat);

direct cause is that in function "isDateLimitMet" such dates parsed as undefined "dateParser.parse('01.01.19002) will be undefined"
and further comparison is not correct.

  function isDateLimitMet(limitName, dateToCheck, viewValue) {
        var parsedDate = dateParser.parse(viewValue, dateFormat);
        var dateCompare = compareDates(parsedDate, dateToCheck);
          console.log('< < < < < < < < compare dates: parsedDate = ' + parsedDate + ' // dateToCheck =' +dateToCheck + '');
        if (limitName == 'minDate') {
          return !dateCompare || dateCompare > 0;
        } else if (limitName == 'maxDate') {
          return !dateCompare || dateCompare < 0;
        }
      }

[saintnikopol]

Real cause is that in parseDate function "new Date(viewValue);" is used as fallback:

[569] var date = dateParser.parse(viewValue, dateFormat) || new Date(viewValue);

but in compare function it's not used

[525] var parsedDate = dateParser.parse(viewValue, dateFormat);

[stevecavanagh]

Thanks! Yes, looks like there are a few changes and some additional tests needed.

---- saintnikopol wrote ----

Read cause is that in parse check "new Date(viewValue);" is used as fallback,

[569] var date = dateParser.parse(viewValue, dateFormat) || new Date(viewValue);
but in compare function it's not used
[525] var parsedDate = dateParser.parse(viewValue, dateFormat);

—
Reply to this email directly or view it on GitHub.

[stevecavanagh]

@saintnikopol , @wesleycho , Sorry for the delay. I haven't looked at eliminating the extra formatters yet, but I am wondering whether you think I've got the test cases handled properly in this branch: https://github.com/stevecavanagh/bootstrap/tree/fix(datepicker)MinMaxInput ?

I ended up making some changes to the dateParser. (I'm guessing these need to go in a separate PR.) Also, I guess there have been some recent updates to the dateParser that I will need to merge my code with.
Thanks.

[saintnikopol]

Changes to code and tests in branch "fix(datepicker)MinMaxInput" looks correct.
I'm that it better to create separate PR

[karianna]

@stevecavanagh are you able to update this PR? We're looking to release 0.13.0 shortly.

[rvanbaalen]

I'm wondering if this is even required for Angular 1.3 support, @karianna. It seems like an added feature for Datepicker that can easily be pushed to 0.13.x if I'm correct.

[karianna]

@rvanbaalen agreed - moving.

[stevecavanagh]

Sorry it's taken me so long to incorporate the suggestions here into PR 3606. Please let me know if there are others. I'd like to close this PR in favor of #3606.

[karianna]

Closing as requested