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

Datepicker and Timepicker use $modelValue instead of $viewValue. #2069

Closed
AlexHi opened this issue Apr 16, 2014 · 18 comments
Closed

Datepicker and Timepicker use $modelValue instead of $viewValue. #2069

AlexHi opened this issue Apr 16, 2014 · 18 comments

Comments

@AlexHi
Copy link

AlexHi commented Apr 16, 2014

It seems that all angular controls use $viewValue in the $render function, so we are able to convert model value(using $formatters) before displaying.

@cleftheris
Copy link

+1

@cleftheris
Copy link

I tried the following fix on the render method "TimepickerController" and worked like a charm.
Only change the first line. Please update the source code because it is very difficult to maintain my own version of such a popular library.

this.render = function() {
    //var date = ngModelCtrl.$modelValue ? new Date( ngModelCtrl.$modelValue ) : null;
    var date = ngModelCtrl.$viewValue ? new Date( ngModelCtrl.$viewValue ) : null;
    if ( isNaN(date) ) {
        ngModelCtrl.$setValidity('time', false);
        $log.error('Timepicker directive: "ng-model" value must be a Date object, a number of milliseconds since 01.01.1970 or a string representing an RFC2822 or ISO 8601 date.');
    } else {
        if ( date ) {
            selected = date;
        }
        makeValid();
        updateTemplate();
    }
};

My fork with the fix is here https://github.com/cleftheris/bootstrap/blob/master/src/timepicker/timepicker.js

@Brovert
Copy link

Brovert commented Jan 14, 2015

+1

@Brovert
Copy link

Brovert commented Jan 14, 2015

Will this be implemented in any future versions of ui-boostrap? We just ran into this issue as well and I noticed the problem before seeing this forum.

@MoeBDC
Copy link

MoeBDC commented Jan 14, 2015

+1

1 similar comment
@nguyen-sonny
Copy link

+1

@karianna
Copy link
Contributor

Hi @cleftheris - Can you submit a PR with an accompanying test for your change?

@cleftheris
Copy link

Hi @karianna I will try to make a PR as soon as possible.
I am currently having trouble with the $viewValue in datePicker 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 hacked my way around it by removing it conditionally.

Will post an update soon

@Brovert
Copy link

Brovert commented Feb 18, 2015

Do we know if this will be fixed in an upcoming version?

@antoinepairet
Copy link

This commit should have fixed it: 5f9afe5

regrads

@karianna karianna removed the PRs plz! label Feb 21, 2015
@karianna karianna added this to the 0.13.0 milestone Feb 21, 2015
@cleftheris
Copy link

@antoinepairet this is not completely compatible with angular version 1.3.x.
Lets say for example that I have a converter directive named "number-to-date-converter" and I need the raw value passed to the datepicker as popup to remain a number. When I update the raw model value the input gets updated but not the popup calendar!!.
here is the directive I am using for testing. (It could be a converter from anything to date and back. I am just using number 2 date for simplicity).

angular.module('ui.bootstrap.demo').directive('numberToDateConverter', function () {
  return {
    require: 'ngModel',
    restrict: 'A',
    link: function (scope, element, attr, ngModel) {
            //view -> model (runs last)
            ngModel.$parsers.unshift(function (date) {
                return date.getTime();
            });
            //model -> view (runs last)
            ngModel.$formatters.unshift(function (date) {
                return angular.isNumber(date) ? new Date(date) : date;
            });
        }
  };
});

and here is the plunker demonstrating the problem.

We should always use the $viewValue inside the render method (check the assignment of scope.date). This fixes the problem.

// Outer change
ngModel.$render = function() {
    var date = ngModel.$viewValue ? dateFilter(parseDate(ngModel.$viewValue), dateFormat) : '';
    element.val(date);
    scope.date = parseDate( ngModel.$viewValue );
};

Thanks

@wesleycho
Copy link
Contributor

Is this possibly related to #3159? It seems we have a lot of issues that may be because dateParser is not supporting everything in dateFilter.

@cleftheris
Copy link

I think not. This is just a matter of whether we are using the $modelValue vs the $viewValue inside the render method.

@wesleycho
Copy link
Contributor

I see - this was addressed in the datepicker recently, if there is a PR for doing the same for the timepicker, then I will gladly look at it. If not, I might take a stab at it this week.

@gitnik
Copy link

gitnik commented Mar 27, 2015

👍 same issue here. A new release would be great

chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Apr 5, 2015
- Remove direct calls to $render
- Remove extra call to $render during intialization (only run when format is
  changed)
- Save last date value in formatter
- Remove use of ngModel.$modelValue as users may add parsers to convert
  $modelValue to other formats

Relates to angular-ui#2069

Fixes angular-ui#3349
chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Apr 5, 2015
- Separate validation from parsing so that validation still runs on model
  change
- Remove direct calls to $render
- Remove extra call to $render during intialization (only run when format is
  changed)
- Save last date value in formatter
- Remove use of ngModel.$modelValue as users may add parsers to convert
  $modelValue to other formats

Relates to angular-ui#2069

Fixes angular-ui#3349
@chrisirhc
Copy link
Contributor

Verified that #3494 fixes @cleftheris 's issue / use case.

wesleycho added a commit that referenced this issue Apr 5, 2015
- Moves render logic converting model values into date or null object into a formatter, which allows the render function to more correctly render using the viewValue

- remove parentheses as per CR

Relates to #2069

Fixes #3160
Closes #3427
chrisirhc added a commit to chrisirhc/angular-ui-bootstrap that referenced this issue Apr 6, 2015
- Separate validation from parsing so that validation still runs on model
  change
- Remove direct calls to $render
- Remove extra call to $render during intialization (only run when format is
  changed)
- Save last date value in formatter
- Remove use of ngModel.$modelValue as users may add parsers to convert
  $modelValue to other formats

Relates to angular-ui#2069

Fixes angular-ui#3349
@cleftheris
Copy link

@chrisirhc looks good!

@karianna
Copy link
Contributor

karianna commented Apr 7, 2015

I think this can be closed now.

@karianna karianna closed this as completed Apr 7, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants