matheo / angular

Open Source Angular Libraries: MatDataSource, MatDatepicker
http://matheo.co/demos/
MIT License
60 stars 15 forks source link

Luxon Date Adapter #33

Closed chikakow closed 3 years ago

chikakow commented 3 years ago

Describe the bug

I'm using matheo Luxon Date Adapter for my app. I am using Luxon in my entire app. I had to copy the code from the source code and swapped out with unofficial Google LuxonDateAdapeter. I realized that Google version and Matheo version is almost identical except Matheo version has conversion for time for obvious reason. However, I found one more difference which is not related to time. getDayOfWeek is modified to negate 1 from luxon default weekday. The default if Sunday returns 7, Matheo version return 6. When I got the code from Google, I also got unit test to go with it too. Then now I swapped with Matheo version, this getDayOfWeek unit test is failing. To fix this unit test, I can just remove negation of 1 but is there a specific reason why Matheo version is negating 1?

Minimal Reproduction

Find the Google version LuxonAdapter from this link https://github.com/andreialecu/ngx-material-luxon/blob/main/projects/ngx-material-luxon/src/lib/adapter/luxon-date-adapter.ts

And Find Unit Test for the Google Version LuxonDateAdapter from this link https://github.com/andreialecu/ngx-material-luxon/blob/main/projects/ngx-material-luxon/src/lib/adapter/luxon-date-adapter.spec.ts

Use Matheo Verion LuxonDateAdapeter and run the unit tests

Expected behavior

All test should still pass.

Screenshots

image image

Your Environment

Angular Version:


Angular CLI: 12.0.4
Node: 14.17.4
Package Manager: npm 7.17.0
OS: win32 x64

Angular: 12.0.4
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, material, platform-browser, platform-browser-dynamic
... router, service-worker

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1200.4
@angular-devkit/build-angular   12.0.4
@angular-devkit/core            12.0.4
@angular-devkit/schematics      12.0.4
@angular/flex-layout            12.0.0-beta.34
@schematics/angular             12.0.4
ng-packagr                      12.0.5
rxjs                            6.6.6

matheo commented 3 years ago

Hi @chikakow Thanks for pointing out this issue.

I was trying to make luxon compatible to Date.getDay which returns 0 for Sunday.

It seems that the fix needs to consider that for luxon, the weekday states Get the day of the week. 1 is Monday and 7 is Sunday also the NativeDateAdapter uses 0 = Sunday, 1 = Monday for the getFirstDayOfWeek

so the fix would look like:

getDayOfWeek(date: DateTime): number {
  return date.weekday === 7 ? 0 : date.weekday;
}

what do you think?

chikakow commented 3 years ago

Yes that code works. The previous code date.weekday - 1 was actually creating off by 1 problem with the picker. May 22nd is Saturday image

With the new code you provided above, It falls into Saturday. image

matheo commented 3 years ago

Great news @chikakow you can try @matheo/datepicker@11.2.16 which includes this fix :)