bugfix/17545-funnel-data-labels-inside

samples/unit-tests/series-funnel/funnel/demo.js

@@ -236,8 +236,8 @@ QUnit.test('Funnel dataLabels', function (assert) {
     const data = [
         ['Website visits', 5654],
         ['Downloads', 4064],
-        ['Requested price list', 1987],
-        ['Invoice sent', 1976],
+        ['Requested price list', 198],
+        ['Invoice sent', 197],
         ['Finalized', 4201]
     ];
 
@@ -277,6 +277,12 @@ QUnit.test('Funnel dataLabels', function (assert) {
         'DataLabels centered horizontally inside the funnel (#10036)'
     );
 
+    assert.deepEqual(
+        series.points.map(p => p.dataLabel.opacity),
+        [1, 1, 1, 0, 1],
+        'One of the data labels should be hidden by overlap detection'
+    );
+
     series.update({
         dataLabels: {
             align: 'left'
@@ -399,15 +405,16 @@ QUnit.test('Funnel dataLabels', function (assert) {
         }
     });
 
+    dataLabel = chart.series[0].points[4].dataLabel;
+    const prevDataLabelPos = dataLabel.x;
+
     Highcharts.fireEvent(chart.series[0].points[0].legendItem.group.element, 'click');
     Highcharts.fireEvent(chart.series[0].points[0].legendItem.group.element, 'click');
     Highcharts.fireEvent(chart.series[0].points[0].legendItem.group.element, 'click');
 
-    dataLabel = chart.series[0].points[1].dataLabel;
-
-    assert.notEqual(
+    assert.equal(
         dataLabel.x,
-        dataLabel.alignAttr.x,
+        prevDataLabelPos,
         'DataLabels with allowOverlap set to false should be positioned correctly after point hide (#12350)'
     );
 
@@ -425,4 +432,48 @@ QUnit.test('Funnel dataLabels', function (assert) {
     chart.series[0].setData(data, true, false, false);
 
     assert.ok(true, '#16176: No error should occur');
+
+    chart.update({
+        plotOptions: {
+            series: {
+                dataLabels: {
+                    inside: true,
+                    allowOverlap: true,
+                    rotation: 0
+                }
+            }
+        }
+    });
+
+    series.update({
+        dataLabels: {
+            align: 'center'
+        }
+    });
+
+    dataLabel = chart.series[0].points[1].dataLabel;
+
+    const insideDataLabelPos = dataLabel.x;
+
+    chart.update({
+        plotOptions: {
+            series: {
+                dataLabels: {
+                    inside: false
+                }
+            }
+        }
+    });
+
+    const legendItem = chart.series[0].points[2].legendItem.group.element;
+
+    Highcharts.fireEvent(legendItem, 'mouseover');
+    Highcharts.fireEvent(legendItem, 'click');
+
+    assert.notEqual(
+        dataLabel.x,
+        insideDataLabelPos,
+        `DataLabel should be positioned outside the series after legend click
+        when inside is false. (#17545)`
+    );
 });

ts/Extensions/OverlappingDataLabels.ts

@@ -263,7 +263,7 @@ function hideOrShow(label: SVGElement, chart: Chart): boolean {
 
             // Make sure the label is completely hidden to avoid catching clicks
             // (#4362)
-            if (label.alignAttr && label.placed) { // data labels
+            if (label.alignAttr && label.placed) { // Data labels
                 label[
                     newOpacity ? 'removeClass' : 'addClass'
                 ]('highcharts-data-label-hidden');
@@ -278,14 +278,13 @@ function hideOrShow(label: SVGElement, chart: Chart): boolean {
                 isLabelAffected = true;
 
                 // Animate or set the opacity
-                label.alignAttr.opacity = newOpacity;
                 label[label.isOld ? 'animate' : 'attr'](
-                    label.alignAttr,
-                    null as any,
+                    { opacity: newOpacity },
+                    void 0,
                     complete
                 );
                 fireEvent(chart, 'afterHideOverlappingLabel');
-            } else { // other labels, tick labels
+            } else { // Other labels, tick labels
                 label.attr({
                     opacity: newOpacity
                 });

ts/Series/Funnel/FunnelSeries.ts

@@ -158,19 +158,21 @@ class FunnelSeries extends PieSeries {
         const series = point.series,
             reversed = series.options.reversed,
             dlBox = point.dlBox || point.shapeArgs,
-            align = options.align,
-            verticalAlign = options.verticalAlign,
+            { align, padding = 0, verticalAlign } = options,
             inside =
                 ((series.options || {}).dataLabels || {}).inside,
             centerY = series.center[1],
+            plotY = point.plotY || 0,
             pointPlotY = (
                 reversed ?
-                    2 * centerY - (point.plotY as any) :
-                    point.plotY
+                    2 * centerY - plotY :
+                    plotY
             ),
+            // #16176: Only SVGLabel has height set
+            dataLabelHeight = dataLabel.height ?? dataLabel.getBBox().height,
             widthAtLabel = series.getWidthAt(
-                (pointPlotY as any) - dlBox.height / 2 +
-                (dataLabel as any).height
+                pointPlotY - dlBox.height / 2 +
+                dataLabelHeight
             ),
             offset = verticalAlign === 'middle' ?
                 (dlBox.topWidth - dlBox.bottomWidth) / 4 :
@@ -179,17 +181,10 @@ class FunnelSeries extends PieSeries {
         let y = dlBox.y,
             x = dlBox.x;
 
-        // #16176: Only SVGLabel has height set
-        const dataLabelHeight = pick(
-            dataLabel.height,
-            dataLabel.getBBox().height
-        );
-
         if (verticalAlign === 'middle') {
             y = dlBox.y - dlBox.height / 2 + dataLabelHeight / 2;
         } else if (verticalAlign === 'top') {
-            y = dlBox.y - dlBox.height + dataLabelHeight +
-                (options.padding || 0);
+            y = dlBox.y - dlBox.height + dataLabelHeight + padding;
         }
 
         if (
@@ -198,9 +193,9 @@ class FunnelSeries extends PieSeries {
             verticalAlign === 'middle'
         ) {
             if (align === 'right') {
-                x = dlBox.x - (options.padding as any) + offset;
+                x = dlBox.x - padding + offset;
             } else if (align === 'left') {
-                x = dlBox.x + (options.padding as any) - offset;
+                x = dlBox.x + padding - offset;
             }
         }
 
@@ -212,9 +207,15 @@ class FunnelSeries extends PieSeries {
         };
 
         options.verticalAlign = 'bottom';
+        if (inside) {
+            // If the distance were positive (as default), the overlapping
+            // labels logic would skip these labels and they would be allowed
+            // to overlap.
+            options.distance = void 0;
+        }
 
         // Call the parent method
-        if (!inside || point.visible) {
+        if (inside && point.visible) {
             baseAlignDataLabel.call(
                 series,
                 point,