Skip to content
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

displayFormats unexpected result #46

Closed
ocarreterom opened this issue Dec 1, 2021 · 11 comments
Closed

displayFormats unexpected result #46

ocarreterom opened this issue Dec 1, 2021 · 11 comments

Comments

@ocarreterom
Copy link

ocarreterom commented Dec 1, 2021

Hi!
I'm trying to display the weekday but the result is unexpected.

I'm not sure if it's an error with this plugin, with Chartjs or I'm doing something wrong.

https://codesandbox.io/s/jovial-hermann-pnb72?file=/src/index.js

I expect to get the label monday, tuesday but I get monday, 29 nov, tuesday, 30 nov

import { Chart, registerables } from "chart.js";
import "chartjs-adapter-luxon";

Chart.register(...registerables);

const canvas = document.getElementById("chart");

const chart = new Chart(canvas, {
  type: "bar",
  data: {
    datasets: [
      {
        data: [
          { x: "1", y: 10 }, // monday
          { x: "2", y: 10 } // tuesday
        ]
      }
    ]
  },
  options: {
    scales: {
      x: {
        type: "time",
        time: {
          unit: "day",
          parser: "E",
          displayFormats: {
            day: { weekday: "long" } // expected monday/tuesday
          }
        }
      }
    }
  }
});

Thanks!

@kurkle
Copy link
Member

kurkle commented Dec 1, 2021

I thinks you need to use the tokens.

cccc | EEEE | day of the week, as an unabbreviated localized string | Wednesday

@stockiNail
Copy link
Contributor

@ocarreterom I think the displayFormats config is wrong.

The displayFormats object can contain values where the key is the unit and the value is the format.
Inner nodes (like day: { weekday: "long" } ) are not enabled.

Try and let us know if the following works:

displayFormats: {
  day: "EEEE"
 }

@ocarreterom
Copy link
Author

ocarreterom commented Dec 1, 2021

@kurkle yes, that works but the result is not localized.

@stockiNail I'm using an object instead because I'm trying to run this line https://github.com/chartjs/chartjs-adapter-luxon/blob/master/src/index.js#L60 and get the localized string using Intl.

If I set an invalid format like { day: { weekday: 'asdf' } } I get this error: weekday must be "narrow", "short", or "long", so the code is running.

It seems that the custom format is merging with this default format.

@ocarreterom
Copy link
Author

@kurkle The Luxon documentation says https://moment.github.io/luxon/#/formatting?id=intl-1

Note toFormat defaults to en-US. If you need the string to be internationalized, you need to set the locale explicitly like in the example above (or more preferably, use toLocaleString).

And this plugin is not setting the locale explicitly.

@stockiNail
Copy link
Contributor

If I set an invalid format like { day: { weekday: 'asdf' } } I get this error: weekday must be "narrow", "short", or "long", so the code is running.

I meant you should use displayFormats: { day: "EEEE" }. I have tried in your sample and seems working.

And this plugin is not setting the locale explicitly.

To set the locale information, you should use adapters.date nodes in the scale options.

See #42 where it's explained which options you can set.

See also this codepen, when a locale is set. https://codepen.io/stockinail/pen/rNyYapz

Config:

scales: {
  x: {
    type: 'time', 
    ...
    adapters: {
      date: {
        locale: 'en'
      }
    },
    ...
  }
}

@stockiNail
Copy link
Contributor

@ocarreterom Using your sandbox.

Options

options

Result

locale

@stockiNail
Copy link
Contributor

I didn't recognize you used locale: 'es'

Here is the result with locale 'es':

Screenshot 2021-12-01 165938
locale

@ocarreterom
Copy link
Author

@stockiNail that works!

It's a little verbose to set the locale two times and I still think that the Intl notation should work.

Thanks for your help!

@stockiNail
Copy link
Contributor

@ocarreterom Agree! See my open issue: chartjs/Chart.js#7859. Planned for CHART.JS version 4

@stockiNail
Copy link
Contributor

stockiNail commented Aug 3, 2022

@stockiNail that works!

It's a little verbose to set the locale two times and I still think that the Intl notation should work.

Thanks for your help!

@ocarreterom with version 1.2.0 of this adapter and Chart.js > 3.9.0, the adapter will use the locale option at chart level if not specified in the adapter.date options.

In this way, you can set the locale only at chart options level.
I think this could be closed, doesn't it?

@ocarreterom
Copy link
Author

@stockiNail yes, this can be close. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants