search

chartjs / chartjs-adapter-luxon

Luxon adapter for Chart.js
MIT License
33 stars 24 forks source link

`isoWeekday` is not a function #22

Closed stockiNail closed 3 years ago

stockiNail commented 3 years ago

I got an error isoWeekday is not a function when I try to use the startOf method of the adapter.

https://github.com/chartjs/chartjs-adapter-luxon/blob/18d1a9b5cd4a441e135a80c7505d4fb3bf089be9/src/index.js#L75

Having a look to Luxon documentation, the isoWeekday(weekday) sounds to belong to Moment and not to Luxon.

The doc https://github.com/moment/luxon/blob/master/docs/moment.md#unit-getters

EDIT Chart.js version: dist/master Luxon adapter: 0.2.1 Luxon lib: 1.24.1

benmccann commented 3 years ago

Just as a heads up, I don't have time to fix at the moment, but I'll review a PR if you want to take a stab

stockiNail commented 3 years ago

@benmccann let me take time because I'm very busy as well but I can try to submit a PR. Ok for you?

benmccann commented 3 years ago

Sure. No rush

stockiNail commented 3 years ago

@benmccann submitted. Take your time!

stockiNail commented 3 years ago

@benmccann fixed again in order to:

  1. calculate the right start of week because previously was wrong the calculation when weekday passed as argument was 7 (Sunday)
  2. normalized the weekday passed as argument in order to have always a consistent result even if the weekday argument is not between 1 and 7.