valor-software / ngx-bootstrap

Fast and reliable Bootstrap widgets in Angular (supports Ivy engine)
https://valor-software.com/ngx-bootstrap
MIT License
5.52k stars 1.69k forks source link

Datepicker does not work with moments #4826

Open jhandel opened 5 years ago

jhandel commented 5 years ago

Bug description or feature request:

Not sure if I should say this is a chronos or bsDatePicker issue, but when you pass a moment (instead of a date) into ngModel or bsValue .toString() is run on it. toString() returns something that is not parased by default by chronos capture

the parser trys to find MM dd YYYY ("L" format) and it doesn't work so well.

I was able to get it to render the correct date by adding a value={{myValue|date: 'yyyy-MM-dd'}} but then updates to the field do not get sent back to the model :-(..

Correct behavior would be to handle the moment as a date not a string, or to parse the string it returns correctly.

some testing with dateInputFormat was done (the format was "ddd MMM DD YYYY kk:mm:ss ZZ") that seemed to work when being passed in via a provider but not so much when attepting to use the value on the input element as a [bsConfig]

Plunker/StackBlitz that reproduces the issue:

https://stackblitz.com/edit/ngx-bootstrap-bsdatepicker-bug-2-fussur

Versions of ngx-bootstrap, Angular, and Bootstrap:

ngx-bootstrap:3.1.2

Angular:7.0.4

Bootstrap:4.1.3

Build system: Angular CLI, System.js, webpack, starter seed:

Angular CLI, but reproduced with stackblitz so what ever it uses too.

valorkin commented 5 years ago

Adding support for momentjs or any other date lib objects will mean that we will have to add 3rd party dependency for each. Most proper way is to convert momentjs object toDate and set it as a value, moment date wrapper always have it ._d field so it will work faster than formatting to string.

Closing as designed

jhandel commented 5 years ago

If others run into this like I have.. You can overwrite how moment does its toString()... Not elegant but universal for your project

(moment as any).fn.toString = function() { return this.format("L"); }; <-- now moment's to string is a simple localized date

valorkin commented 5 years ago

If rather suggesting to use .toDate() then .toString()

valorkin commented 5 years ago

One way or another I will think about it, how it could be done: first thing on my mind is to create moment date adapter!

valorkin commented 5 years ago

@domainv point to roadmap, this is second mention of custom date adaptors per week

jhandel commented 5 years ago

I would be cool with idea of a custom adapter 👍 (because I am sitting on ASPNetzero framework which has a mountain of Angular and theme framework that sits on Moment)

Domainv commented 5 years ago

@valorkin roger that

egandro commented 5 years ago

I love the idea that ngx-bootstrap is not using moments. Moments is buggy as hell :(

One example:

let x = day when daylight saving starts. so length of x + 30 days = 30days + minus hour :)

They will never fix this - it doesn't matter how much bug reports we write :(

valorkin commented 5 years ago

That's why I rewrote momentjs in typescript, chronos module. Just need a bit of time to finish :(

egandro commented 5 years ago

In my new projets i use Luxon/DateTime. The point is - it has very few bugs - but it' doesn't support natural dates e.g. "Last tuesday".

oguzhankahyaoglu commented 5 years ago

Are there any updates on this issue? We are using AspnetZero framework which heavily relies on momentjs and ngx-bootstrap and there are lots of compilation errors due to the momentjs integration.

ismcagdas commented 5 years ago

@oguzhankahyaoglu could you create an issue on http://support.aspnetzero.com and we can help you if the problem is related to AspNet Zero.

oguzhankahyaoglu commented 5 years ago

@oguzhankahyaoglu could you create an issue on http://support.aspnetzero.com and we can help you if the problem is related to AspNet Zero.

I dont have the account information for the licensing product, my company acquired the license and I dont have it actually. By the way, we solved the issue by defining Date properties for each usage of Datepicker/momentjs properties and get/setting them accordingly. Moreover, RAD tool needs some improvements if there is a repository for that tool, i can write my opinions there.

ismcagdas commented 5 years ago

@oguzhankahyaoglu if you can find the account information and create an issue, we can try to help you because we are using latest version of ngx-bootstrap (4.2.0) and moment date fields and we don't have such a problem. If not, you can continue using date type until this issue is fixed.