pepstock-org / Charba

J2CL and GWT Charts library based on CHART.JS
https://pepstock-org.github.io/Charba-Wiki/docs
Apache License 2.0
62 stars 6 forks source link

TickCallback with CartesianTimeTick #42

Closed ak80 closed 5 years ago

ak80 commented 5 years ago

Hi,

I'd like to apply custom formatting for ticks on a CartesianTimeAxis. The signature of the Callback says it will give me a double value, but when I print it to the web console it looks like a formatted date value:

Here is my code in the tick callback

    ticks.setCallback((axis, v, i, list) -> {
      DebugLogger.debugLog("axis={}", axis);
      DebugLogger.debugLog("v={} i={}, list={}", v, i, list);

      return dateLabels.get(i).toString();
    });

This gives

XXXClient-0.js:408989 axis=org.pepstock.charba.client.configuration.CartesianTimeAxis@31c1
log_35_g$ @ XXXClient-0.js:408989
XXXClient-0.js:408989 v=Sep 18 i=7, list=[array=[{"value":1565733600000,"major":false},{"value":1566165600000,"major":false},{"value":1566597600000,"major":false},{"value":1567029600000,"major":false},{"value":1567461600000,"major":false},{"value":1567893600000,"major":false},{"value":1568325600000,"major":false},{"value":1568757600000,"major":false},{"value":1569189600000,"major":false},{"value":1569621600000,"major":false},{"value":1570053600000,"major":false}]]

Why is a double (v) printed as Sep 18?

stockiNail commented 5 years ago

@ak80 the "value" field passed to callback, is the value shown in the chart. Using CartesianTimeAxis, the value is the formatted time by TimeUnit which is used. And also the list of doubles is not correct. Let me work on that.

A workaround, waiting for fixing, even if not it's so elegant, could be the follwoing:

ticks.setCallback((axis, v, i, list) -> {
    LineDataset dataset = (LineDataset)axis.getChart().getData().getDatasets().get(0);
    DataPoint dataPoint = dataset.getDataPoints().get(i);
    Date date = dataPoint.getT();
    // apply your date formatting
    return FORMAT.format(date);
});
ak80 commented 5 years ago

@stockiNail This is awesome. Again you saved the day. Thank you so much. My colleague offered to bake you a cake - but I am not sure if it makes sense to send it from Germany. Can you think about another way to show our gratitude?

We will use the workaround!

stockiNail commented 5 years ago

@ak80 I apologize for this bug. Unfortunately it's always the same story about JavaScript vs Java, where in JS there isn't any type.... And my fault was and is do not figure out it for the tick callback interface! I'm gonna fix it even if it could be more complex than I think. If you agree, I want to fix the bug in the next release (we are waiting for new Chart.js official version).

And THANK YOUUU! Well, honestly I don't know what I can propose you... Let me think even if a cake is not a bad idea!!! ;) I'm very happy you are using Charba and this is already a gift for me!

ak80 commented 5 years ago

@stockiNail No apologies please, it is not your fault JavaScript has no strong type system... I figured something like this.

We are happy to use the date, as we need to transform it anyway to fit a user defined timezone! No rush to change anything.

Thank you again!

PS: We are happy too, to use charba. There will be people looking at the charts we are creating with it almost daily! Yay!

stockiNail commented 5 years ago

@ak80 Yesterday evening I thought a little how to address the topic in consistent way.

The current TickCallback is designed to manage xes where data are doubles. Therefore linear, log and radial ones. What is not able to manage are category (where the data are strings) and time (where data are dates)

Then I thought to specialized into different axes the the tick callaback (I'm still evaluating if do it by generics or not) and arguementlist could be:

For time axes, I'd like to maintain the same signature of Chart.js where the value is already a formatted date as string. For the list of values, it will be a List<TimeTickItem> where the class does not exist but it can map exactly what Chat.js is providing (see your log) and for performance point of view not additional object must be created.

The TimeTickItem will contains:

   double value;
   boolean major;

In this case, if you need to use your date formatter, you should ignore the value (the strign already formatted) but use the tick item of the list at index provided as argument.

Make sense for you?

stockiNail commented 5 years ago

@ak80 as above described, we have created specific tick callbacks for each axes (and then own tick object) where the type of ticks could be different.

The time tick callback is receiving the string representation already created by CHART.JS as value, based on TimeUnit defined for axis and the default format defined for that unit. It provides also a list of TimeTickItems which are mapping the CHART.JS object passed when the axis is a time one.

In your case, this could be the code:

ticks.setCallback((axis, v, i, list) -> {
    TimeTickItem item = list.get(i);
    Date date = item.getValue();
    // apply your date formatting
    return FORMAT.format(date);
});
stockiNail commented 5 years ago

Checking also AXIS callbacks consistency

stockiNail commented 5 years ago

Changed also the Axis build ticks callabacks in order to get the created ticks into onAfterBuildTicks method. The list of ticks can have different types depending on the type of the axis.

These are the 2 main added features:

ak80 commented 5 years ago

@stockiNail wow, great work, we will check it out in the next release! Thank you :)

stockiNail commented 5 years ago

@ak80 thank you very much.

Nevertheless I'm still not glad about time tick callback because I want to pass anyway the value as Date.

I think 90% of users needs the value to transform in tick and they should access always to ticks item list. Does not make sense.

Therefore I am gonna change the signature as following:

Date value, String label, int index, List ticks.

More details in next days.

stockiNail commented 5 years ago

@ak80 FYI changed the signature for TimeTickCallback ass following:

String onCallback(Axis axis, Date value, String label, int index, List<TimeTickItem> values);

Now the code could be:

ticks.setCallback((axis, v, l, i, list) -> {
    // apply your date formatting
    return FORMAT.format(v);
});

The other tick callback interfaces remaining with the same CHART.JS signature (without the label).

stockiNail commented 5 years ago

@ak80 FYI, I was ready to release the new version of Charba with new CHART.JS 2.9.1 but doing the tests I have seen that CHART.JS 2.9.1 is using a JS function not supported by IE/EDGE.

I open an issue and wait for fixing before releasing new version.

ak80 commented 5 years ago

@stockiNail thanks for the update. Cool that you caught the problem before the release, I hope it is an easy fix!

ak80 commented 4 years ago

@stockiNail I just now have updated our dependency and use the new Tick callback, it works perfectly. Thanks again, @stockiNail

stockiNail commented 4 years ago

VERY GOOD!! THANK YOU!