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

datepicker popup removes angularjs basicText formatter from ngModel.$formatters #3279

Closed
wants to merge 1 commit into from

Conversation

cleftheris
Copy link

fixes #2899 and #2069 in angular 1.3.x.

As it seems there is a default formatter in there put by the framework that converts a javascript Date object to a string using the toString() method in order to render the value to the view. I have managed to get around it by inspecting the formatters and just removing the default if found.

@cleftheris cleftheris changed the title Update datepicker.js datepicker popup removes angularjs basicText formatter from ngModel.$formatters Feb 10, 2015
@karianna
Copy link
Contributor

@cleftheris Can you add tests please?

@cleftheris
Copy link
Author

@karianna there are 8 existing tests failing before my fix related to datepicker if I use AngularJS v1.3.11 plus updated Angular Mocks in "datepicker.spec.js".

But my fork of the master branch uses angular.js v1.2.16 and not 1.3.x.
These tests are

  • 200 datepicker directive setting datepickerPopupConfig changes date format
  • 202 datepicker directive as popup initially to display the correct value in input
  • 207 datepicker directive as popup initially opened updates the input correctly when model changes
  • 218 datepicker directive as popup custom format to display the correct value in input
  • 220 datepicker directive as popup custom format updates the input correctly when model changes
  • 221 datepicker directive as popup dynamic custom format to display the correct value in input
  • 223 datepicker directive as popup dynamic custom format updates the input correctly when model changes
  • 224 datepicker directive as popup dynamic custom format updates the input correctly when format changes

Should I also commit a change in the version of the angular lib used for tests "misc/test-lib/angular.js" & "misc/test-lib/angular-mocks.js"?

Please advise

@cleftheris
Copy link
Author

@karianna also keep in mind that I run the tests with my fix in place in angular 1.2.x and all is good there too.

@antoinepairet
Copy link

@cleftheris Thanks.
Another PR was merged.
This commit should have fixed it: 5f9afe5

see the working plunkr: http://plnkr.co/edit/0EEK4JpvW7GXqbhAgVwJ?p=preview

regrads

@karianna karianna added this to the 0.13.0 milestone Feb 21, 2015
antoinepairet pushed a commit that referenced this pull request Feb 21, 2015
Closes #3293
Closes #3279
Closes #2440
Closes #2932
Closes #3074
Closes #2943
Closes #2733

Fixes #3047
Fixes #2659
Fixes #2681
@cleftheris
Copy link
Author

@antoinepairet @karianna Thanks for the update. Its true that my fix is not needed. But I got an additional bug related to commit 5f9afe5 please check my answer here. It is related to the viewValue and parsers/formatters.

@cleftheris
Copy link
Author

@antoinepairet I would like to point out IMHO that parsing the viewValue into a date and then filtering back in order to get the correct string to put in the input element seems like an overkill to me.

The root cause of the problem is as I point out in my PR description a default $formatter put by angular core that does not apply to datepicker popup directive and should be removed once upon initialization. The accepted solution puts unnecessary overhead to the digest cycle.

The default text formatter is pushed in the list of formatters by the ngModelController only when it is bound to an <input type="text" /> element. This is an acceptable behavior because one cannot set an object instance to the value attribute. But when the datepickerpopup comes into play this formatter is not welcome anymore thus I am suggesting we remove it from the list altogether. Here is a link to the angular source of the input directive. You can see what happens when the input is of type text.

Regards

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

Successfully merging this pull request may close these issues.

ControllerAs with DatePicker binds strings, not Dates
3 participants