diff --git a/samples/unit-tests/series-funnel/funnel/demo.js b/samples/unit-tests/series-funnel/funnel/demo.js index 6f989fc8f0a..0685b766d06 100644 --- a/samples/unit-tests/series-funnel/funnel/demo.js +++ b/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)` + ); }); diff --git a/ts/Extensions/OverlappingDataLabels.ts b/ts/Extensions/OverlappingDataLabels.ts index 89ad0c122bc..a251dbd820e 100644 --- a/ts/Extensions/OverlappingDataLabels.ts +++ b/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 }); diff --git a/ts/Series/Funnel/FunnelSeries.ts b/ts/Series/Funnel/FunnelSeries.ts index 410d0769635..abc16f4585f 100644 --- a/ts/Series/Funnel/FunnelSeries.ts +++ b/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,