Scale: autoSkip before fit (#8627)

Scale: autoSkip now occurs before fit in the update process

src/core/core.scale.js

@@ -192,6 +192,14 @@ function getTitleHeight(options, fallback) {
   return (lines * font.lineHeight) + padding.height;
 }
 
+function determineMaxTicks(scale) {
+  const offset = scale.options.offset;
+  const tickLength = scale._tickSize();
+  const maxScale = scale._length / tickLength + (offset ? 0 : 1);
+  const maxChart = scale._maxLength / tickLength;
+  return Math.floor(Math.min(maxScale, maxChart));
+}
+
 /**
  * @param {number[]} arr
  */
@@ -411,6 +419,7 @@ export default class Scale extends Element {
     /** @type {object|null} */
     this._labelSizes = null;
     this._length = 0;
+    this._maxLength = 0;
     this._longestTextCache = {};
     /** @type {number} */
     this._startPixel = undefined;
@@ -572,7 +581,7 @@ export default class Scale extends Element {
     // Absorb the master measurements
     me.maxWidth = maxWidth;
     me.maxHeight = maxHeight;
-    me._margins = Object.assign({
+    me._margins = margins = Object.assign({
       left: 0,
       right: 0,
       top: 0,
@@ -589,6 +598,10 @@ export default class Scale extends Element {
     me.setDimensions();
     me.afterSetDimensions();
 
+    me._maxLength = me.isHorizontal()
+      ? me.width + margins.left + margins.right
+      : me.height + margins.top + margins.bottom;
+
     // Data min/max
     if (!me._dataLimitsCached) {
       me.beforeDataLimits();
@@ -620,18 +633,21 @@ export default class Scale extends Element {
     me.calculateLabelRotation(); // Preconditions: number of ticks and sizes of largest labels must be calculated beforehand
     me.afterCalculateLabelRotation();
 
-    me.beforeFit();
-    me.fit(); // Preconditions: label rotation and label sizes must be calculated beforehand
-    me.afterFit();
-
     // Auto-skip
-    me.ticks = tickOpts.display && (tickOpts.autoSkip || tickOpts.source === 'auto') ? me._autoSkip(me.ticks) : me.ticks;
+    if (tickOpts.display && (tickOpts.autoSkip || tickOpts.source === 'auto')) {
+      me.ticks = me._autoSkip(me.ticks);
+      me._labelSizes = null;
+    }
 
     if (samplingEnabled) {
       // Generate labels using all non-skipped ticks
       me._convertTicksToLabels(me.ticks);
     }
 
+    me.beforeFit();
+    me.fit(); // Preconditions: label rotation and label sizes must be calculated beforehand
+    me.afterFit();
+
     // IMPORTANT: after this point, we consider that `this.ticks` will NEVER change!
 
     me.afterUpdate();
@@ -1162,8 +1178,8 @@ export default class Scale extends Element {
 	 */
   _autoSkip(ticks) {
     const me = this;
-    const {offset, ticks: tickOpts} = me.options;
-    const ticksLimit = tickOpts.maxTicksLimit || (me._length / me._tickSize() + (offset ? 0 : 1));
+    const tickOpts = me.options.ticks;
+    const ticksLimit = tickOpts.maxTicksLimit || determineMaxTicks(me);
     const majorIndices = tickOpts.major.enabled ? getMajorIndices(ticks) : [];
     const numMajorIndices = majorIndices.length;
     const first = majorIndices[0];

test/fixtures/core.layouts/refit-vertical-boxes.js

@@ -1,4 +1,5 @@
 module.exports = {
+  tolerance: 0.002,
   config: {
     type: 'line',
     data: {

test/fixtures/core.scale/autoSkip/fit-after.js

@@ -0,0 +1,51 @@
+module.exports = {
+  description: 'https://github.com/chartjs/Chart.js/issues/3694',
+  tolerance: 0.002,
+  config: {
+    type: 'line',
+    data: {
+      labels: [
+        'Aaron',
+        'Adam',
+        'Albert',
+        'Alex',
+        'Allan',
+        'Aman',
+        'Anthony',
+        'Autoenrolment',
+        'Avril',
+        'Bernard'
+      ],
+      datasets: [{
+        backgroundColor: 'rgba(252,233,79,0.5)',
+        borderColor: 'rgba(252,233,79,1)',
+        borderWidth: 1,
+        data: [101,
+          185,
+          24,
+          311,
+          17,
+          21,
+          462,
+          340,
+          140,
+          24
+        ]
+      }],
+    },
+    options: {
+      scales: {
+        x: {
+          backgroundColor: '#eee'
+        }
+      }
+    }
+  },
+  options: {
+    spriteText: true,
+    canvas: {
+      width: 185,
+      height: 185
+    }
+  }
+};

test/specs/core.scale.tests.js

@@ -64,14 +64,14 @@ describe('Core.scale', function() {
           'January 2019', 'February 2019', 'March 2019', 'April 2019',
           'May 2019', 'June 2019', 'July 2019', 'August 2019',
           'September 2019', 'October 2019', 'November 2019', 'December 2019',
-          'January 2020', 'February 2020'
+          'January 2020', 'February 2020', 'March 2020', 'April 2020'
         ],
         datasets: [{
-          data: [12, 19, 3, 5, 2, 3, 7, 8, 9, 10, 11, 12, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]
+          data: [1, 1, 1, 1, 2, 2, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7, 7, 7, 7]
         }]
       });
 
-      expect(lastTick(chart).label).toEqual('January 2020');
+      expect(lastTick(chart).label).toEqual('March 2020');
     });
   });
 

test/specs/scale.time.tests.js

@@ -1113,7 +1113,7 @@ describe('Time scale tests', function() {
     });
     const scale = chart.scales.x;
     expect(scale.getPixelForDecimal(0)).toBeCloseToPixel(29);
-    expect(scale.getPixelForDecimal(1.0)).toBeCloseToPixel(494);
+    expect(scale.getPixelForDecimal(1.0)).toBeCloseToPixel(509);
   });
 
   ['data', 'labels'].forEach(function(source) {