Skip to content
This repository has been archived by the owner on May 5, 2018. It is now read-only.

date not updating UI when existing date object is modified #331

Closed
coldfire22x opened this issue Jan 4, 2013 · 19 comments
Closed

date not updating UI when existing date object is modified #331

coldfire22x opened this issue Jan 4, 2013 · 19 comments
Assignees
Labels
Milestone

Comments

@coldfire22x
Copy link

I've noticed that the ui-date directive will respond when the model (a Date object) is replaced with an entirely new Date object, however it does not update the display if the existing Date object is modified via date.setDate().

If you manually set up a $watch on the existing Date object, it does fire the callback when updating via date.setDate() so I'm wondering if it's a bug that the display doesn't catch this and update accordingly?

@petebacondarwin
Copy link
Member

Hi @coldfire22x. Thanks for spotting this.

To progress bugs like this, it would be really helpful to have a bit of runnable code, perhaps a Plnkr: http://plnkr.co/edit/AKX5bxbfpG1JEN5H7Fgo?p=preview, or even better a Jasmine unit test that demonstrates the problem.

As you can see from the Plnkr above, the identity of the date object does not change when we call setDate. This means that while the string interpolation updates and the $watch that is checking deep object equality is triggered, the basic object identity $watch is not triggered.

Currently the date directive is using the object identity form of $watch, which is why it is not triggering a change.

I guess this is a bug and so I'll put in place a fix.

@ghost ghost assigned petebacondarwin Jan 6, 2013
@ProLoser
Copy link
Member

ProLoser commented Jan 6, 2013

You're just going to watch by. Value instead of by reference right?
On Jan 6, 2013 10:42 AM, "Pete Bacon Darwin" notifications@github.com
wrote:

Hi @coldfire22x https://github.com/coldfire22x. Thanks for spotting
this.

To progress bugs like this, it would be really helpful to have a bit of
runnable code, perhaps a Plnkr:
http://plnkr.co/edit/AKX5bxbfpG1JEN5H7Fgo?p=preview, or even better a
Jasmine unit test that demonstrates the problem.

As you can see from the Plnkr above, the identity of the date object does
not change when we call setDate. This means that while the string
interpolation updates and the $watch that is checking deep object equality
is triggered, the basic object identity $watch is not triggered.

Currently the date directive is using the object identity form of $watch,
which is why it is not triggering a change.

I guess this is a bug and so I'll put in place a fix.


Reply to this email directly or view it on GitHubhttps://github.com//issues/331#issuecomment-11932235.

@petebacondarwin
Copy link
Member

That is plan A, but I wanted to have a think about the repercussions of
doing so...

On 6 January 2013 20:44, Dean Sofer notifications@github.com wrote:

You're just going to watch by. Value instead of by reference right?
On Jan 6, 2013 10:42 AM, "Pete Bacon Darwin" notifications@github.com
wrote:

Hi @coldfire22x https://github.com/coldfire22x. Thanks for spotting
this.

To progress bugs like this, it would be really helpful to have a bit of
runnable code, perhaps a Plnkr:
http://plnkr.co/edit/AKX5bxbfpG1JEN5H7Fgo?p=preview, or even better a
Jasmine unit test that demonstrates the problem.

As you can see from the Plnkr above, the identity of the date object
does
not change when we call setDate. This means that while the string
interpolation updates and the $watch that is checking deep object
equality
is triggered, the basic object identity $watch is not triggered.

Currently the date directive is using the object identity form of
$watch,
which is why it is not triggering a change.

I guess this is a bug and so I'll put in place a fix.


Reply to this email directly or view it on GitHub<
https://github.com/angular-ui/angular-ui/issues/331#issuecomment-11932235>.


Reply to this email directly or view it on GitHubhttps://github.com//issues/331#issuecomment-11933926.

@rickkln
Copy link

rickkln commented Feb 21, 2013

Any news on when this will be fixed/changed?

@petebacondarwin
Copy link
Member

Looking at this, it is a bit more tricky since we are not actually doing the watching of the model! It is ngModel that does this and so we can simply change the type of watch it is using. I am considering adding a second watch that watches the data object "deeply" and then completely replaces the object if it changes, which would trigger ngModel's watch.

Any thoughts on this approach?

@rickkln
Copy link

rickkln commented Feb 21, 2013

That sounds like it would work. I really need to be able to watch the value of the date because on my app if the user changes the date then I need recheck for availability before allowing the user to make a booking.

@petebacondarwin
Copy link
Member

I will look to add this fix, but I guess I would question what part of your
application is calling setDate on a date object? Can't you just change
that to create a new date object each time instead?

On 21 February 2013 21:09, Rick Kleinhans notifications@github.com wrote:

That sounds like it would work. I really need to be able to watch the
value of the date because on my app if the user changes the date then I
need recheck for availability before allowing the user to make a booking.


