-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Give the boot to chartjs-adapter-moment #7155
Conversation
Wow! I'm not sure what you changed to get it working that I didn't figure out, but glad you figured it out! We should update the docs too. E.g. You can also update some of the comments in the time scale code: benmccann@e586f4e |
I'm surprised you don't have to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may require website changes to ensure that the same files still work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked closer. The samples will need to be updated as they do not include any adapter, only moment. Perhaps we can use this as an opportunity to convert them to use luxon instead
It probably is a good idea to demonstrate something other than Moment. Luxon is more fully featured and has a better API and date-fns is smaller. Moment is rarely the best choice to use on the client side |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Thanks!
No description provided.