Fix layout refit logic (#8567)

* Fix layout refit logic

* CC

* Update fixture

src/core/core.layouts.js

@@ -60,6 +60,7 @@ function setLayoutDims(layouts, params) {
 
 function buildLayoutBoxes(boxes) {
   const layoutBoxes = wrapBoxes(boxes);
+  const fullSize = sortByWeight(layoutBoxes.filter(wrap => wrap.box.fullSize), true);
   const left = sortByWeight(filterByPosition(layoutBoxes, 'left'), true);
   const right = sortByWeight(filterByPosition(layoutBoxes, 'right'));
   const top = sortByWeight(filterByPosition(layoutBoxes, 'top'), true);
@@ -68,6 +69,7 @@ function buildLayoutBoxes(boxes) {
   const centerVertical = filterDynamicPositionByAxis(layoutBoxes, 'y');
 
   return {
+    fullSize,
     leftAndTop: left.concat(top),
     rightAndBottom: right.concat(centerVertical).concat(bottom).concat(centerHorizontal),
     chartArea: filterByPosition(layoutBoxes, 'chartArea'),
@@ -80,13 +82,20 @@ function getCombinedMax(maxPadding, chartArea, a, b) {
   return Math.max(maxPadding[a], chartArea[a]) + Math.max(maxPadding[b], chartArea[b]);
 }
 
+function updateMaxPadding(maxPadding, boxPadding) {
+  maxPadding.top = Math.max(maxPadding.top, boxPadding.top);
+  maxPadding.left = Math.max(maxPadding.left, boxPadding.left);
+  maxPadding.bottom = Math.max(maxPadding.bottom, boxPadding.bottom);
+  maxPadding.right = Math.max(maxPadding.right, boxPadding.right);
+}
+
 function updateDims(chartArea, params, layout) {
   const box = layout.box;
   const maxPadding = chartArea.maxPadding;
 
   if (isObject(layout.pos)) {
     // dynamically placed boxes are not considered
-    return;
+    return {same: false, other: false};
   }
   if (layout.size) {
     // this layout was already counted for, lets first reduce old size
@@ -96,23 +105,23 @@ function updateDims(chartArea, params, layout) {
   chartArea[layout.pos] += layout.size;
 
   if (box.getPadding) {
-    const boxPadding = box.getPadding();
-    maxPadding.top = Math.max(maxPadding.top, boxPadding.top);
-    maxPadding.left = Math.max(maxPadding.left, boxPadding.left);
-    maxPadding.bottom = Math.max(maxPadding.bottom, boxPadding.bottom);
-    maxPadding.right = Math.max(maxPadding.right, boxPadding.right);
+    updateMaxPadding(maxPadding, box.getPadding());
   }
 
   const newWidth = Math.max(0, params.outerWidth - getCombinedMax(maxPadding, chartArea, 'left', 'right'));
   const newHeight = Math.max(0, params.outerHeight - getCombinedMax(maxPadding, chartArea, 'top', 'bottom'));
 
-  if (newWidth !== chartArea.w || newHeight !== chartArea.h) {
+  const widthChanged = newWidth !== chartArea.w;
+  const heightChanged = newHeight !== chartArea.h;
+  if (widthChanged || heightChanged) {
     chartArea.w = newWidth;
     chartArea.h = newHeight;
-
-    // return true if chart area changed in layout's direction
-    return layout.horizontal ? newWidth !== chartArea.w : newHeight !== chartArea.h;
   }
+
+  // return booleans on the changes per direction
+  return layout.horizontal
+    ? {same: widthChanged, other: heightChanged}
+    : {same: heightChanged, other: widthChanged};
 }
 
 function handleMaxPadding(chartArea) {
@@ -158,13 +167,15 @@ function fitBoxes(boxes, chartArea, params) {
       layout.height || chartArea.h,
       getMargins(layout.horizontal, chartArea)
     );
-    if (updateDims(chartArea, params, layout)) {
+    const {same, other} = updateDims(chartArea, params, layout);
+    if (same && refitBoxes.length) {
+      // Dimensions changed and there were non full width boxes before this
+      // -> we have to refit those
+      refit = true;
+    }
+    if (other) {
+      // Chart area changed in the opposite direction
       changed = true;
-      if (refitBoxes.length) {
-        // Dimensions changed and there were non full width boxes before this
-        // -> we have to refit those
-        refit = true;
-      }
     }
     if (!box.fullSize) { // fullSize boxes don't need to be re-fitted in any case
       refitBoxes.push(layout);
@@ -365,7 +376,10 @@ export default {
 
     setLayoutDims(verticalBoxes.concat(horizontalBoxes), params);
 
-    // First fit vertical boxes
+    // First fit the fullSize boxes, to reduce probability of re-fitting.
+    fitBoxes(boxes.fullSize, chartArea, params);
+
+    // Then fit vertical boxes
     fitBoxes(verticalBoxes, chartArea, params);
 
     // Then fit horizontal boxes

src/core/core.scale.js

@@ -2,7 +2,7 @@ import defaults from './core.defaults';
 import Element from './core.element';
 import {_alignPixel, _measureText, renderText, clipArea, unclipArea} from '../helpers/helpers.canvas';
 import {callback as call, each, finiteOrDefault, isArray, isFinite, isNullOrUndef, isObject, valueOrDefault} from '../helpers/helpers.core';
-import {_factorize, toDegrees, toRadians, _int16Range, HALF_PI} from '../helpers/helpers.math';
+import {_factorize, toDegrees, toRadians, _int16Range, HALF_PI, _limitValue} from '../helpers/helpers.math';
 import {toFont, toPadding} from '../helpers/helpers.options';
 import Ticks from './core.ticks';
 
@@ -734,7 +734,7 @@ export default class Scale extends Element {
 
     // Estimate the width of each grid based on the canvas width, the maximum
     // label width and the number of tick intervals
-    const maxWidth = Math.min(me.maxWidth, me.chart.width - maxLabelWidth);
+    const maxWidth = _limitValue(me.chart.width - maxLabelWidth, 0, me.maxWidth);
     tickWidth = options.offset ? me.maxWidth / numTicks : maxWidth / (numTicks - 1);
 
     // Allow 3 pixels x2 padding either side for label readability

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

@@ -0,0 +1,52 @@
+module.exports = {
+  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: {
+      maintainAspectRatio: false,
+      plugins: {
+        legend: true,
+        title: {
+          display: true,
+          text: 'test'
+        }
+      }
+    }
+  },
+  options: {
+    spriteText: true,
+    canvas: {
+      height: 185,
+      width: 185
+    }
+  }
+};

test/specs/core.controller.tests.js

@@ -1430,14 +1430,16 @@ describe('Chart', function() {
       update: [
         'beforeUpdate',
         'beforeLayout',
-        'beforeDataLimits',
+        'beforeDataLimits', // y-axis fit
         'afterDataLimits',
         'beforeBuildTicks',
         'afterBuildTicks',
-        'beforeDataLimits',
+        'beforeDataLimits', // x-axis fit
         'afterDataLimits',
         'beforeBuildTicks',
         'afterBuildTicks',
+        'beforeBuildTicks', // y-axis re-fit
+        'afterBuildTicks',
         'afterLayout',
         'beforeDatasetsUpdate',
         'beforeDatasetUpdate',