Refactor radialLinear scale and renderText helper (#9276)

* Refactor radialLinear scale and renderText helper
* Undo the big move to make review possible

.codeclimate.yml

@@ -8,9 +8,12 @@ plugins:
   fixme:
     enabled: true
 checks:
+  argument-count:
+    config:
+      threshold: 5
   method-complexity:
     config:
-      threshold: 6
+      threshold: 7
 exclude_patterns:
   - "dist/"
   - "docs/"

src/helpers/helpers.canvas.js

@@ -310,28 +310,8 @@ export function renderText(ctx, text, x, y, font, opts = {}) {
   let i, line;
 
   ctx.save();
-
-  if (opts.translation) {
-    ctx.translate(opts.translation[0], opts.translation[1]);
-  }
-
-  if (!isNullOrUndef(opts.rotation)) {
-    ctx.rotate(opts.rotation);
-  }
-
   ctx.font = font.string;
-
-  if (opts.color) {
-    ctx.fillStyle = opts.color;
-  }
-
-  if (opts.textAlign) {
-    ctx.textAlign = opts.textAlign;
-  }
-
-  if (opts.textBaseline) {
-    ctx.textBaseline = opts.textBaseline;
-  }
+  setRenderOpts(ctx, opts);
 
   for (i = 0; i < lines.length; ++i) {
     line = lines[i];
@@ -349,35 +329,61 @@ export function renderText(ctx, text, x, y, font, opts = {}) {
     }
 
     ctx.fillText(line, x, y, opts.maxWidth);
+    decorateText(ctx, x, y, line, opts);
 
-    if (opts.strikethrough || opts.underline) {
-      /**
-			 * Now that IE11 support has been dropped, we can use more
-			 * of the TextMetrics object. The actual bounding boxes
-			 * are unflagged in Chrome, Firefox, Edge, and Safari so they
-			 * can be safely used.
-			 * See https://developer.mozilla.org/en-US/docs/Web/API/TextMetrics#Browser_compatibility
-			 */
-      const metrics = ctx.measureText(line);
-      const left = x - metrics.actualBoundingBoxLeft;
-      const right = x + metrics.actualBoundingBoxRight;
-      const top = y - metrics.actualBoundingBoxAscent;
-      const bottom = y + metrics.actualBoundingBoxDescent;
-      const yDecoration = opts.strikethrough ? (top + bottom) / 2 : bottom;
-
-      ctx.strokeStyle = ctx.fillStyle;
-      ctx.beginPath();
-      ctx.lineWidth = opts.decorationWidth || 2;
-      ctx.moveTo(left, yDecoration);
-      ctx.lineTo(right, yDecoration);
-      ctx.stroke();
-    }
     y += font.lineHeight;
   }
 
   ctx.restore();
 }
 
+function setRenderOpts(ctx, opts) {
+  if (opts.translation) {
+    ctx.translate(opts.translation[0], opts.translation[1]);
+  }
+
+  if (!isNullOrUndef(opts.rotation)) {
+    ctx.rotate(opts.rotation);
+  }
+
+  if (opts.color) {
+    ctx.fillStyle = opts.color;
+  }
+
+  if (opts.textAlign) {
+    ctx.textAlign = opts.textAlign;
+  }
+
+  if (opts.textBaseline) {
+    ctx.textBaseline = opts.textBaseline;
+  }
+}
+
+function decorateText(ctx, x, y, line, opts) {
+  if (opts.strikethrough || opts.underline) {
+    /**
+     * Now that IE11 support has been dropped, we can use more
+     * of the TextMetrics object. The actual bounding boxes
+     * are unflagged in Chrome, Firefox, Edge, and Safari so they
+     * can be safely used.
+     * See https://developer.mozilla.org/en-US/docs/Web/API/TextMetrics#Browser_compatibility
+     */
+    const metrics = ctx.measureText(line);
+    const left = x - metrics.actualBoundingBoxLeft;
+    const right = x + metrics.actualBoundingBoxRight;
+    const top = y - metrics.actualBoundingBoxAscent;
+    const bottom = y + metrics.actualBoundingBoxDescent;
+    const yDecoration = opts.strikethrough ? (top + bottom) / 2 : bottom;
+
+    ctx.strokeStyle = ctx.fillStyle;
+    ctx.beginPath();
+    ctx.lineWidth = opts.decorationWidth || 2;
+    ctx.moveTo(left, yDecoration);
+    ctx.lineTo(right, yDecoration);
+    ctx.stroke();
+  }
+}
+
 /**
  * Add a path of a rectangle with rounded corners to the current sub-path
  * @param {CanvasRenderingContext2D} ctx Context

src/scales/scale.radialLinear.js

@@ -16,17 +16,11 @@ function getTickBackdropHeight(opts) {
   return 0;
 }
 
-function measureLabelSize(ctx, lineHeight, label) {
-  if (isArray(label)) {
-    return {
-      w: _longestText(ctx, ctx.font, label),
-      h: label.length * lineHeight
-    };
-  }
-
+function measureLabelSize(ctx, font, label) {
+  label = isArray(label) ? label : [label];
   return {
-    w: ctx.measureText(label).width,
-    h: lineHeight
+    w: _longestText(ctx, font.string, label),
+    h: label.length * font.lineHeight
   };
 }
 
@@ -89,22 +83,18 @@ function fitWithPointLabels(scale) {
     b: scale.height - scale.paddingTop
   };
   const furthestAngles = {};
-  let i, textSize, pointPosition;
-
   const labelSizes = [];
   const padding = [];
 
   const valueCount = scale.getLabels().length;
-  for (i = 0; i < valueCount; i++) {
+  for (let i = 0; i < valueCount; i++) {
     const opts = scale.options.pointLabels.setContext(scale.getContext(i));
     padding[i] = opts.padding;
-    pointPosition = scale.getPointPosition(i, scale.drawingArea + padding[i]);
+    const pointPosition = scale.getPointPosition(i, scale.drawingArea + padding[i]);
     const plFont = toFont(opts.font);
-    scale.ctx.font = plFont.string;
-    textSize = measureLabelSize(scale.ctx, plFont.lineHeight, scale._pointLabels[i]);
+    const textSize = measureLabelSize(scale.ctx, plFont, scale._pointLabels[i]);
     labelSizes[i] = textSize;
 
-    // Add quarter circle to make degree 0 mean top of circle
     const angleRadians = scale.getIndexAngle(i);
     const angle = toDegrees(angleRadians);
     const hLimits = determineLimits(angle, pointPosition.x, textSize.w, 0, 180);
@@ -133,50 +123,43 @@ function fitWithPointLabels(scale) {
 
   scale._setReductions(scale.drawingArea, furthestLimits, furthestAngles);
 
-  scale._pointLabelItems = [];
-
   // Now that text size is determined, compute the full positions
+  scale._pointLabelItems = buildPointLabelItems(scale, labelSizes, padding);
+}
+
+function buildPointLabelItems(scale, labelSizes, padding) {
+  const items = [];
+  const valueCount = scale.getLabels().length;
   const opts = scale.options;
   const tickBackdropHeight = getTickBackdropHeight(opts);
   const outerDistance = scale.getDistanceFromCenterForValue(opts.ticks.reverse ? scale.min : scale.max);
 
-  for (i = 0; i < valueCount; i++) {
+  for (let i = 0; i < valueCount; i++) {
     // Extra pixels out for some label spacing
     const extra = (i === 0 ? tickBackdropHeight / 2 : 0);
     const pointLabelPosition = scale.getPointPosition(i, outerDistance + extra + padding[i]);
-
     const angle = toDegrees(scale.getIndexAngle(i));
     const size = labelSizes[i];
-    adjustPointPositionForLabelHeight(angle, size, pointLabelPosition);
-
+    const y = yForAngle(pointLabelPosition.y, size.h, angle);
     const textAlign = getTextAlignForAngle(angle);
-    let left;
+    const left = leftForTextAlign(pointLabelPosition.x, size.w, textAlign);
 
-    if (textAlign === 'left') {
-      left = pointLabelPosition.x;
-    } else if (textAlign === 'center') {
-      left = pointLabelPosition.x - (size.w / 2);
-    } else {
-      left = pointLabelPosition.x - size.w;
-    }
-
-    const right = left + size.w;
-
-    scale._pointLabelItems[i] = {
+    items.push({
       // Text position
       x: pointLabelPosition.x,
-      y: pointLabelPosition.y,
+      y,
 
       // Text rendering data
       textAlign,
 
       // Bounding box
       left,
-      top: pointLabelPosition.y,
-      right,
-      bottom: pointLabelPosition.y + size.h,
-    };
+      top: y,
+      right: left + size.w,
+      bottom: y + size.h
+    });
   }
+  return items;
 }
 
 function getTextAlignForAngle(angle) {
@@ -189,12 +172,22 @@ function getTextAlignForAngle(angle) {
   return 'right';
 }
 
-function adjustPointPositionForLabelHeight(angle, textSize, position) {
-  if (angle === 90 || angle === 270) {
-    position.y -= (textSize.h / 2);
-  } else if (angle > 270 || angle < 90) {
-    position.y -= textSize.h;
+function leftForTextAlign(x, w, align) {
+  if (align === 'right') {
+    x -= w;
+  } else if (align === 'center') {
+    x -= (w / 2);
   }
+  return x;
+}
+
+function yForAngle(y, h, angle) {
+  if (angle === 90 || angle === 270) {
+    y -= (h / 2);
+  } else if (angle > 270 || angle < 90) {
+    y -= h;
+  }
+  return y;
 }
 
 function drawPointLabels(scale, labelCount) {
@@ -543,6 +536,7 @@ export default class RadialLinearScale extends LinearScaleBase {
       offset = me.getDistanceFromCenterForValue(me.ticks[index].value);
 
       if (optsAtIndex.showLabelBackdrop) {
+        ctx.font = tickFont.string;
         width = ctx.measureText(tick.label).width;
         ctx.fillStyle = optsAtIndex.backdropColor;
 

test/specs/helpers.canvas.tests.js

@@ -316,15 +316,15 @@ describe('Chart.helpers.canvas', function() {
       expect(context.getCalls()).toEqual([{
         name: 'save',
         args: [],
+      }, {
+        name: 'setFont',
+        args: ['12px arial'],
       }, {
         name: 'translate',
         args: [10, 20],
       }, {
         name: 'rotate',
         args: [90],
-      }, {
-        name: 'setFont',
-        args: ['12px arial'],
       }, {
         name: 'fillText',
         args: ['foo', 0, 0, undefined],

test/specs/plugin.title.tests.js

@@ -131,15 +131,15 @@ describe('Plugin.title', function() {
     expect(context.getCalls()).toEqual([{
       name: 'save',
       args: []
+    }, {
+      name: 'setFont',
+      args: ["normal bold 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"],
     }, {
       name: 'translate',
       args: [300, 67.2]
     }, {
       name: 'rotate',
       args: [0]
-    }, {
-      name: 'setFont',
-      args: ["normal bold 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"],
     }, {
       name: 'setFillStyle',
       args: ['#666']
@@ -192,15 +192,15 @@ describe('Plugin.title', function() {
     expect(context.getCalls()).toEqual([{
       name: 'save',
       args: []
+    }, {
+      name: 'setFont',
+      args: ["normal bold 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"],
     }, {
       name: 'translate',
       args: [117.2, 250]
     }, {
       name: 'rotate',
       args: [-0.5 * Math.PI]
-    }, {
-      name: 'setFont',
-      args: ["normal bold 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"],
     }, {
       name: 'setFillStyle',
       args: ['#666']
@@ -234,15 +234,15 @@ describe('Plugin.title', function() {
     expect(context.getCalls()).toEqual([{
       name: 'save',
       args: []
+    }, {
+      name: 'setFont',
+      args: ["normal bold 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"],
     }, {
       name: 'translate',
       args: [117.2, 250]
     }, {
       name: 'rotate',
       args: [0.5 * Math.PI]
-    }, {
-      name: 'setFont',
-      args: ["normal bold 12px 'Helvetica Neue', 'Helvetica', 'Arial', sans-serif"],
     }, {
       name: 'setFillStyle',
       args: ['#666']