mominsamir / smDateTimeRangePicker

Angular Material Date Picker, DateTime Picker, Date Range Picker, Date Time range picker
http://mominsamir.github.io/smDateTimeRangePicker/
MIT License
130 stars 50 forks source link

undefined value on onRangeSelect callback #93

Closed pablorsk closed 7 years ago

pablorsk commented 7 years ago

On datetimerangepicker allways return undefined (v2.0.6) on onRangeSelect callback

Check this plunkr: http://plnkr.co/edit/HGnecL5uVO8oWk3RvzRV?p=preview

Also, Is not possible set initial ngModel range value.

pablorsk commented 7 years ago

I see this problem is solved on last commits. But, it is possible change strings data on callback to Moment/Date objects?

For example, we need send data to BackEnd on an especial format, and I think is not a good solution convert again to Moment/Date object the received data from callback().

Also, with moment/date objects can be used like a precise ngModel. Actual ngModel like a string need more code when this library is used.

Maybe is time to plain change ngModel to Moment or Date objects, or the callback paraments. I prefer ngModel, because on this way we can set a precise initial value (and get it on the same format)

What do you think?

mominsamir commented 7 years ago

Yes it is possible to change to Moment Object on call back, Just pass selected date object before formatting and assigning to setNgModelCtrl.

pablorsk commented 7 years ago

I think Date Object is better, because Angular Material use the same method. On this way, smDateTimeRangePicker will be like an angular-material extension.

mominsamir commented 7 years ago

problem with date object is we need to change entire module to accept this. Other problem is i developed this module because material-angular was not working with moment.

pablorsk commented 7 years ago

problem with date object is we need to change entire module to accept this

I see all project work with external ecosystem with string (callbacks and ngModels). Internally, it work with moment(). Rigth?

Other problem is i developed this module because material-angular was not working with moment.

I see this library like a better and powerfull <md-datepicker />. Also, user position don't see any Date or Moment object. Rigth?

Please, tell me if I wrong. The idea of this question is determine a final way for give to the users a solution with formats, models and callbacks. Maybe @oscar09 can we help us...

I vote for Date Object for ngModel. Another reason of this is generate a more agnostic library. But, if you say Moment is better, I don't have problem, I'm trying to collaborate building a better library.

mominsamir commented 7 years ago

only difference between Date and moment object is later provide pre build api while former does not.

we need to build api and test it before integrating with this.

oscar09 commented 7 years ago

I also like momentJS more. I believe it is more powerful. If the moment objects are needed, I believe they can be added as additional properties of the "range" object that is passed on the callback function. Currently we have "startDate" and "endDate" as string, maybe we can add "startDateMoment" and "endDateMoment" that way the change will provide backward compatibility. I think that the model as a moment object is not a big change; you can take either a string or a moment object to provide backward compatibility as well. However, the "range" will be tricky because you will need to provide an object with the "startDate" / "endDate" properties initialized with a date string or moment object. Right now, you can (sort of) initialize the component with a range:

self.datemodel = moment().startOf('month').format('YYYY-MM-DD') + ' : ' + moment().format('YYYY-MM-DD');

It does not initialize the internal values of the component, but at least it tricks the user that the component has already a value.

oscar09 commented 7 years ago

@pablorsk I am closing this issue, the main problem (undefined value) was fixed on latest commits. I have created issue #100 for the moment objects on the rangeSelect callback.