chartjs/chartjs-adapter-date-fns

Need update for date-fns 2.0

sebtiz13 opened this issue ยท 12 comments

Hello

The adapter need an update for version 2.0 of date-fns
Its explain here: https://github.com/date-fns/date-fns/blob/master/docs/unicodeTokens.md

Old:

var FORMATS = {
datetime: 'MMM D, YYYY, h:mm:ss a',
millisecond: 'h:mm:ss.SSS a',
second: 'h:mm:ss a',
minute: 'h:mm a',
hour: 'ha',
day: 'MMM D',
week: 'DD',
month: 'MMM YYYY',
quarter: '[Q]Q - YYYY',
year: 'YYYY'
};

New version

var FORMATS = {
	datetime: 'MMM d, yyyy, h:mm:ss a',
	millisecond: 'h:mm:ss.SSS a',
	second: 'h:mm:ss a',
	minute: 'h:mm a',
	hour: 'ha',
	day: 'MMM d',
	week: 'w',
	month: 'MMM yyyy',
	quarter: '[Q]Q - yyyy',
	year: 'yyyy'
};

May be show the version of date-fns for backward compatibility

Does @next version work for you?

@kurkle Yes thanks you its work for me
I'm sorry i didn't see @next version before.
date-fns no longer in @next, it could be communicated in master no ?

We have discovered a bug in the pre-release (e745f3d) with the week time unit.

The chart won't render with:

...
xAxes: [
  {
    type: 'time',
    time: {
      unit: 'week',
    },
  }
],
...

This works fine with the default moment library.

Let me know if you need an example to reproduce the bug. ๐Ÿ™‚

Thanks for a great package. I appreciate the exclusion of the huge moment dependency.

Thanks @mpskovvang!
Pushed new release to @next, can you test it?

That was fast, thanks!

The chart now renders with the week unit, however, it is still broken with the isoWeekday: true (week starting on Mondays):

weekStartsOn must be between 0 and 6 inclusively

Minor comment: Moment shows weeks as "Nov 10, 2019", but date-fns shows actual week numbers. Perhaps the format can be changed with the adapter options (I haven't tested).

Edited:
The Chart.js documentation mentions these display formats: https://www.chartjs.org/docs/latest/axes/cartesian/time.html#display-formats

The same format as Moment can be achieved with:

...
displayFormats: {
  week: 'PP'
}
...

Interestingly the format was 'PP' in alpha1. Was the discovered bug actually about isoWeekDay?
Can you try with isoWeekDay: 1?

The bug was discovered with the default value (false) of isoWeekday.

When I found the chart wasn't rendering in the week mode, I switched to moment to verify my implementation. The chart rendered correctly, but I noticed a misplacement between labels and ticks because we use Monday as start of week. That's when I discovered the isoWeekday option in the documentation. moment works fine with and without isoWeekday enabled.

I've tried the adapter both with and without the isoWeekday enabled too, but the alpha2 only works with isoWeekday disabled in my case. The alpha1 doesn't work with week unit at all. ๐Ÿ™‚

Did you try alpha2 with isoWeekDay: 1 (number, not true)?

Sorry, I didn't, because the documentation uses Boolean as the type.

But the number instead of boolean works, thanks! ๐Ÿ‘

Would you consider updating the default formats to match the documentation?

Edited
I just realized that this was actually what you changed. Let me try alpha1 again to ensure I wasn't wrong about the bug in the first place.

Pushed alpha3.

I wasn't able to reproduce the bug in alpha1 - only with isoWeekday: true. I might have lost track of the actual JS build. Apologizes.

However, alpha3 fixes the boolean value with isoWeekday and shows the expected date format.

Version 1.0.0 released