From cb95a36b946f9898b1eab3c4c6ea403f2b369f1d Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Thu, 14 Nov 2019 04:15:44 -0800 Subject: [PATCH] Fix value passed to getPixelForTick (#6724) --- docs/getting-started/v3-migration.md | 1 + src/core/core.scale.js | 20 +++++------ test/specs/core.scale.tests.js | 2 +- test/specs/scale.time.tests.js | 53 ++++++++++++++++++++++------ 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/docs/getting-started/v3-migration.md b/docs/getting-started/v3-migration.md index 8cb859cd1..66486cf80 100644 --- a/docs/getting-started/v3-migration.md +++ b/docs/getting-started/v3-migration.md @@ -100,6 +100,7 @@ Chart.js is no longer providing the `Chart.bundle.js` and `Chart.bundle.min.js`. ##### Ticks +* When `autoSkip` is enabled, `scale.ticks` now contains only the non-skipped ticks instead of all ticks. * `scale.ticks` now contains objects instead of strings * `buildTicks` is now expected to return tick objects * `afterBuildTicks` now has no parameters like the other callbacks diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 7fafa704a..cd2bcb3b5 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -508,11 +508,11 @@ class Scale extends Element { me.afterFit(); // Auto-skip - me._ticksToDraw = tickOpts.display && (tickOpts.autoSkip || tickOpts.source === 'auto') ? me._autoSkip(me.ticks) : me.ticks; + me.ticks = tickOpts.display && (tickOpts.autoSkip || tickOpts.source === 'auto') ? me._autoSkip(me.ticks) : me.ticks; if (samplingEnabled) { // Generate labels using all non-skipped ticks - me._convertTicksToLabels(me._ticksToDraw); + me._convertTicksToLabels(me.ticks); } // IMPORTANT: after this point, we consider that `this.ticks` will NEVER change! @@ -987,14 +987,12 @@ class Scale extends Element { var position = options.position; var offsetGridLines = gridLines.offsetGridLines; var isHorizontal = me.isHorizontal(); - var ticks = me._ticksToDraw; + var ticks = me.ticks; var ticksLength = ticks.length + (offsetGridLines ? 1 : 0); - var context; - var tl = getTickMarkLength(gridLines); var items = []; - context = { + var context = { scale: me, tick: ticks[0], }; @@ -1046,7 +1044,7 @@ class Scale extends Element { const borderDash = gridLines.borderDash || []; const borderDashOffset = resolve([gridLines.borderDashOffset], context, i); - lineValue = getPixelForGridLine(me, tick._index || i, offsetGridLines); + lineValue = getPixelForGridLine(me, i, offsetGridLines); // Skip if the pixel is out of the range if (lineValue === undefined) { @@ -1093,7 +1091,7 @@ class Scale extends Element { var position = options.position; var isMirrored = optionTicks.mirror; var isHorizontal = me.isHorizontal(); - var ticks = me._ticksToDraw; + var ticks = me.ticks; var fonts = parseTickFontOptions(optionTicks); var tickPadding = optionTicks.padding; var tl = getTickMarkLength(options.gridLines); @@ -1119,7 +1117,7 @@ class Scale extends Element { tick = ticks[i]; label = tick.label; - pixel = me.getPixelForTick(tick._index || i) + optionTicks.labelOffset; + pixel = me.getPixelForTick(i) + optionTicks.labelOffset; font = tick.major ? fonts.major : fonts.minor; lineHeight = font.lineHeight; lineCount = isArray(label) ? label.length : 1; @@ -1164,7 +1162,7 @@ class Scale extends Element { var alignPixel = helpers._alignPixel; var context = { scale: me, - tick: me._ticksToDraw[0], + tick: me.ticks[0], }; var axisWidth = gridLines.drawBorder ? resolve([gridLines.lineWidth, 0], context, 0) : 0; var items = me._gridLineItems || (me._gridLineItems = me._computeGridLineItems(chartArea)); @@ -1206,7 +1204,7 @@ class Scale extends Element { var firstLineWidth = axisWidth; context = { scale: me, - tick: me._ticksToDraw[items.ticksLength - 1], + tick: me.ticks[items.ticksLength - 1], }; var lastLineWidth = resolve([gridLines.lineWidth, 1], context, items.ticksLength - 1); var borderValue = items.borderValue; diff --git a/test/specs/core.scale.tests.js b/test/specs/core.scale.tests.js index 582f952d1..413b014ee 100644 --- a/test/specs/core.scale.tests.js +++ b/test/specs/core.scale.tests.js @@ -75,7 +75,7 @@ describe('Core.scale', function() { }] }); - expect(lastTick(chart).label).toBeUndefined(); + expect(lastTick(chart).label).toEqual('January 2020'); }); }); diff --git a/test/specs/scale.time.tests.js b/test/specs/scale.time.tests.js index 97ef82a1b..f314af77a 100755 --- a/test/specs/scale.time.tests.js +++ b/test/specs/scale.time.tests.js @@ -126,7 +126,7 @@ describe('Time scale tests', function() { var ticks = getLabels(scale); // `bounds === 'data'`: first and last ticks removed since outside the data range - expect(ticks.length).toEqual(217); + expect(ticks.length).toEqual(44); }); it('should accept labels as date objects', function() { @@ -137,7 +137,7 @@ describe('Time scale tests', function() { var ticks = getLabels(scale); // `bounds === 'data'`: first and last ticks removed since outside the data range - expect(ticks.length).toEqual(217); + expect(ticks.length).toEqual(44); }); it('should accept data as xy points', function() { @@ -185,7 +185,7 @@ describe('Time scale tests', function() { var ticks = getLabels(xScale); // `bounds === 'data'`: first and last ticks removed since outside the data range - expect(ticks.length).toEqual(217); + expect(ticks.length).toEqual(37); }); it('should accept data as ty points', function() { @@ -233,7 +233,7 @@ describe('Time scale tests', function() { var ticks = getLabels(tScale); // `bounds === 'data'`: first and last ticks removed since outside the data range - expect(ticks.length).toEqual(217); + expect(ticks.length).toEqual(37); }); }); @@ -704,7 +704,7 @@ describe('Time scale tests', function() { it('should get the correct labels for ticks', function() { var labels = getLabels(this.scale); - expect(labels.length).toEqual(61); + expect(labels.length).toEqual(21); expect(labels[0]).toEqual('<8:00:00>'); expect(labels[labels.length - 1]).toEqual('<8:01:00>'); }); @@ -717,7 +717,7 @@ describe('Time scale tests', function() { chart.update(); var labels = getLabels(this.scale); - expect(labels.length).toEqual(61); + expect(labels.length).toEqual(21); expect(labels[0]).toEqual('{8:00:00}'); expect(labels[labels.length - 1]).toEqual('{8:01:00}'); }); @@ -765,7 +765,7 @@ describe('Time scale tests', function() { it('should get the correct labels for major and minor ticks', function() { var labels = getLabels(this.scale); - expect(labels.length).toEqual(61); + expect(labels.length).toEqual(21); expect(labels[0]).toEqual('[[8:00 pm]]'); expect(labels[Math.floor(labels.length / 2)]).toEqual('(8:00:30 pm)'); expect(labels[labels.length - 1]).toEqual('[[8:01 pm]]'); @@ -777,7 +777,7 @@ describe('Time scale tests', function() { chart.update(); var labels = getLabels(this.scale); - expect(labels.length).toEqual(61); + expect(labels.length).toEqual(21); expect(labels[0]).toEqual('(8:00:00 pm)'); expect(labels[labels.length - 1]).toEqual('(8:01:00 pm)'); }); @@ -788,7 +788,7 @@ describe('Time scale tests', function() { chart.update(); var labels = getLabels(this.scale); - expect(labels.length).toEqual(61); + expect(labels.length).toEqual(21); expect(labels[0]).toEqual('<8:00 pm>'); expect(labels[labels.length - 1]).toEqual('<8:01 pm>'); }); @@ -799,7 +799,7 @@ describe('Time scale tests', function() { chart.update(); var labels = getLabels(this.scale); - expect(labels.length).toEqual(61); + expect(labels.length).toEqual(21); expect(labels[0]).toEqual('[[8:00 pm]]'); expect(labels[Math.floor(labels.length / 2)]).toEqual('<8:00:30 pm>'); expect(labels[labels.length - 1]).toEqual('[[8:01 pm]]'); @@ -1469,6 +1469,37 @@ describe('Time scale tests', function() { expect(scale.getPixelForValue('2012')).toBeCloseToPixel(scale.left); expect(scale.getPixelForValue('2051')).toBeCloseToPixel(scale.left + scale.width); }); + }); + }); + }); + + ['data', 'labels'].forEach(function(source) { + ['series', 'linear'].forEach(function(distribution) { + describe('when ticks.source is "' + source + '" and distribution is "' + distribution + '"', function() { + beforeEach(function() { + this.chart = window.acquireChart({ + type: 'line', + data: { + labels: ['2017', '2019', '2020', '2025', '2042'], + datasets: [{data: [0, 1, 2, 3, 4, 5]}] + }, + options: { + scales: { + xAxes: [{ + id: 'x', + type: 'time', + time: { + parser: 'YYYY' + }, + ticks: { + source: source + }, + distribution: distribution + }] + } + } + }); + }); it ('should add offset if min and max extend the labels range and offset is true', function() { var chart = this.chart; @@ -1816,7 +1847,7 @@ describe('Time scale tests', function() { var scale = chart.scales.xScale0; - var labels = scale._ticksToDraw.map(function(t) { + var labels = scale.ticks.map(function(t) { return t.label; }); expect(labels).toEqual([ -- 2.47.2