Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix layout refit logic #8567

Merged
merged 3 commits into from
Mar 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 31 additions & 17 deletions src/core/core.layouts.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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'),
Expand All @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/core/core.scale.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand Down
Binary file modified test/fixtures/core.layouts/long-labels.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
52 changes: 52 additions & 0 deletions test/fixtures/core.layouts/refit-vertical-boxes.js
Original file line number Diff line number Diff line change
@@ -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
}
}
};
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/fixtures/scale.time/invalid-data.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 4 additions & 2 deletions test/specs/core.controller.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down