Update tick positioning (#8657)

* Update tick positioning
* Update tests

Author: kurkle

src/core/core.scale.js

@@ -43,7 +43,7 @@ defaults.set('scale', {
     drawBorder: true,
     drawOnChartArea: true,
     drawTicks: true,
-    tickLength: 10,
+    tickLength: 8,
     tickWidth: (_ctx, options) => options.lineWidth,
     tickColor: (_ctx, options) => options.color,
     offset: false,
@@ -75,7 +75,7 @@ defaults.set('scale', {
     mirror: false,
     textStrokeWidth: 0,
     textStrokeColor: '',
-    padding: 0,
+    padding: 3,
     display: true,
     autoSkip: true,
     autoSkipPadding: 3,
@@ -783,7 +783,7 @@ export default class Scale extends Element {
 
     const labelSizes = me._getLabelSizes();
     const maxLabelWidth = labelSizes.widest.width;
-    const maxLabelHeight = labelSizes.highest.height - labelSizes.highest.offset;
+    const maxLabelHeight = labelSizes.highest.height;
 
     // Estimate the width of each grid based on the canvas width, the maximum
     // label width and the number of tick intervals
@@ -822,104 +822,40 @@ export default class Scale extends Element {
       height: 0
     };
 
-    const chart = me.chart;
-    const opts = me.options;
-    const tickOpts = opts.ticks;
-    const titleOpts = opts.title;
-    const gridLineOpts = opts.grid;
+    const {chart, options: {ticks: tickOpts, title: titleOpts, grid: gridOpts}} = me;
     const display = me._isVisible();
-    const labelsBelowTicks = opts.position !== 'top' && me.axis === 'x';
     const isHorizontal = me.isHorizontal();
-    const titleHeight = display && getTitleHeight(titleOpts, chart.options.font);
-
-    // Width
-    if (isHorizontal) {
-      minSize.width = me.maxWidth;
-    } else if (display) {
-      minSize.width = getTickMarkLength(gridLineOpts) + titleHeight;
-    }
 
-    // height
-    if (!isHorizontal) {
-      minSize.height = me.maxHeight; // fill all the height
-    } else if (display) {
-      minSize.height = getTickMarkLength(gridLineOpts) + titleHeight;
-    }
+    if (display) {
+      const titleHeight = getTitleHeight(titleOpts, chart.options.font);
+      if (isHorizontal) {
+        minSize.width = me.maxWidth;
+        minSize.height = getTickMarkLength(gridOpts) + titleHeight;
+      } else {
+        minSize.height = me.maxHeight; // fill all the height
+        minSize.width = getTickMarkLength(gridOpts) + titleHeight;
+      }
 
-    // Don't bother fitting the ticks if we are not showing the labels
-    if (tickOpts.display && display && me.ticks.length) {
-      const labelSizes = me._getLabelSizes();
-      const firstLabelSize = labelSizes.first;
-      const lastLabelSize = labelSizes.last;
-      const widestLabelSize = labelSizes.widest;
-      const highestLabelSize = labelSizes.highest;
-      const lineSpace = highestLabelSize.offset * 0.8;
-      const tickPadding = tickOpts.padding;
+      // Don't bother fitting the ticks if we are not showing the labels
+      if (tickOpts.display && me.ticks.length) {
+        const {first, last, widest, highest} = me._getLabelSizes();
+        const tickPadding = tickOpts.padding * 2;
+        const angleRadians = toRadians(me.labelRotation);
+        const cos = Math.cos(angleRadians);
+        const sin = Math.sin(angleRadians);
 
-      if (isHorizontal) {
+        if (isHorizontal) {
         // A horizontal axis is more constrained by the height.
-        const isRotated = me.labelRotation !== 0;
-        const angleRadians = toRadians(me.labelRotation);
-        const cosRotation = Math.cos(angleRadians);
-        const sinRotation = Math.sin(angleRadians);
-
-        const labelHeight = sinRotation * widestLabelSize.width
-					+ cosRotation * (highestLabelSize.height - (isRotated ? highestLabelSize.offset : 0))
-					+ (isRotated ? 0 : lineSpace); // padding
-
-        minSize.height = Math.min(me.maxHeight, minSize.height + labelHeight + tickPadding);
-
-        const offsetLeft = me.getPixelForTick(0) - me.left;
-        const offsetRight = me.right - me.getPixelForTick(me.ticks.length - 1);
-        let paddingLeft, paddingRight;
-
-        // Ensure that our ticks are always inside the canvas. When rotated, ticks are right aligned
-        // which means that the right padding is dominated by the font height
-        if (isRotated) {
-          paddingLeft = labelsBelowTicks ?
-            cosRotation * firstLabelSize.width + sinRotation * firstLabelSize.offset :
-            sinRotation * (firstLabelSize.height - firstLabelSize.offset);
-          paddingRight = labelsBelowTicks ?
-            sinRotation * (lastLabelSize.height - lastLabelSize.offset) :
-            cosRotation * lastLabelSize.width + sinRotation * lastLabelSize.offset;
-        } else if (tickOpts.align === 'start') {
-          paddingLeft = 0;
-          paddingRight = lastLabelSize.width;
-        } else if (tickOpts.align === 'end') {
-          paddingLeft = firstLabelSize.width;
-          paddingRight = 0;
+          const labelHeight = sin * widest.width + cos * highest.height;
+          minSize.height = Math.min(me.maxHeight, minSize.height + labelHeight + tickPadding);
         } else {
-          paddingLeft = firstLabelSize.width / 2;
-          paddingRight = lastLabelSize.width / 2;
-        }
-
-        // Adjust padding taking into account changes in offsets
-        // and add 3 px to move away from canvas edges
-        me.paddingLeft = Math.max((paddingLeft - offsetLeft) * me.width / (me.width - offsetLeft), 0) + 3;
-        me.paddingRight = Math.max((paddingRight - offsetRight) * me.width / (me.width - offsetRight), 0) + 3;
-      } else {
         // A vertical axis is more constrained by the width. Labels are the
         // dominant factor here, so get that length first and account for padding
-        const labelWidth = tickOpts.mirror ? 0 :
-        // use lineSpace for consistency with horizontal axis
-        // tickPadding is not implemented for horizontal
-          widestLabelSize.width + tickPadding + lineSpace;
-
-        minSize.width = Math.min(me.maxWidth, minSize.width + labelWidth);
-
-        let paddingTop = lastLabelSize.height / 2;
-        let paddingBottom = firstLabelSize.height / 2;
-
-        if (tickOpts.align === 'start') {
-          paddingTop = 0;
-          paddingBottom = firstLabelSize.height;
-        } else if (tickOpts.align === 'end') {
-          paddingTop = lastLabelSize.height;
-          paddingBottom = 0;
-        }
+          const labelWidth = tickOpts.mirror ? 0 : cos * widest.width + sin * highest.height;
 
-        me.paddingTop = paddingTop;
-        me.paddingBottom = paddingBottom;
+          minSize.width = Math.min(me.maxWidth, minSize.width + labelWidth + tickPadding);
+        }
+        me._calculatePadding(first, last, sin, cos);
       }
     }
 
@@ -934,6 +870,57 @@ export default class Scale extends Element {
     }
   }
 
+  _calculatePadding(first, last, sin, cos) {
+    const me = this;
+    const {ticks: {align, padding}, position} = me.options;
+    const isRotated = me.labelRotation !== 0;
+    const labelsBelowTicks = position !== 'top' && me.axis === 'x';
+
+    if (me.isHorizontal()) {
+      const offsetLeft = me.getPixelForTick(0) - me.left;
+      const offsetRight = me.right - me.getPixelForTick(me.ticks.length - 1);
+      let paddingLeft = 0;
+      let paddingRight = 0;
+
+      // Ensure that our ticks are always inside the canvas. When rotated, ticks are right aligned
+      // which means that the right padding is dominated by the font height
+      if (isRotated) {
+        if (labelsBelowTicks) {
+          paddingLeft = cos * first.width;
+          paddingRight = sin * last.height;
+        } else {
+          paddingLeft = sin * first.height;
+          paddingRight = cos * last.width;
+        }
+      } else if (align === 'start') {
+        paddingRight = last.width;
+      } else if (align === 'end') {
+        paddingLeft = first.width;
+      } else {
+        paddingLeft = first.width / 2;
+        paddingRight = last.width / 2;
+      }
+
+      // Adjust padding taking into account changes in offsets
+      me.paddingLeft = Math.max((paddingLeft - offsetLeft + padding) * me.width / (me.width - offsetLeft), 0);
+      me.paddingRight = Math.max((paddingRight - offsetRight + padding) * me.width / (me.width - offsetRight), 0);
+    } else {
+      let paddingTop = last.height / 2;
+      let paddingBottom = first.height / 2;
+
+      if (align === 'start') {
+        paddingTop = 0;
+        paddingBottom = first.height;
+      } else if (align === 'end') {
+        paddingTop = last.height;
+        paddingBottom = 0;
+      }
+
+      me.paddingTop = paddingTop + padding;
+      me.paddingBottom = paddingBottom + padding;
+    }
+  }
+
   /**
 	 * Handle margins and padding interactions
 	 * @private
@@ -990,7 +977,13 @@ export default class Scale extends Element {
     let labelSizes = me._labelSizes;
 
     if (!labelSizes) {
-      me._labelSizes = labelSizes = me._computeLabelSizes();
+      const sampleSize = me.options.ticks.sampleSize;
+      let ticks = me.ticks;
+      if (sampleSize < ticks.length) {
+        ticks = sample(ticks, sampleSize);
+      }
+
+      me._labelSizes = labelSizes = me._computeLabelSizes(ticks, ticks.length);
     }
 
     return labelSizes;
@@ -1002,26 +995,17 @@ export default class Scale extends Element {
 	 * @return {{ first: object, last: object, widest: object, highest: object }}
 	 * @private
 	 */
-  _computeLabelSizes() {
-    const me = this;
-    const ctx = me.ctx;
-    const caches = me._longestTextCache;
-    const sampleSize = me.options.ticks.sampleSize;
+  _computeLabelSizes(ticks, length) {
+    const {ctx, _longestTextCache: caches} = this;
     const widths = [];
     const heights = [];
-    const offsets = [];
     let widestLabelSize = 0;
     let highestLabelSize = 0;
-    let ticks = me.ticks;
-    if (sampleSize < ticks.length) {
-      ticks = sample(ticks, sampleSize);
-    }
-    const length = ticks.length;
     let i, j, jlen, label, tickFont, fontString, cache, lineHeight, width, height, nestedLabel;
 
     for (i = 0; i < length; ++i) {
       label = ticks[i].label;
-      tickFont = me._resolveTickFontOptions(i);
+      tickFont = this._resolveTickFontOptions(i);
       ctx.font = fontString = tickFont.string;
       cache = caches[fontString] = caches[fontString] || {data: {}, gc: []};
       lineHeight = tickFont.lineHeight;
@@ -1043,7 +1027,6 @@ export default class Scale extends Element {
       }
       widths.push(width);
       heights.push(height);
-      offsets.push(lineHeight / 2);
       widestLabelSize = Math.max(width, widestLabelSize);
       highestLabelSize = Math.max(height, highestLabelSize);
     }
@@ -1052,13 +1035,7 @@ export default class Scale extends Element {
     const widest = widths.indexOf(widestLabelSize);
     const highest = heights.indexOf(highestLabelSize);
 
-    function valueAt(idx) {
-      return {
-        width: widths[idx] || 0,
-        height: heights[idx] || 0,
-        offset: offsets[idx] || 0
-      };
-    }
+    const valueAt = (idx) => ({width: widths[idx] || 0, height: heights[idx] || 0});
 
     return {
       first: valueAt(0),
@@ -1299,7 +1276,7 @@ export default class Scale extends Element {
       tx2 = me.left + tl;
     } else if (axis === 'x') {
       if (position === 'center') {
-        borderValue = alignBorderValue((chartArea.top + chartArea.bottom) / 2);
+        borderValue = alignBorderValue((chartArea.top + chartArea.bottom) / 2 + 0.5);
       } else if (isObject(position)) {
         const positionAxisID = Object.keys(position)[0];
         const value = position[positionAxisID];
@@ -1458,24 +1435,20 @@ export default class Scale extends Element {
         x = pixel;
         if (position === 'top') {
           if (crossAlign === 'near' || rotation !== 0) {
-            textOffset = (Math.sin(rotation) * halfCount + 0.5) * lineHeight;
-            textOffset -= (rotation === 0 ? (lineCount - 0.5) : Math.cos(rotation) * halfCount) * lineHeight;
+            textOffset = -lineCount * lineHeight + lineHeight / 2;
           } else if (crossAlign === 'center') {
-            textOffset = -1 * (labelSizes.highest.height / 2);
-            textOffset -= halfCount * lineHeight;
+            textOffset = -labelSizes.highest.height / 2 - halfCount * lineHeight + lineHeight;
           } else {
-            textOffset = (-1 * labelSizes.highest.height) + (0.5 * lineHeight);
+            textOffset = -labelSizes.highest.height + lineHeight / 2;
           }
         } else {
           // eslint-disable-next-line no-lonely-if
           if (crossAlign === 'near' || rotation !== 0) {
-            textOffset = Math.sin(rotation) * halfCount * lineHeight;
-            textOffset += (rotation === 0 ? 0.5 : Math.cos(rotation) * halfCount) * lineHeight;
+            textOffset = lineHeight / 2;
           } else if (crossAlign === 'center') {
-            textOffset = labelSizes.highest.height / 2;
-            textOffset -= halfCount * lineHeight;
+            textOffset = labelSizes.highest.height / 2 - halfCount * lineHeight;
           } else {
-            textOffset = labelSizes.highest.height - ((lineCount - 0.5) * lineHeight);
+            textOffset = labelSizes.highest.height - lineCount * lineHeight;
           }
         }
       } else {
@@ -1522,12 +1495,10 @@ export default class Scale extends Element {
 
   _getYAxisLabelAlignment(tl) {
     const me = this;
-    const {position, ticks} = me.options;
-    const {crossAlign, mirror, padding} = ticks;
+    const {position, ticks: {crossAlign, mirror, padding}} = me.options;
     const labelSizes = me._getLabelSizes();
     const tickAndPadding = tl + padding;
     const widest = labelSizes.widest.width;
-    const lineSpace = labelSizes.highest.offset * 0.8;
 
     let textAlign;
     let x;
@@ -1546,7 +1517,7 @@ export default class Scale extends Element {
           x -= (widest / 2);
         } else {
           textAlign = 'left';
-          x = me.left + lineSpace;
+          x = me.left;
         }
       }
     } else if (position === 'right') {
@@ -1563,7 +1534,7 @@ export default class Scale extends Element {
           x += widest / 2;
         } else {
           textAlign = 'right';
-          x = me.right - lineSpace;
+          x = me.right;
         }
       }
     } else {