Datepicker selection off by one selection for display from ngModel with ng-model-options

[tophallen]

So I have an issue with the datepicker popup when I have ng-model-options present on the element, I have been able to replicate in a plunker here and able to confirm that removing the ngModelOptions does indeed restore normal display of the actual active date.

What seems to be happening on a auto-closing datepicker is that the model is getting updated after the datepicker leaves scope, because of the way the mgModelOptions are delaying the model update until it is too late(the datepicker is closed already).

I think it is the default blur or the debounce I have provided, but I don't see any reason why I wouldn't be able to have them on there for when a user manually types a date in. I found that just wrapping the self.activeDate assignment like so: $timeout(function () { self.activeDate = date; }); in $scope.select function in the DatepickerController does seem to ensure that the displayed selection stays in sync on every change, since it is delaying the update of the active selection until the digest cycle that the model update is happening in, the demo shows it broken, but I included a version of the datepicker.js with the fix, so you can switch and confirm that it does fix it. But there are probably a couple of other options as well to ensuring that the displayed date gets properly updated.

[awerlang]

It could be related to time zone.

Compare:

minDate: new Date('2015-03-01')

and

minDate: new Date('2015-03-01 00:00')

[wesleycho]

This appears to be independent of ng-model-options.

Edit: Sorry, I misunderstood the issue - guess that's what happens when I look at code for 12+ hours.

[wesleycho]

I am a bit confused on this issue - I am not able to reproduce what you are saying (or what I can tell at least) here.

Here is a fork of your Plunker based on current master.

[tophallen]

The fork you just provided does the same thing as well - here are the steps to repro:

• select a date (I picked 3/25/15, but any date will do)
• let the datepicker blur
• open the datepicker again
• select a new date and let it blur again (i picked 4/3/15 but again any date will do)
• open the datepicker again
• it now has 3/25/15 showing as the selected date, but the ng-model says 4/3/15

This will happen continuously - the date showing as selected will always be one selection behind the model.

I've tested this with timezones as well to see if that could be an issue - but I get the same result with UTC(+0), CEST(+1/+2), and PST(-8/-7).

[tophallen]

Just to be thorough on the specific issue - I tested a bit more - It is indeed specifically the debounce option, I removed it and it the active date functions normally - Plunker here

[wesleycho]

I see, this seems to be a serious bug with 1.3 then. I will take a look at it tomorrow when I am more rested :)

[awerlang]

Version 2 of that plunker doesn't work and has this on console:

RangeError: Invalid time value
    at Date.toISOString (native)

Locale here is pt-br

[tophallen]

That is because I was lazy in making my sample and I'm callling new Date(new Date().toLocaleDateString()) which only works with US date formats - if you change the minDate in the js to be new Date(new Date().toISOString().substr(0,10)) it will work in all cultures. I updated the plunk to work here.

[wesleycho]

Moved this to 1.0 - I would like to get this in 0.13, but I believe we may need support in Angular.js in the form of angular/angular.js#11423

[tophallen]

I agree that angular should support promises on $setViewValue, however that change could be a while - if at all(short of 2.0) - as it is a breaking change. It might make more sense to look at using a $timeout and even possibly optionally including the model options to know the debounce time - but an easier fix is to just wrap the $timeout around the render or the activeDate value as I proposed in my OP, as an empty timeout should run the render in the next digest cycle. Thoughts?

[wesleycho]

I do have a concern here with performance - use of $timeout guarantees opening of another $digest cycle, and we need to schedule working on performance optimization for this as soon as we get 0.13 out the door.

Can you explain your use case for this in detail?

[tophallen]

My use-case for the datepicker in general? or the need for debounce? or my desire for a fix sooner rather than later?

I had been running a patched version of the datepicker on my development instance for a while and saw little to no performance hit because digest cycles were already happening on other things with-in scope and the controller - that doesn't mean there isn't a hit obviously, but in my case it was not noticeable. My particular use case for the debounce is most users of the application I'm building will more likely than not be working predominantly from their keyboards and typing in dates rather than trying to find them in a datepicker - however I think it's a fairly big deal if they are scheduling something to be in a few months - and they open the datepicker to find it displaying the current month because the active date didn't update on page load, change, etc. They most certainly would find this functionality confusing and be annoyed that they need to click forward a few months in order to actually change the date from the datepicker.

The specific reason for the debounce around them typing is that there are some validation functions that run around those dates which do have a performance hit if they aren't running minimally(these validations also pull from other datepickers within scope). However - if you feel strongly that this is angular's problem with the model-options and $setViewValue, I can look at implementing a change in my codebase to remove the debounce for the time being. As I said before - it would most likely be a while before the angular team implemented that change.

It is probably a better decision on your part not to implement a slightly hacky change to fix an issue and wait for angular, as so far it seems like my team are the only ones who have even run into this according to SO and the open issues here.

However a small point of note on the empty $timeout - it only guarantees opening another $digest cycle if there isn't one pending or in process, and tbh, I think that if $setViewValue returned a promise you would end up with the same number of $digest cycles either way. But I am not sure (i.e. I would need to test) if the $timeout works for debounce times other than the ones in my plunk, it's very possible that it isn't really even a fix, and I just got lucky with my model-options.

[tophallen]

Just to clarify, I'll run a few more tests and various scenarios with the model options to see if the timeout might even be suitable as a fix and let you know my findings, then we could re-examine if it has a shot of making it into 0.13, if not we can wait for 1.0.

[wesleycho]

I'd be more than happy to review a PR if you want to implement support for this.