Closed sebtiz13 closed 4 years ago
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 (https://github.com/chartjs/chartjs-adapter-date-fns/commit/e745f3d74ee4affd296d15b09a49641e7ed18b11) 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
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: https://github.com/chartjs/chartjs-adapter-date-fns/blob/46c3bb8f33891e5a9934e1ab41788d858f2f8c14/src/index.js#L15-L26
New version
May be show the version of date-fns for backward compatibility