Charts: Problem with setDrilldownCallback() / DrilldownCallback.DrilldownDetails() control flow

[enver-haase]

Description

Clicking the ITEM NAME UNDERNEATH THE BAR:

Interestingly, the number of call-backs is exactly the number of call-backs expected, but they are item Detsinyi Chrstian - series Change --- good! Item Projekt 1 - series Detsinyi Chrstian Change --- WTF??? Item Projekt 1 - series Projekt 1 Detsinyi Chrstian Change --- WTF!???

so the call-back event data somehow depends on events handled before the current one.

Expected outcome

If you click the item description, e.g. 'Detsinyi Chrstian', then I expect three callbacks:

item Detsinyi Chrstian - series Change item Detsinyi Chrstian - series Run item Detsinyi Chrstian - series Test

-- as if you were clicking all the three defined series one after another. Customer claims this is also what happened in Charts for Vaadin 8.

Minimal reproducible example

package com.example.application.views.test;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;

import com.vaadin.flow.component.notification.Notification;
import org.apache.commons.lang3.RandomStringUtils;

import com.example.application.views.MainLayout;
import com.vaadin.flow.component.charts.Chart;
import com.vaadin.flow.component.charts.model.AxisType;
import com.vaadin.flow.component.charts.model.ChartType;
import com.vaadin.flow.component.charts.model.Configuration;
import com.vaadin.flow.component.charts.model.Cursor;
import com.vaadin.flow.component.charts.model.DataSeries;
import com.vaadin.flow.component.charts.model.DataSeriesItem;
import com.vaadin.flow.component.charts.model.DrilldownCallback;
import com.vaadin.flow.component.charts.model.PlotOptionsColumn;
import com.vaadin.flow.component.charts.model.PlotOptionsSeries;
import com.vaadin.flow.component.charts.model.Series;
import com.vaadin.flow.component.charts.model.StackLabels;
import com.vaadin.flow.component.charts.model.Stacking;
import com.vaadin.flow.component.charts.model.XAxis;
import com.vaadin.flow.component.charts.model.YAxis;
import com.vaadin.flow.component.charts.model.style.SolidColor;
import com.vaadin.flow.component.combobox.ComboBox;
import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.orderedlayout.HorizontalLayout;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;
import com.vaadin.flow.router.PageTitle;
import com.vaadin.flow.router.Route;

@PageTitle("KPITest")
@Route(value = "", layout = MainLayout.class)
public class KPITest extends Div {

    public static class KPIChangeRunDto implements Serializable {
        private static final long serialVersionUID = 6032642107534296188L;
        private final String categorie;
        private final Double sum;
        public KPIChangeRunDto(String categorie, Double sum) {
            super();
            this.categorie = categorie;
            this.sum = sum;
        }
        public String getCategorie() {
            return categorie;
        }
        public Double getSum() {
            return sum;
        }
    }
    public static final String NAME = "KPITest";
    private final VerticalLayout container;
    private final HorizontalLayout filter;
    private final ComboBox<java.lang.String> cbKindOfReport;
    public KPITest() {
        container = new VerticalLayout();
        container.setSizeFull();
        add(container);
        filter = new HorizontalLayout();
        cbKindOfReport = new ComboBox<>();
        cbKindOfReport.setItems("percent", "absolute", "absolutestacked");
        cbKindOfReport.setValue("percent");
        cbKindOfReport.addValueChangeListener(listener -> drawBarChart());
        drawBarChart();
    }

