moberwasserlechner / vaadin-chartjs

Vaadin 8 wrapper for ChartJS v2.x library
MIT License
56 stars 35 forks source link

DonutChart: Tooltips of inner layers are wrong #58

Open sebastiansto opened 7 years ago

sebastiansto commented 7 years ago

Hi there,

When you have multiple layers in a DonutChart, the tooltips of the inner layers are not correct. The tooltip is always "stuck" with the outer tooltip.

It would be nice, if the correct data of the layer is used (like in the ClickListener):

dataset.getDataLabels().get(dataIdx)
dataset.getData().get(dataIdx).intValue()

BR

moberwasserlechner commented 7 years ago

Hi,

this is most propably not a problem of this addon but of the underlying chartjs javascript library.

It may be fixed in the latest js release but I'm not sure. So please try to reproduce in pure javascript and in case it's really a js bug report it in ChartJS repo.

thanks, Michael

moberwasserlechner commented 7 years ago

Hi @sebastiansto,

could you share your code. It would help me a lot hunting your problem down.

BR

sebastiansto commented 7 years ago

Hi Michael,

Thank you for the fast response. I didn't had the time yet to check the JS.

My code looks like this:

DonutChartConfig chartConfiguration = new DonutChartConfig();
chartConfiguration.data().labelsAsList(ApplicationType.names());

PieDataset applicationTypeDataset = new PieDataset().label("Application type").randomBackgroundColors(true);
for (ApplicationNodeStatisticDto application : applicationStats) {
    applicationTypeDataset.addLabeledData(application.getApplicationType().name(), 1.0);
}
chartConfiguration.data().addDataset(applicationTypeDataset);

PieDataset applicationIdDataset = new PieDataset().label("Application id").randomBackgroundColors(true);
for (ApplicationNodeStatisticDto application : applicationStats) {
    applicationIdDataset.addLabeledData(application.getApplicationId(), 1.0);
}
chartConfiguration.data().addDataset(applicationIdDataset);
chartConfiguration.data().and().options().responsive(true).title().display(true).text("Application node count").and().animation().animateScale(true)
    .animateRotate(true).and().done();

ChartJs chart = new ChartJs(chartConfiguration);
chart.addClickListener((dataSetIdx, dataIdx) -> {
    PieDataset dataset = (PieDataset) chartConfiguration.data().getDatasets().get(dataSetIdx);
    NotificationUtils.showTrayNotification(dataset.getDataLabels().get(dataIdx), dataset.getData().get(dataIdx).intValue() + " nodes");
});

BR

moberwasserlechner commented 7 years ago

Is this your real config?

You have 2 datasets and only add values equals 1.0.

//...
applicationTypeDataset.addLabeledData(application.getApplicationType().name(), 1.0);
//...
applicationIdDataset.addLabeledData(application.getApplicationId(), 1.0);

Please recheck your config.

moberwasserlechner commented 7 years ago

The tooltips in this example work. http://vaadin-demos.qqjtxeeuih.eu-central-1.elasticbeanstalk.com:5600/#!multi-donut-chart

Unfortuantly I'm not sure want you mean by "stuck"

The tooltip is always "stuck" with the outer tooltip.

sebastiansto commented 7 years ago

You have 2 datasets and only add values equals 1.0.

It's a node count, so yes, it's correct.

application.getApplicationType().name() would be something like this: TYPE_A TYPE_B ...

applicationIdDataset.addLabeledData(application.getApplicationId(), 1.0); would be something like this: TYPE_A_APP01 TYPE_A_APP02 TYPE_B_APP01 ...

The combinations like TYPE_A and TYPE_A_APP01 can exist multiple times in the list (for every node there is).

When I now go over the outer layer, the tooltip is fine: TYPE_A: 9 TYPE_B: 1

But when I move my mouse over the inner layer e.g. TYPE_A_APP01, I still get the outer layers tooltip: TYPE_A: 9 Here I would expect something like this, as a sub category: TYPE_A_APP01: 3

