FIX: render multiline legend items without overlapping (#10532) (#10641)

* FIX: render multiline legend items without overlapping (#10532) Co-authored-by: Nirav Chavda <[email protected]> * CLN: Extract method to fix codeclimate line count Co-authored-by: Nirav Chavda <[email protected]> * CLN: Shift helper methods from class to module scope Co-authored-by: Nirav Chavda <[email protected]> * TST: Add test with fixtures Co-authored-by: kartik <[email protected]> * FIX: Fix test case for multiline label Co-authored-by: kartik <[email protected]> * 10532-ENH: Calculate legend item width for multiline labels Co-authored-by: Nirav Chavda <[email protected]> * 10532-TST: use spriteText and non-empty labels for test Co-authored-by: Nirav Chavda <[email protected]> * 10532-FIX: failing test case due to legendItem.text being undefined Co-authored-by: Nirav Chavda <[email protected]> * 10532-FIX: Update compression size Co-authored-by: kartik <[email protected]> Co-authored-by: Nirav Chavda <[email protected]>
chartjs · Sep 16, 2022 · 1253ced · 1253ced
1 parent d4e106c
commit 1253ced
Show file tree

Hide file tree

Showing 5 changed files with 138 additions and 8 deletions.
diff --git a/.size-limit.cjs b/.size-limit.cjs
@@ -34,7 +34,7 @@ module.exports = [
   },
   {
     path: 'dist/chart.js',
-    limit: '27.1 KB',
+    limit: '27.5 KB',
     import: '{ Decimation, Filler, Legend, SubTitle, Title, Tooltip }',
     running: false,
     modifyWebpackConfig

diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js
@@ -3,12 +3,20 @@ import Element from '../core/core.element';
 import layouts from '../core/core.layouts';
 import {addRoundedRectPath, drawPointLegend, renderText} from '../helpers/helpers.canvas';
 import {
-  callback as call, valueOrDefault, toFont,
-  toPadding, getRtlAdapter, overrideTextDirection, restoreTextDirection,
-  clipArea, unclipArea, _isBetween
+  _isBetween,
+  callback as call,
+  clipArea,
+  getRtlAdapter,
+  overrideTextDirection,
+  restoreTextDirection,
+  toFont,
+  toPadding,
+  unclipArea,
+  valueOrDefault,
 } from '../helpers/index';
-import {_toLeftRightCenter, _alignStartEnd, _textX} from '../helpers/helpers.extras';
+import {_alignStartEnd, _textX, _toLeftRightCenter} from '../helpers/helpers.extras';
 import {toTRBLCorners} from '../helpers/helpers.options';
+
 /**
  * @typedef { import("../../types").ChartEvent } ChartEvent
  */
@@ -139,7 +147,7 @@ export class Legend extends Element {
       height = this._fitRows(titleHeight, fontSize, boxWidth, itemHeight) + 10;
     } else {
       height = this.maxHeight; // fill all the height
-      width = this._fitCols(titleHeight, fontSize, boxWidth, itemHeight) + 10;
+      width = this._fitCols(titleHeight, labelFont, boxWidth, itemHeight) + 10;
     }
 
     this.width = Math.min(width, options.maxWidth || this.maxWidth);
@@ -180,7 +188,7 @@ export class Legend extends Element {
     return totalHeight;
   }
 
-  _fitCols(titleHeight, fontSize, boxWidth, itemHeight) {
+  _fitCols(titleHeight, labelFont, boxWidth, _itemHeight) {
     const {ctx, maxHeight, options: {labels: {padding}}} = this;
     const hitboxes = this.legendHitBoxes = [];
     const columnSizes = this.columnSizes = [];
@@ -194,7 +202,7 @@ export class Legend extends Element {
     let col = 0;
 
     this.legendItems.forEach((legendItem, i) => {
-      const itemWidth = boxWidth + (fontSize / 2) + ctx.measureText(legendItem.text).width;
+      const {itemWidth, itemHeight} = calculateItemSize(boxWidth, labelFont, ctx, legendItem, _itemHeight);
 
       // If too tall, go to new column
       if (i > 0 && currentColHeight + itemHeight + 2 * padding > heightLimit) {
@@ -418,6 +426,9 @@ export class Legend extends Element {
 
       if (isHorizontal) {
         cursor.x += width + padding;
+      } else if (typeof legendItem.text !== 'string') {
+        const fontLineHeight = labelFont.lineHeight;
+        cursor.y += calculateLegendItemHeight(legendItem, fontLineHeight);
       } else {
         cursor.y += lineHeight;
       }
@@ -541,6 +552,33 @@ export class Legend extends Element {
   }
 }
 
+function calculateItemSize(boxWidth, labelFont, ctx, legendItem, _itemHeight) {
+  const itemWidth = calculateItemWidth(legendItem, boxWidth, labelFont, ctx);
+  const itemHeight = calculateItemHeight(_itemHeight, legendItem, labelFont.lineHeight);
+  return {itemWidth, itemHeight};
+}
+
+function calculateItemWidth(legendItem, boxWidth, labelFont, ctx) {
+  let legendItemText = legendItem.text;
+  if (legendItemText && typeof legendItemText !== 'string') {
+    legendItemText = legendItemText.reduce((a, b) => a.length > b.length ? a : b);
+  }
+  return boxWidth + (labelFont.size / 2) + ctx.measureText(legendItemText).width;
+}
+
+function calculateItemHeight(_itemHeight, legendItem, fontLineHeight) {
+  let itemHeight = _itemHeight;
+  if (typeof legendItem.text !== 'string') {
+    itemHeight = calculateLegendItemHeight(legendItem, fontLineHeight);
+  }
+  return itemHeight;
+}
+
+function calculateLegendItemHeight(legendItem, fontLineHeight) {
+  const labelHeight = legendItem.text ? legendItem.text.length + 0.5 : 0;
+  return fontLineHeight * labelHeight;
+}
+
 function isListened(type, opts) {
   if ((type === 'mousemove' || type === 'mouseout') && (opts.onHover || opts.onLeave)) {
     return true;

diff --git a/test/fixtures/plugin.legend/legend-doughnut-right-center-mulitiline-labels.json b/test/fixtures/plugin.legend/legend-doughnut-right-center-mulitiline-labels.json
@@ -0,0 +1,28 @@
+{
+	"config": {
+		"type": "doughnut",
+		"data": {
+			"labels": ["Example Label", ["I like these colors", "Red", "Green", "Blue", "Yellow"], "Example Label", "Example Label", "Example Label"],
+			"datasets": [{
+				"data": [10, 20, 30, 40, 50],
+				"backgroundColor": "#00ff00",
+				"borderWidth": 0
+			}]
+		},
+		"options": {
+			"plugins": {
+				"legend": {
+					"position": "right",
+					"align": "center"
+				}
+			}
+		}
+	},
+	"options": {
+    "spriteText": true,
+		"canvas": {
+			"height": 256,
+			"width": 512
+		}
+	}
+}
diff --git a/test/fixtures/plugin.legend/legend-doughnut-right-center-mulitiline-labels.png b/test/fixtures/plugin.legend/legend-doughnut-right-center-mulitiline-labels.png
diff --git a/test/specs/plugin.legend.tests.js b/test/specs/plugin.legend.tests.js
@@ -507,6 +507,70 @@ describe('Legend block tests', function() {
     });
   });
 
+  it('should draw legend with multiline labels', function() {
+    const chart = window.acquireChart({
+      type: 'doughnut',
+      data: {
+        labels: [
+          'ABCDE',
+          [
+            'ABCDE',
+            'ABCDE',
+          ],
+          [
+            'Some Text',
+            'Some Text',
+            'Some Text',
+          ],
+          'ABCDE',
+        ],
+        datasets: [
+          {
+            label: 'test',
+            data: [
+              73.42,
+              18.13,
+              7.54,
+              0.9,
+              0.0025,
+              1.8e-5,
+            ],
+            backgroundColor: [
+              '#0078C2',
+              '#56CAF5',
+              '#B1E3F9',
+              '#FBBC8D',
+              '#F6A3BE',
+              '#4EC2C1',
+            ],
+          },
+        ],
+      },
+      options: {
+        plugins: {
+          legend: {
+            labels: {
+              usePointStyle: true,
+              pointStyle: 'rect',
+            },
+            position: 'right',
+            align: 'center',
+            maxWidth: 860,
+          },
+        },
+        aspectRatio: 3,
+      },
+    });
+
+    // Check some basic assertions about the test setup
+    expect(chart.legend.legendHitBoxes.length).toBe(4);
+
+    // Check whether any legend items reach outside the established bounds
+    chart.legend.legendHitBoxes.forEach(function(item) {
+      expect(item.left + item.width).toBeLessThanOrEqual(chart.width);
+    });
+  });
+
   it('should draw items with a custom boxHeight', function() {
     var chart = window.acquireChart(
       {