    void drawBarChart() {
        Chart chart = new Chart(ChartType.COLUMN);
        Configuration conf = chart.getConfiguration();
        conf.setTitle("Change/Run");
        YAxis yAxis = new YAxis();
        yAxis.setMin(0);
        StackLabels sLabels = new StackLabels(true);
        yAxis.setStackLabels(sLabels);
        conf.addyAxis(yAxis);
        PlotOptionsColumn plotOptions = new PlotOptionsColumn();
        SolidColor[] color = {new SolidColor(54, 77, 110), new SolidColor(192, 207, 225)};
        plotOptions.setColors(color);
        switch (cbKindOfReport.getValue()) {
            case "absolute":
                plotOptions.setStacking(Stacking.NONE);
                break;
            case "absolutestacked":
                plotOptions.setStacking(Stacking.NORMAL);
                break;
            default:
                plotOptions.setStacking(Stacking.PERCENT);
                break;
        }

        plotOptions.setAllowPointSelect(true);
        plotOptions.setCursor(Cursor.POINTER);
        plotOptions.setShowInLegend(true);
        conf.setPlotOptions(plotOptions);

        DataSeries changeSeries = new DataSeries("Change");
        changeSeries.addItemWithDrilldown(new DataSeriesItem("Detsinyi Christian", 40.1));
        changeSeries.addItemWithDrilldown(new DataSeriesItem("Antoni Roland", 30.1));
        changeSeries.addItemWithDrilldown(new DataSeriesItem("Columbo, Frank", 70.1));

        DataSeries runSeries = new DataSeries("Run");
        runSeries.addItemWithDrilldown(new DataSeriesItem("Detsinyi Christian", 40.1));
        runSeries.addItemWithDrilldown(new DataSeriesItem("Antoni Roland", 20.1));
        runSeries.addItemWithDrilldown(new DataSeriesItem("Columbo, Frank", 60.1));

        DataSeries testSeries = new DataSeries("Test");
        testSeries.addItemWithDrilldown(new DataSeriesItem("Detsinyi Christian", 50.1));
        testSeries.addItemWithDrilldown(new DataSeriesItem("Antoni Roland", 40.1));
        testSeries.addItemWithDrilldown(new DataSeriesItem("Columbo, Frank", 30.1));

        XAxis xAxis = new XAxis();
        conf.addxAxis(xAxis);
        xAxis.setType(AxisType.CATEGORY);

        PlotOptionsSeries poChangeSeries = new PlotOptionsSeries();
        poChangeSeries.setColor(new SolidColor(192, 207, 225));
        changeSeries.setPlotOptions(poChangeSeries);

        PlotOptionsSeries poRunSeries = new PlotOptionsSeries();
        poRunSeries.setColor(new SolidColor(111, 141, 185));
        runSeries.setPlotOptions(poRunSeries);

        PlotOptionsSeries poTestSeries = new PlotOptionsSeries();
        poTestSeries.setColor(new SolidColor(0, 0, 255));
        testSeries.setPlotOptions(poTestSeries);

        conf.addSeries(changeSeries);
        conf.addSeries(runSeries);
        conf.addSeries(testSeries);

        conf.getyAxis().setTitle("Test");

        chart.setDrilldownCallback(KPITest::getPointDrilldown);

        chart.setConfiguration(conf);
        chart.drawChart();

        container.removeAll();
        container.add(filter, chart);
    }

    private static Series getPointDrilldown(DrilldownCallback.DrilldownDetails event) {
        List<KPIChangeRunDto> listChangeRun = new ArrayList<>();
        Notification.show("Event: Item is " + event.getItem().getName() + ", item index is " + event.getItemIndex() + ", Series is " + event.getSeries().getName());

        System.err.println(event.getItem().getName() + " ## " + event.getSeries().getName());
        listChangeRun.add(new KPIChangeRunDto("Projekt1", 24.5));
        listChangeRun.add(new KPIChangeRunDto("Projekt2", 42.0));
        listChangeRun.add(new KPIChangeRunDto("Projekt3", 14.5));
        listChangeRun.add(new KPIChangeRunDto("Projekt4", 24.5));

        ArrayList<String> alCategories = new ArrayList<>();
        ArrayList<Number> alData = new ArrayList<>();

        for (KPIChangeRunDto kpiChangeRun : listChangeRun) {
            alCategories.add(kpiChangeRun.getCategorie());
            alData.add(kpiChangeRun.getSum());
        }

        DataSeries drill = new DataSeries(event.getItem().getName() + " " + event.getSeries().getName());
        String[] categories = new String[alCategories.size()];
        categories = alCategories.toArray(categories);
        Number[] data = new Number[alData.size()];
        data = alData.toArray(data);

        drill.setData(categories, data);
        drill.setId(RandomStringUtils.randomAlphabetic(15));
        return drill;
    }
}

Steps to reproduce

If you click the item description, e.g. 'Detsinyi Chrstian', then I expect three callbacks:

item Detsinyi Chrstian - series Change item Detsinyi Chrstian - series Run item Detsinyi Chrstian - series Test

-- as if you were clicking all the three defined series one after another. Customer claims this is also what happened in Charts for Vaadin 8.

Interestingly, the number of call-backs is exactly the number of call-backs expected, but they are item Detsinyi Chrstian - series Change --- good! Item Projekt 1 - series Detsinyi Chrstian Change --- WTF??? Item Projekt 1 - series Projekt 1 Detsinyi Chrstian Change --- WTF!???