I think the problem here is, that I only add the outer layer to the label list (I don't want to see every subcategory there). But then it doesn't generate the inner tooltips. Would it be possible, to define custom tooltips for inner layers?

BR

P.S.: The Clicklistener shows always the correct data.

moberwasserlechner commented 7 years ago

Ok thx for the clearification. This is indeed weared.

Could you please enable the javascript logging

chart.setJsLoggingEnabled(true);

and paste the json config output here.

sebastiansto commented 7 years ago

Sure:

chartjs-connector.js:18 chartjs: accessing onStateChange the 1. time
chartjs-connector.js:22 chartjs: create canvas
chartjs-connector.js:46 chartjs: init
chartjs-connector.js:50 chartjs: configuration is
 {
  "type": "doughnut",
  "data": {
    "labels": [
      "TYPE_A",
      "TYPE_B",
      "TYPE_C"
    ],
    "datasets": [
      {
        "data": [
          2,
          1
        ],
        "label": "Application type",
        "backgroundColor": [
          "rgba(117,183,70,0.7)",
          "rgba(245,211,83,0.7)"
        ]
      },
      {
        "data": [
          2,
          1
        ],
        "label": "Application id",
        "backgroundColor": [
          "rgba(11,145,46,0.7)",
          "rgba(226,242,119,0.7)"
        ]
      }
    ]
  },
  "options": {
    "responsive": true,
    "title": {
      "display": true,
      "text": "Application node count"
    },
    "animation": {
      "animateRotate": true,
      "animateScale": true
    }
  }
}
moberwasserlechner commented 7 years ago

I think I now understand what's the difference between your need and the demo example.

The data in the inner "ring" has nothing in common with the outer ring.

I searched for a chartjs example but did not find one.

So please create a plunkr in javascript with chartjs for your need and post the link here. Right now I can not do any further research but if this is possible with javascript I will include it in the next release.

sebastiansto commented 7 years ago

Ok, thank you for your help so far. I'm pretty busy right now, but I'll do it as soon I find some time.

BR

sebastiansto commented 7 years ago

Hi, sorry for the long wait.

I played around a little and this is what would work for me: https://jsfiddle.net/dagpcx9m/4/

var ctx = document.getElementById("myChart").getContext('2d');
var myChart = new Chart(ctx, {
  "type": "doughnut",
  "data": {
    "labels": [
      "TYPE_A",
      "TYPE_B",
      "TYPE_C"
    ],
    "datasets": [
      {
        "data": [
          3,
          2
        ],
        "label": "Application type",
        "backgroundColor": [
          "rgba(117,183,70,0.7)",
          "rgba(245,211,83,0.7)"
        ],
        "labels": [
          "TYPE_A",
          "TYPE_B",
          "TYPE_C"
        ]
      },
      {
        "data": [
          2,
          1,
          2
        ],
        "label": "Application id",
        "backgroundColor": [
          "rgba(11,145,46,0.7)",
          "rgba(226,242,119,0.7)",
          "rgba(25,21,83,0.7)"
        ],
        "labels": [
          "TYPE_A_APP01",
          "TYPE_A_APP02",
          "TYPE_B_APP01"
        ]
      }
    ]
  },
  "options": {
    "responsive": true,
    "title": {
      "display": true,
      "text": "Application node count"
    },
    "animation": {
      "animateRotate": true,
      "animateScale": true
    },
    "tooltips": {
       "callbacks": {
          "label": function(tooltipItem, chart) {
             var label = chart.datasets[tooltipItem.datasetIndex].labels[tooltipItem.index];
             var value = chart.datasets[tooltipItem.datasetIndex].data[tooltipItem.index];
             return label + ': ' + value + ' nodes';
          }
       }
    }
  }
});

I added labels into the dataset and used the callback method for label.

BR

moberwasserlechner commented 7 years ago

Hi Sebastian,

thanks for taking the time to try this.

I feared that a JS callback function is the only way to realize your request/question.

I will try to find a solution but can not give you a specific timeline. I hope to get it done in November.

BR

sebastiansto commented 7 years ago

Hi Michael,

Thank you for taking this on.

On my side this isn't an urgent matter. So don't worry about the time.

BR

sebastiansto commented 6 years ago

Hi Michael,

Are there any plans yet regarding the JS callback function?

BR