bugfix/17589-multiple-datalabels-update

[hubertkozik]

Fixed #17589, update of multiple data labels didn't work in specific conditions.

[highsoft-bot]

File size comparison

Sizes for compiled+gzipped (bold) and compiled files.

	master	candidate	difference
highcharts.js	98.7 kB 280.2 kB	98.8 kB 280.3 kB	81 B 58 B
highstock.js	130.9 kB 379.2 kB	131.0 kB 379.3 kB	94 B 67 B
highmaps.js	123.7 kB 356.9 kB	123.7 kB 357.0 kB	63 B 61 B
highcharts-gantt.js	133.5 kB 385.2 kB	133.5 kB 385.3 kB	55 B 62 B

[highsoft-bot]

Visual test results - No difference found

[TorsteinHonsi]

Looking at the changes, I am wondering if we can simplify this a bit.

We know that point.dataLabels should always be an array, and that point.dataLabel should always be the same as point.dataLabels[0]. If this is not the case, it should be considered a bug. (Or am I wrong? Check DataLabel.ts, line 563).

So say the assumption above is true, or we make it true. In that case, all the juggling between point.dataLabel and point.dataLabels and checking i === 0 can be replaced with a structure like this:

for (let dataLabel of point.dataLabels) {
    // Do stuff to each data label. Destroy if that's what we want, but remember
    // to splice it out from `point.dataLabels`
}

// Write back to `point.dataLabel`. `point.dataLabels[0]` can now be either
// an SVGElement or undefined, both of which are correct.
point.dataLabel = point.dataLabels[0];

[hubertkozik]

@TorsteinHonsi It seems to me, that it won't be so simple. In the place, where I have added a fix we are already in a loop through data labels for each point. In my opinion, to change the logic (to use point.dataLabel = point.dataLabels[0];) we would need to refactor this entire function (drawDataLabels). Unfortunately, the current code looks to me a bit like spaghetti...