Reply to this email directly or view it on GitHubhttps://github.com//issues/331#issuecomment-13912952.

@rickkln
Copy link

rickkln commented Feb 21, 2013

Well all I want is to be able to automatically do something every time the person picks a new date with the datepicker. As far as I can see the only way to do this would be to watch the date variable and when a new one is picked do the thing you want to do.

@petebacondarwin
Copy link
Member

That is precisely how you should do it. That is the angular way. How else
would you do it?

If you really wanted to be close into to the date directive, then you could
create a directive that requires ngModel and then adds in a function to the
$parsers array, which will also be called every time a date gets picked.

One more thing you can do is use the ng-change directive on your ui-date
input, which gets called every time the model is updated from the view.

Pete

On 21 February 2013 21:45, Rick Kleinhans notifications@github.com wrote:

Well all I want is to be able to automatically do something every time the
person picks a new date with the datepicker. As far as I can see the only
way to do this would be to watch the date variable and when a new one is
picked do the thing you want to do.


Reply to this email directly or view it on GitHubhttps://github.com//issues/331#issuecomment-13914843.

@rickkln
Copy link

rickkln commented Feb 21, 2013

Yes, I know it is the angular way and the right way to do it but currently it doesn't work because watching the date variable does nothing because of this issue. I will try to bypass the problem using ng-change as you suggested.

@petebacondarwin
Copy link
Member

Hold on Rick. This is a different issue to the original problem. Watching
the date variable should work. Can you provide an example of it not
working?

On 21 February 2013 22:03, Rick Kleinhans notifications@github.com wrote:

Yes, I know it is the angular way and the right way to do it but currently
it doesn't work because watching the date variable does nothing because of
this issue. I will try to bypass the problem using ng-change as you
suggested.


Reply to this email directly or view it on GitHubhttps://github.com//issues/331#issuecomment-13915797.

@rickkln
Copy link

rickkln commented Feb 21, 2013

I don't have the time to put together a plunkr right this second but basically I had this line in my html (jade):

input#datePicker(ng-model="selectedDate", value="{{selectedDate | date:'dd MMMM yyyy'}}", ui-date, readonly)

and then this method in the controller:

$scope.$watch('selectedDate', function() {
  console.log("hello");
  $scope.availabilityDone = false;
});

And no matter if you changed the date you would never get "hello" in the console.

I changed it to this now and it works! -

input#datePicker(ng-model="selectedDate", value="{{selectedDate | date:'dd MMMM yyyy'}}", ui-date, readonly, ng-change="changeDate()")

and

$scope.changeDate = function() {
  console.log("hello");
  $scope.availabilityDone = false;
};

@rickkln
Copy link

rickkln commented Feb 21, 2013

All the other watches on my drop downs work without ng-change.

@petebacondarwin
Copy link
Member

Will look at this tomorrow.

On 21 February 2013 22:27, Rick Kleinhans notifications@github.com wrote:

I don't have the time to put together a plunkr right this second but
basically I had this line in my html (jade):

input#datePicker(ng-model="selectedDate", value="{{selectedDate | date:'dd MMMM yyyy'}}", ui-date, readonly)

and then this method in the controller:

$scope.$watch('selectedDate', function() {
console.log("hello");
$scope.availabilityDone = false;
});

And no matter if you changed the date you would never get "hello" in the
console.

I changed it to this now and it works! -

input#datePicker(ng-model="selectedDate", value="{{selectedDate | date:'dd MMMM yyyy'}}", ui-date, readonly,       ng-change="changeDate()")

and

$scope.changeDate = function() {
console.log("hello");
$scope.availabilityDone = false;
};


Reply to this email directly or view it on GitHubhttps://github.com//issues/331#issuecomment-13916958.

@rickkln
Copy link

rickkln commented Feb 21, 2013

Thanks, take your time though it is no longer a priority issue for me seeing as adding the ng-change had the desired effect for me. Thanks for the suggestion.

@petebacondarwin
Copy link
Member

This works for me...

http://plnkr.co/edit/c065LJ9CtGgeBbbeUgRE?p=preview

On 21 February 2013 22:31, Rick Kleinhans notifications@github.com wrote:

Thanks, take your time though it is no longer a priority issue for me
seeing as adding the ng-change had the desired effect for me. Thanks for
the suggestion.


Reply to this email directly or view it on GitHubhttps://github.com//issues/331#issuecomment-13917206.

@rickkln
Copy link

rickkln commented Feb 22, 2013

Yes that works, that's really weird, I have no idea what the problem is locally then.

@shaungrady
Copy link
Contributor

I'm assuming this is an invalid issue then.

@petebacondarwin
Copy link
Member

Time to move on :-)

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

No branches or pull requests

5 participants