vitalets / angular-xeditable

Edit in place for AngularJS
http://vitalets.github.io/angular-xeditable
MIT License
1.91k stars 403 forks source link

editable-combodate multiple problems. #705

Closed josemonsalve2 closed 6 years ago

josemonsalve2 commented 6 years ago

I am having some troubles with editable-combodate when used with different formats.

Please refer to this fiddle.

First if you only have Year (YYYY) format, the displayed value gets reduced by one after submission, even though the angular model is updated correctly. If I add months this behavior is not like that anymore. (See problem 1 and problem 2 in the attached jsfiddle)

Second, if I have Months only (MMMM), there is an error when trying to do the following in xeditable.js:430.

combodate.$widget.find('select').bind('change', function(e) {
          //.replace is so this works in Safari
          self.scope.$data = combodate.getValue() ?
              (new Date(combodate.getValue().replace(/-/g, "/"))).toISOString() : null;
        });

This problem can be seen in the jsfiddle problem 3.

Finally, whenever a new date cannot be created in this line there is actually a problem. Let's say that we have dd/MM/YYYY like in problem 4. if the selected day is more then 12 it won't change, if it is less than 12 it will change the month instead.

In general there is a problem between date and format.

a possible solution would be to use moment to parse the date with the specific format.

josemonsalve2 commented 6 years ago

I am not that familiarized with your code, but would this work?

combodate.$widget.find('select').bind('change', function(e) {
          //.replace is so this works in Safari
          self.scope.$data = combodate.getValue() ?
              moment(combodate.getValue(), combodate.options.format).toDate().toISOString() : null;
        });
josemonsalve2 commented 6 years ago

I just realized that the problem with YYYY - 1 is with the | date: filter

ckosloski commented 6 years ago

It appears your change would work. Did you want to try and create a PR? If so, add test cases to dev-combodate folder for all your tests cases in the fiddle.

josemonsalve2 commented 6 years ago

I will do this by the end of next week, I can't right now. I am trying to finish a project.

ckosloski commented 6 years ago

@josemonsalve2 would you like me to help you with a PR? I believe I have your change working locally for me.

josemonsalve2 commented 6 years ago

I would highly appreciate that. I have had no time to do that.