You can experimentally make the call-back function in the example code 'static' and thus prove that only copies are created, no old data manipulated. Somehow Flow messes up its internal data structures with the returned Series however.

Once things have gone wrong, even clicking a value no longer creates an expected good call-back, but some Frankenstein something that has to do with the handling of previous events.

Environment

Vaadin version(s): 23.2.1 OS: macOS latest

Browsers

Chrome, Issue is not browser related

[enver-haase]

Related: #3741

[TatuLund]

It is not clear what should happen when label is clicked as there is three drill downs associated.

Just noting, the drill down works correctly by clicking the data series items themselves.

I can propose two alternative workarounds.

The first one is to disable drill down by label clicking.

@CssImport(value = "./vaadin-chart.css", themeFor = "vaadin-chart")

vaadin-chart.css

.highcharts-drilldown-axis-label {
    pointer-events: none;
    text-decoration: unset !important;  
}

This will guide the user to use the functionality that works.

Alternatively you can filter out event on server side using

private static Series getPointDrilldown(
            DrilldownCallback.DrilldownDetails event) {
        if (event.getItem().getName().startsWith("Projekt")) {
            return null;
        }
        ...

[mhosio]

The root cause of this problem is that the current drill down callback implementation assumes only one drill down event per user click. However, in the case of stacked columns when you click the category label under the column, you will actually get a drill down event for all the DataSeriesItems belonging to the category. This confuses the current implementation and it acts as the user would be drilling down to deeper levels of items (nested drill down). The potential fix would be that https://github.com/vaadin/flow-components/blob/643c992de9c3afa649e247ec277b382364a1d810/vaadin-charts-flow-parent/vaadin-charts-flow/src/main/java/com/vaadin/flow/component/charts/Chart.java#L661 will be refactored to handle multiple drilled down series at the same time (single stack is not sufficient). Also it should be expected that when the user drills up in this type of a scenario, there will be multiple drill up event callbacks as well respectively for each drilled down datapoint.

[enver-haase]

"one drill down event / user click"

The "/" reads "per" here.

[DiegoCardoso]

From what I can see, the refactoring of the current implementation to support multiple drilled-down series can become a bit complex.

There isn't an easy way to tell from Highcharts drill-down events, which ones belong to the same action (when the user clicks on the category label, for instance).

There's a way, however, to tell if a click is made from the category by checking the points (which returns false or a list of points) and the category (which returns undefined or the category index) from the Highcharts drilldown event.

Using this information, the right Series could be given to the drill-down callback. That would "solve" the issue presented on the logs on this issue description, but it would still not solve it completely because if the drildown callback returns series for every call, then it wouldn't be able to add the 2nd+ series.

Let me try to explain the reason for that:

1. The user clicks on a category with 3 series
2. 3 events are fired
3. The first event is received by DrillCallbackHandler.onDrilldown
   1. The series is correctly retrieved and passed to the drill-down callback
   2. The callback returns a new series, which is added as a drill-down using the series/point passed by the original event and is also added to the internal drill-down stack
   3. The new series is shown on the chart
4. The second event arrives
   1. The series can be correctly retrieved thanks to the points information from the event and passed to the callback
   2. The callback returns a new series, which is added to the charts, but since the "parent" series is no longer on the chart, it fails

That happens because the Highcharts' method used to add the drilldown only accepts one series. There's another undocumented but public method that will not draw the new series, until Charts#applyDrilldown() is called. This could be used instead to support multiple drilldown series, but again, it falls into the issue of finding out which events belong to the same action.

[TatuLund]

@DiegoCardoso , yes. That describes in detail what I was also thinking. This is why I proposed as first alternative to disable drilling to multiple series at the same time with CSS, as it feels to me how it should work.

[enver-haase]

@TatuLund looks like the workaround https://github.com/vaadin/flow-components/issues/3742#issuecomment-1251012214 no longer works for com.vaadin:vaadin-charts-flow:jar:23.3.19 . @DiegoCardoso any profound solution on the horizon?

[DiegoCardoso]

Which workaround stopped working? I can't remember any changes in Charts that would make the workaround no longer work.

As for a solution to this problem, I believe there are no plans to address it now. Drilldown in stacked series doesn't seem to be a well-supported use case in Highcharts and from my research, it would require us to use some methods that are not exactly public, and even so, I am not sure how the integration on the server side would work.