ng-matero / extensions

Angular Material Extensions Library.
https://ng-matero.github.io/extensions/
MIT License
388 stars 48 forks source link

[BUG] DateTimePicker does not support AM/PM when parsing text from an input. #284

Closed HeathJared closed 1 month ago

HeathJared commented 1 month ago

The issue is reproducible using the current demo website. https://ng-matero.github.io/extensions/components/datetimepicker/overview

Under "Datetimepicker configuration" select the following options.

In the "Result" input type "4:30 PM" and hit tab. The value converts to "04:30" (and uses the 24-hour format instead of 12-hour) which represents "4:30 AM". This can be confirmed when opening the DateTimePicker using the toggle icon. It opens with 4:30 AM selected.

Recording 2024-03-13 135833

nzbin commented 1 month ago

Thanks for your advice. This isn't a bug. You should modify the datetime config if you want to use the ampm.

https://github.com/ng-matero/extensions/blob/f2b54708ebc662c153603cae460059b979749745/projects/dev-app/src/app/datetimepicker/datetimepicker-demo.component.ts#L51-L70

{ 
   parse: { 
     timeInput: 'HH:mm A', 
   }, 
   display: { 
     timeInput: 'HH:mm A', 
   }, 
 }, 
HeathJared commented 1 month ago

Thanks for showing me how to fix it.

However, I still think that this behavior could still be improved as it does not appear consistent.

Currently, when the mtx-datetimepicker is in "datetime" mode, it appears to support AM/PM by default. It is unexpected that when changing it to "time" only mode that it would no longer support AM/PM without customizing the provider.

I also believe that it would be good to support AM/PM by default when in 12-hour mode (as it requires AM/PM).

For my immediate tests I was using Angular 14 and MtxNativeDatetimeModule.

Using the same setup, with [type]="'datetime'" with [twelvehour]="true", typing 12/12/12 4:30 PM works as expected no further customizations needed. Changing the type to time, the behavior changes to no longer support typing 4:30 PM without customizing the providers.

Please consider.

Thanks.

HeathJared commented 1 month ago

For anyone else having this issue, the luxon adapter works as expected.