From: Jukka Kurkela Date: Mon, 29 Mar 2021 20:53:47 +0000 (+0300) Subject: Scale: draw offset grid for labels before autoSkip (#8748) X-Git-Tag: v3.0.0-rc.7~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cdba66ccef67c4671fe4bf7873fbba7524c4e935;p=thirdparty%2FChart.js.git Scale: draw offset grid for labels before autoSkip (#8748) * Scale: draw offset grid for labels before autoSkip * fix tests --- diff --git a/docs/docs/axes/styling.mdx b/docs/docs/axes/styling.mdx index 5a98fe3fe..76ce82152 100644 --- a/docs/docs/axes/styling.mdx +++ b/docs/docs/axes/styling.mdx @@ -23,7 +23,7 @@ Namespace: `options.scales[scaleId].grid`, it defines options for the grid lines | `drawOnChartArea` | `boolean` | | | `true` | If true, draw lines on the chart area inside the axis lines. This is useful when there are multiple axes and you need to control which grid lines are drawn. | `drawTicks` | `boolean` | | | `true` | If true, draw lines beside the ticks in the axis area beside the chart. | `lineWidth` | `number` | Yes | Yes | `1` | Stroke width of grid lines. -| `offset` | `boolean` | | | `false` | If true, grid lines will be shifted to be between labels. This is set to `true` for a bar chart by default. +| `offset` | `boolean` | | | `false` | If true, grid lines will be shifted to be between labels. This is set to `true` for a bar chart by default. Note: AutoSkip does not remove offset grid lines. | `tickBorderDash` | `number[]` | | | | Length and spacing of the tick mark line. If not set, defaults to the grid line `borderDash` value. | `tickBorderDashOffset` | `number` | Yes | Yes | | Offset for the line dash of the tick mark. If unset, defaults to the grid line `borderDashOffset` value | `tickColor` | [`Color`](../general/colors.md) | Yes | Yes | | Color of the tick line. If unset, defaults to the grid line color. diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 3913ad302..9d8d14d4c 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -42,21 +42,21 @@ function sample(arr, numItems) { * @param {boolean} offsetGridLines */ function getPixelForGridLine(scale, index, offsetGridLines) { - const length = scale.ticks.length; + const length = offsetGridLines ? scale._allTicks.length : scale.ticks.length; const validIndex = Math.min(index, length - 1); const start = scale._startPixel; const end = scale._endPixel; const epsilon = 1e-6; // 1e-6 is margin in pixels for accumulated error. - let lineValue = scale.getPixelForTick(validIndex); + let lineValue = scale.getPixelForTick(validIndex, offsetGridLines); let offset; if (offsetGridLines) { if (length === 1) { offset = Math.max(lineValue - start, end - lineValue); } else if (index === 0) { - offset = (scale.getPixelForTick(1) - lineValue) / 2; + offset = (scale.getPixelForTick(1, offsetGridLines) - lineValue) / 2; } else { - offset = (lineValue - scale.getPixelForTick(validIndex - 1)) / 2; + offset = (lineValue - scale.getPixelForTick(validIndex - 1, offsetGridLines)) / 2; } lineValue += validIndex < index ? offset : -offset; @@ -205,7 +205,7 @@ export default class Scale extends Element { this.min = undefined; this.max = undefined; /** @type {Tick[]} */ - this.ticks = []; + this.ticks = this._allTicks = []; /** @type {object[]|null} */ this._gridLineItems = null; /** @type {object[]|null} */ @@ -428,6 +428,7 @@ export default class Scale extends Element { me.afterCalculateLabelRotation(); // Auto-skip + me._allTicks = me.ticks; if (tickOpts.display && (tickOpts.autoSkip || tickOpts.source === 'auto')) { me.ticks = autoSkip(me, me.ticks); me._labelSizes = null; @@ -666,7 +667,7 @@ export default class Scale extends Element { _calculatePadding(first, last, sin, cos) { const me = this; - const {ticks: {align, padding}, position} = me.options; + const {position, ticks: {align, padding}} = me.options; const isRotated = me.labelRotation !== 0; const labelsBelowTicks = position !== 'top' && me.axis === 'x'; @@ -696,8 +697,8 @@ export default class Scale extends Element { } // 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); + me.paddingLeft = Math.max((paddingLeft - offsetLeft + padding) * me.width / (me.width - offsetLeft), padding); + me.paddingRight = Math.max((paddingRight - offsetRight + padding) * me.width / (me.width - offsetRight), padding); } else { let paddingTop = last.height / 2; let paddingBottom = first.height / 2; @@ -871,10 +872,11 @@ export default class Scale extends Element { * Returns the location of the tick at the given index * The coordinate (0, 0) is at the upper-left corner of the canvas * @param {number} index + * @param {boolean} [all] - use ticks before autoSkip * @return {number} */ - getPixelForTick(index) { - const ticks = this.ticks; + getPixelForTick(index, all = false) { + const ticks = all ? this._allTicks : this.ticks; if (index < 0 || index > ticks.length - 1) { return null; } @@ -992,7 +994,7 @@ export default class Scale extends Element { const {grid, position} = options; const offset = grid.offset; const isHorizontal = me.isHorizontal(); - const ticks = me.ticks; + const ticks = offset ? me._allTicks : me.ticks; const ticksLength = ticks.length + (offset ? 1 : 0); const tl = getTickMarkLength(grid); const items = []; diff --git a/src/scales/scale.category.js b/src/scales/scale.category.js index 28f57b082..ea0716a4c 100644 --- a/src/scales/scale.category.js +++ b/src/scales/scale.category.js @@ -109,17 +109,6 @@ export default class CategoryScale extends Scale { return value === null ? NaN : me.getPixelForDecimal((value - me._startValue) / me._valueRange); } - // Must override base implementation because it calls getPixelForValue - // and category scale can have duplicate values - getPixelForTick(index) { - const me = this; - const ticks = me.ticks; - if (index < 0 || index > ticks.length - 1) { - return null; - } - return me.getPixelForValue(ticks[index].value); - } - getValueForPixel(pixel) { const me = this; return Math.round(me._startValue + me.getDecimalForPixel(pixel) * me._valueRange); diff --git a/test/fixtures/core.scale/autoSkip/offset.png b/test/fixtures/core.scale/autoSkip/offset.png index 72ee4eef4..95db4cd4b 100644 Binary files a/test/fixtures/core.scale/autoSkip/offset.png and b/test/fixtures/core.scale/autoSkip/offset.png differ diff --git a/test/fixtures/core.scale/crossAlignment/cross-align-bottom-center.png b/test/fixtures/core.scale/crossAlignment/cross-align-bottom-center.png index 6f3aaed90..4cd38a6bd 100644 Binary files a/test/fixtures/core.scale/crossAlignment/cross-align-bottom-center.png and b/test/fixtures/core.scale/crossAlignment/cross-align-bottom-center.png differ diff --git a/test/fixtures/core.scale/crossAlignment/cross-align-bottom-far.png b/test/fixtures/core.scale/crossAlignment/cross-align-bottom-far.png index abca217fa..e4c99e597 100644 Binary files a/test/fixtures/core.scale/crossAlignment/cross-align-bottom-far.png and b/test/fixtures/core.scale/crossAlignment/cross-align-bottom-far.png differ diff --git a/test/fixtures/core.scale/crossAlignment/cross-align-bottom-near.png b/test/fixtures/core.scale/crossAlignment/cross-align-bottom-near.png index 02791ad91..73dae7174 100644 Binary files a/test/fixtures/core.scale/crossAlignment/cross-align-bottom-near.png and b/test/fixtures/core.scale/crossAlignment/cross-align-bottom-near.png differ diff --git a/test/fixtures/core.scale/crossAlignment/cross-align-top-center.png b/test/fixtures/core.scale/crossAlignment/cross-align-top-center.png index c9083a830..04865d402 100644 Binary files a/test/fixtures/core.scale/crossAlignment/cross-align-top-center.png and b/test/fixtures/core.scale/crossAlignment/cross-align-top-center.png differ diff --git a/test/fixtures/core.scale/crossAlignment/cross-align-top-far.png b/test/fixtures/core.scale/crossAlignment/cross-align-top-far.png index b186168a8..8e0316424 100644 Binary files a/test/fixtures/core.scale/crossAlignment/cross-align-top-far.png and b/test/fixtures/core.scale/crossAlignment/cross-align-top-far.png differ diff --git a/test/fixtures/core.scale/crossAlignment/cross-align-top-near.png b/test/fixtures/core.scale/crossAlignment/cross-align-top-near.png index ce9e5c67a..c306c9b98 100644 Binary files a/test/fixtures/core.scale/crossAlignment/cross-align-top-near.png and b/test/fixtures/core.scale/crossAlignment/cross-align-top-near.png differ diff --git a/test/fixtures/scale.category/autoskip-grid-x.js b/test/fixtures/scale.category/autoskip-grid-x.js new file mode 100644 index 000000000..6912fa329 --- /dev/null +++ b/test/fixtures/scale.category/autoskip-grid-x.js @@ -0,0 +1,38 @@ +module.exports = { + config: { + type: 'bar', + data: { + labels: ['Label 1', 'Label 2', 'Label 3', 'Label 4', 'Label 5', 'Label 6', 'Label 7', 'Label 8', 'Label 9', 'Label 10', 'Label 11', 'Label 12'], + datasets: [{ + data: [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1] + }, { + data: [2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2] + }, { + data: [3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3] + }] + }, + options: { + scales: { + x: { + ticks: { + maxRotation: 0 + }, + grid: { + color: 'red', + lineWidth: 1 + } + }, + y: { + display: false + } + } + } + }, + options: { + spriteText: true, + canvas: { + width: 512, + height: 256 + } + } +}; diff --git a/test/fixtures/scale.category/autoskip-grid-x.png b/test/fixtures/scale.category/autoskip-grid-x.png new file mode 100644 index 000000000..370678003 Binary files /dev/null and b/test/fixtures/scale.category/autoskip-grid-x.png differ diff --git a/test/fixtures/scale.category/autoskip-grid-y.js b/test/fixtures/scale.category/autoskip-grid-y.js new file mode 100644 index 000000000..0f9b83eb7 --- /dev/null +++ b/test/fixtures/scale.category/autoskip-grid-y.js @@ -0,0 +1,37 @@ +module.exports = { + config: { + type: 'bar', + data: { + // labels: [['Label 1', 'Line 2'], ['Label 2', 'Line 2'], ['Label 3', 'Line 2'], ['Label 4', 'Line 2'], ['Label 5', 'Line 2'], ['Label 6', 'Line 2'], ['Label 7', 'Line 2'], ['Label 8', 'Line 2'], ['Label 9', 'Line 2'], ['Label 10', 'Line 2'], ['Label 11', 'Line 2'], ['Label 12', 'Line 2']], + labels: ['Label 1', 'Label 2', 'Label 3', 'Label 4', 'Label 5', 'Label 6', 'Label 7', 'Label 8', 'Label 9', 'Label 10', 'Label 11', 'Label 12'], + datasets: [{ + data: [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1] + }, { + data: [2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2] + }, { + data: [3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3] + }] + }, + options: { + indexAxis: 'y', + scales: { + y: { + grid: { + color: 'red', + lineWidth: 1 + } + }, + x: { + display: false + } + } + } + }, + options: { + spriteText: true, + canvas: { + width: 128, + height: 200 + } + } +}; diff --git a/test/fixtures/scale.category/autoskip-grid-y.png b/test/fixtures/scale.category/autoskip-grid-y.png new file mode 100644 index 000000000..a88c838ba Binary files /dev/null and b/test/fixtures/scale.category/autoskip-grid-y.png differ diff --git a/test/fixtures/scale.time/offset-with-1-tick.png b/test/fixtures/scale.time/offset-with-1-tick.png index 87870bff5..7088421b1 100644 Binary files a/test/fixtures/scale.time/offset-with-1-tick.png and b/test/fixtures/scale.time/offset-with-1-tick.png differ diff --git a/test/fixtures/scale.time/offset-with-2-ticks.png b/test/fixtures/scale.time/offset-with-2-ticks.png index 5b02f0015..8de98fa33 100644 Binary files a/test/fixtures/scale.time/offset-with-2-ticks.png and b/test/fixtures/scale.time/offset-with-2-ticks.png differ diff --git a/test/fixtures/scale.timeseries/financial-daily.png b/test/fixtures/scale.timeseries/financial-daily.png index 659c0a1da..c5d5e4e7c 100644 Binary files a/test/fixtures/scale.timeseries/financial-daily.png and b/test/fixtures/scale.timeseries/financial-daily.png differ diff --git a/test/specs/core.layouts.tests.js b/test/specs/core.layouts.tests.js index 67a3d4b93..c37ee5fc1 100644 --- a/test/specs/core.layouts.tests.js +++ b/test/specs/core.layouts.tests.js @@ -32,13 +32,13 @@ describe('Chart.layouts', function() { expect(chart.chartArea.bottom).toBeCloseToPixel(120); expect(chart.chartArea.left).toBeCloseToPixel(31); - expect(chart.chartArea.right).toBeCloseToPixel(250); + expect(chart.chartArea.right).toBeCloseToPixel(247); expect(chart.chartArea.top).toBeCloseToPixel(32); // Is xScale at the right spot expect(chart.scales.x.bottom).toBeCloseToPixel(150); expect(chart.scales.x.left).toBeCloseToPixel(31); - expect(chart.scales.x.right).toBeCloseToPixel(250); + expect(chart.scales.x.right).toBeCloseToPixel(247); expect(chart.scales.x.top).toBeCloseToPixel(120); expect(chart.scales.x.labelRotation).toBeCloseTo(0); @@ -79,13 +79,13 @@ describe('Chart.layouts', function() { }); expect(chart.chartArea.bottom).toBeCloseToPixel(139); - expect(chart.chartArea.left).toBeCloseToPixel(0); + expect(chart.chartArea.left).toBeCloseToPixel(3); expect(chart.chartArea.right).toBeCloseToPixel(218); expect(chart.chartArea.top).toBeCloseToPixel(62); // Is xScale at the right spot expect(chart.scales.x.bottom).toBeCloseToPixel(62); - expect(chart.scales.x.left).toBeCloseToPixel(0); + expect(chart.scales.x.left).toBeCloseToPixel(3); expect(chart.scales.x.right).toBeCloseToPixel(218); expect(chart.scales.x.top).toBeCloseToPixel(32); expect(chart.scales.x.labelRotation).toBeCloseTo(0); @@ -160,13 +160,13 @@ describe('Chart.layouts', function() { expect(chart.chartArea.bottom).toBeCloseToPixel(110); expect(chart.chartArea.left).toBeCloseToPixel(70); - expect(chart.chartArea.right).toBeCloseToPixel(250); + expect(chart.chartArea.right).toBeCloseToPixel(247); expect(chart.chartArea.top).toBeCloseToPixel(32); // Is xScale at the right spot expect(chart.scales.x.bottom).toBeCloseToPixel(150); expect(chart.scales.x.left).toBeCloseToPixel(70); - expect(chart.scales.x.right).toBeCloseToPixel(250); + expect(chart.scales.x.right).toBeCloseToPixel(247); expect(chart.scales.x.top).toBeCloseToPixel(110); expect(chart.scales.x.labelRotation).toBeCloseTo(40, -1); diff --git a/test/specs/platform.basic.tests.js b/test/specs/platform.basic.tests.js index 16b969912..e117259f1 100644 --- a/test/specs/platform.basic.tests.js +++ b/test/specs/platform.basic.tests.js @@ -54,7 +54,7 @@ describe('Platform.basic', function() { expect(chart.chartArea.bottom).toBeCloseToPixel(120); expect(chart.chartArea.left).toBeCloseToPixel(31); - expect(chart.chartArea.right).toBeCloseToPixel(250); + expect(chart.chartArea.right).toBeCloseToPixel(247); expect(chart.chartArea.top).toBeCloseToPixel(32); }); @@ -84,7 +84,7 @@ describe('Platform.basic', function() { expect(chart.chartArea.bottom).toBeCloseToPixel(150); expect(chart.chartArea.left).toBeCloseToPixel(31); - expect(chart.chartArea.right).toBeCloseToPixel(300); + expect(chart.chartArea.right).toBeCloseToPixel(297); expect(chart.chartArea.top).toBeCloseToPixel(32); }); }); diff --git a/test/specs/scale.category.tests.js b/test/specs/scale.category.tests.js index fe3aad52e..f7da13015 100644 --- a/test/specs/scale.category.tests.js +++ b/test/specs/scale.category.tests.js @@ -473,7 +473,7 @@ describe('Category scale tests', function() { var xScale = chart.scales.x; expect(xScale.getPixelForValue(0)).toBeCloseToPixel(89); expect(xScale.getPixelForValue(3)).toBeCloseToPixel(451); - expect(xScale.getPixelForValue(4)).toBeCloseToPixel(572); + expect(xScale.getPixelForValue(4)).toBeCloseToPixel(570); }); it('Should get the correct pixel for an object value in a horizontal bar chart', function() { diff --git a/test/specs/scale.time.tests.js b/test/specs/scale.time.tests.js index e9fccdf8f..9309ffd2b 100644 --- a/test/specs/scale.time.tests.js +++ b/test/specs/scale.time.tests.js @@ -1114,7 +1114,7 @@ describe('Time scale tests', function() { }); const scale = chart.scales.x; expect(scale.getPixelForDecimal(0)).toBeCloseToPixel(29); - expect(scale.getPixelForDecimal(1.0)).toBeCloseToPixel(512); + expect(scale.getPixelForDecimal(1.0)).toBeCloseToPixel(509); }); ['data', 'labels'].forEach(function(source) {