From 8b1c940280ba73aa50d381098a03ba0063d4b08d Mon Sep 17 00:00:00 2001 From: Zamaroth Date: Mon, 14 Aug 2017 04:37:17 -0700 Subject: [PATCH 1/4] Create gridLines plugin --- src/chart.js | 7 +- src/core/core.scale.js | 82 +++---- src/plugins/plugin.gridlines.js | 308 +++++++++++++++++++++++++ test/specs/core.helpers.tests.js | 15 +- test/specs/scale.category.tests.js | 7 + test/specs/scale.linear.tests.js | 7 + test/specs/scale.logarithmic.tests.js | 7 + test/specs/scale.radialLinear.tests.js | 7 + test/specs/scale.time.tests.js | 7 + 9 files changed, 390 insertions(+), 57 deletions(-) create mode 100644 src/plugins/plugin.gridlines.js diff --git a/src/chart.js b/src/chart.js index df17bb92c3f..c8c910f199d 100644 --- a/src/chart.js +++ b/src/chart.js @@ -52,9 +52,10 @@ require('./charts/Chart.Scatter')(Chart); var plugins = []; plugins.push( - require('./plugins/plugin.filler')(Chart), - require('./plugins/plugin.legend')(Chart), - require('./plugins/plugin.title')(Chart) + require('./plugins/plugin.filler')(Chart), + require('./plugins/plugin.gridlines')(Chart), + require('./plugins/plugin.legend')(Chart), + require('./plugins/plugin.title')(Chart) ); Chart.plugins.register(plugins); diff --git a/src/core/core.scale.js b/src/core/core.scale.js index ffe13cbff82..a1af03e93a7 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -672,7 +672,7 @@ module.exports = function(Chart) { }, // Actually draw the scale on the canvas - // @param {rectangle} chartArea : the area of the chart to draw full grid lines on + // @param {rectangle} chartArea : the area of the chart to draw all ticks and labels on draw: function(chartArea) { var me = this; var options = me.options; @@ -732,7 +732,7 @@ module.exports = function(Chart) { } // Common properties - var tx1, ty1, tx2, ty2, x1, y1, x2, y2, labelX, labelY; + var tx1, ty1, tx2, ty2, labelX, labelY; var textAlign = 'middle'; var textBaseline = 'middle'; var tickPadding = optionTicks.padding; @@ -760,11 +760,9 @@ module.exports = function(Chart) { labelX = me.getPixelForTick(index) + optionTicks.labelOffset; // x values for optionTicks (need to consider offsetLabel option) - tx1 = tx2 = x1 = x2 = xLineValue; + tx1 = tx2 = xLineValue; ty1 = yTickStart; ty2 = yTickEnd; - y1 = chartArea.top; - y2 = chartArea.bottom; } else { var isLeft = options.position === 'left'; var labelXOffset; @@ -789,9 +787,7 @@ module.exports = function(Chart) { tx1 = xTickStart; tx2 = xTickEnd; - x1 = chartArea.left; - x2 = chartArea.right; - ty1 = ty2 = y1 = y2 = yLineValue; + ty1 = ty2 = yLineValue; } itemsToDraw.push({ @@ -799,16 +795,12 @@ module.exports = function(Chart) { ty1: ty1, tx2: tx2, ty2: ty2, - x1: x1, - y1: y1, - x2: x2, - y2: y2, labelX: labelX, labelY: labelY, - glWidth: lineWidth, - glColor: lineColor, - glBorderDash: borderDash, - glBorderDashOffset: borderDashOffset, + tmWidth: lineWidth, + tmColor: lineColor, + tmBorderDash: borderDash, + tmBorderDashOffset: borderDashOffset, rotation: -1 * labelRotationRadians, label: label, major: tick.major, @@ -817,15 +809,30 @@ module.exports = function(Chart) { }); }); - // Draw all of the tick labels, tick marks, and grid lines at the correct places + // When offsetGridLines is enabled, there is one less tick mark than + // there are tick labels, thefore it has to be manually added + if (gridLines.offsetGridLines) { + itemsToDraw.push({ + tx1: !isHorizontal ? xTickStart : chartArea.right, + ty1: !isHorizontal ? chartArea.bottom : yTickStart, + tx2: !isHorizontal ? xTickEnd : chartArea.right, + ty2: !isHorizontal ? chartArea.bottom : yTickEnd, + tmWidth: gridLines.lineWidth, + tmColor: gridLines.color, + tmBorderDash: gridLines.borderDash, + tmBorderDashOffset: gridLines.borderDashOffset + }); + } + + // Draw all of the tick labels and tick marks at the correct places helpers.each(itemsToDraw, function(itemToDraw) { if (gridLines.display) { context.save(); - context.lineWidth = itemToDraw.glWidth; - context.strokeStyle = itemToDraw.glColor; + context.lineWidth = itemToDraw.tmWidth; + context.strokeStyle = itemToDraw.tmColor; if (context.setLineDash) { - context.setLineDash(itemToDraw.glBorderDash); - context.lineDashOffset = itemToDraw.glBorderDashOffset; + context.setLineDash(itemToDraw.tmBorderDash); + context.lineDashOffset = itemToDraw.tmBorderDashOffset; } context.beginPath(); @@ -835,16 +842,11 @@ module.exports = function(Chart) { context.lineTo(itemToDraw.tx2, itemToDraw.ty2); } - if (gridLines.drawOnChartArea) { - context.moveTo(itemToDraw.x1, itemToDraw.y1); - context.lineTo(itemToDraw.x2, itemToDraw.y2); - } - context.stroke(); context.restore(); } - if (optionTicks.display) { + if (optionTicks.display && itemToDraw.label !== undefined && itemToDraw.label !== '') { // Make sure we draw text in the correct color and font context.save(); context.translate(itemToDraw.labelX, itemToDraw.labelY); @@ -900,32 +902,6 @@ module.exports = function(Chart) { context.fillText(scaleLabel.labelString, 0, 0); context.restore(); } - - if (gridLines.drawBorder) { - // Draw the line at the edge of the axis - context.lineWidth = helpers.valueAtIndexOrDefault(gridLines.lineWidth, 0); - context.strokeStyle = helpers.valueAtIndexOrDefault(gridLines.color, 0); - var x1 = me.left; - var x2 = me.right; - var y1 = me.top; - var y2 = me.bottom; - - var aliasPixel = helpers.aliasPixel(context.lineWidth); - if (isHorizontal) { - y1 = y2 = options.position === 'top' ? me.bottom : me.top; - y1 += aliasPixel; - y2 += aliasPixel; - } else { - x1 = x2 = options.position === 'left' ? me.right : me.left; - x1 += aliasPixel; - x2 += aliasPixel; - } - - context.beginPath(); - context.moveTo(x1, y1); - context.lineTo(x2, y2); - context.stroke(); - } } }); }; diff --git a/src/plugins/plugin.gridlines.js b/src/plugins/plugin.gridlines.js new file mode 100644 index 00000000000..33f642712e2 --- /dev/null +++ b/src/plugins/plugin.gridlines.js @@ -0,0 +1,308 @@ +'use strict'; + +// Undefined border is a border not explictly set by any axis. Four undefined borders are always created around +// the chartArea. If an axis has border.display set to true, a defined border will be created for that axis, if +// it's set to false an undefined border will be created. +// If there is an undefined and a defined border overlapping each other, only the defined will be drawn. +// +// Simplified, undefined border is some kind of placeholder for every border, which is replaced by a defined one, +// if such border overlapping it exists, guarateeing there will be always a line, even if the border isn't displayed. + +module.exports = function(Chart) { + var helpers = Chart.helpers; + var globalDefaults = Chart.defaults.global; + + Chart.defaults.scale.border = { + display: false, + color: 'rgba(0, 0, 0, 0.4)', + lineWidth: 1, + borderDash: [], + borderDashOffset: 0.0 + }; + + function bordersOverlap(b) { + return b.undefinedBorder === false + && b.x1 === this.x1 + && b.x2 === this.x2 + && b.y1 === this.y1 + && b.y2 === this.y2; + } + + function drawLine(context, x1, x2, y1, y2) { + var aliasPixel = helpers.aliasPixel(context.lineWidth); + + if (y1 === y2) { + y1 += aliasPixel; + y2 += aliasPixel; + } else { + x1 += aliasPixel; + x2 += aliasPixel; + } + + context.beginPath(); + + context.moveTo(x1, y1); + context.lineTo(x2, y2); + + context.stroke(); + context.restore(); + } + + function getChartAreaBorder(index, scale, chartArea) { + var gridLines = scale.options.gridLines; + + var isHorizontal = scale.isHorizontal(); + + var border = { + x1: chartArea.left, + x2: chartArea.right, + y1: chartArea.top, + y2: chartArea.bottom, + isHorizontal: isHorizontal, + undefinedBorder: true + }; + + if (!isHorizontal) { + border.y1 = border.y2 = (index === 0 ? chartArea.top : chartArea.bottom); + } else { + border.x1 = border.x2 = (index === 0 ? chartArea.left : chartArea.right); + } + + // If this gridLine is zeroLine, include zeroLine properties + if ((typeof scale.zeroLineIndex !== 'undefined' ? scale.zeroLineIndex : 0) === index) { + border.forceUseStyle = true; + border.lineWidth = gridLines.zeroLineWidth; + border.lineColor = gridLines.zeroLineColor; + border.borderDash = gridLines.zeroLineBorderDash; + border.borderDashOffset = gridLines.zeroLineBorderDashOffset; + } + + return border; + } + + function getChartAreaBorders(scale, chartArea, gridLinesCount, undefinedBorderOptions) { + var gridLines = scale.options.gridLines; + var isHorizontal = scale.isHorizontal(); + + // The undefinedBorder variables are used to determine if the undefinedBorders for the specific + // direction(determined by isHorizontal) is already set, as every direction can only have them set once. + if (isHorizontal && undefinedBorderOptions.horizontal === undefined) { + undefinedBorderOptions.horizontal = { + lineWidth: gridLines.lineWidth, + lineColor: gridLines.color, + borderDash: gridLines.borderDash, + borderDashOffset: gridLines.borderDashOffset + }; + } else if (!isHorizontal && undefinedBorderOptions.vertical === undefined) { + undefinedBorderOptions.vertical = { + lineWidth: gridLines.lineWidth, + lineColor: gridLines.color, + borderDash: gridLines.borderDash, + borderDashOffset: gridLines.borderDashOffset + }; + } else { + // Ensures that every chartArea border is added only once + return undefined; + } + + // Get the first and last gridLine as undefined borders + return [getChartAreaBorder(0, scale, chartArea), getChartAreaBorder(gridLinesCount-1, scale, chartArea)]; + } + + function getScaleBorder(scale, borderOptions) { + // Get position of the border + var bx1 = scale.left, + bx2 = scale.right, + by1 = scale.top, + by2 = scale.bottom; + + if (scale.isHorizontal()) { + by1 = by2 = scale.position === 'top' ? scale.bottom : scale.top; + } else { + bx1 = bx2 = scale.position === 'left' ? scale.right : scale.left; + } + + // Axis border(the line near ticks) is defined when border.display option is true or undefined otherwise + // Undefined borders may not be drawn in the end if there is any defined border overlapping them, + // therefore they must be drawn after all the scales are iterated + // The direction has to be flipped because the border line of an axis has the opposite direction than + // its gridLines. For visual explanation check the issue #4041 + return { + x1: bx1, + x2: bx2, + y1: by1, + y2: by2, + isHorizontal: !scale.isHorizontal(), + lineWidth: borderOptions.lineWidth, + lineColor: borderOptions.color, + borderDash: helpers.getValueOrDefault(borderOptions.borderDash, globalDefaults.borderDash), + borderDashOffset: helpers.getValueOrDefault(borderOptions.borderDashOffset, globalDefaults.borderDashOffset), + undefinedBorder: !borderOptions.display + }; + } + + function drawGridLines(chart, scale, undefinedBorderOptions, bordersToDraw) { + var gridLines = scale.options.gridLines; + if (!gridLines.display) { + return; + } + + var context = chart.ctx; + var chartArea = chart.chartArea; + + var isHorizontal = scale.isHorizontal(); + + var lineWidth, lineColor, borderDash, borderDashOffset; + + // When gridLines.offsetGridLines is enabled, there is one less tick than + // there should be gridLines, so we have to take that into account + var gridLinesCount = scale.ticks.length + (gridLines.offsetGridLines ? 1 : 0); + + // First and last gridLine of first found axis for each direction are marked as undefined borders. + // First and last gridLines are never drawn for any scale to ensure that there won't be any overlapping on chartArea borders. + // All four sides of chartArea have to be marked, to be sure there will always be some border. If the border is defined by + // any axis, that border will be preferred later + // getChartAreaBorder returns undefined when chartArea border for this direction were already marked + var chartAreaBorders = getChartAreaBorders(scale, chartArea, gridLinesCount, undefinedBorderOptions); + if (chartAreaBorders !== undefined) { + Array.prototype.push.apply(bordersToDraw, chartAreaBorders); + } + + // Draw all gridLines excluding the first and the last + for (var index = 1; index < gridLinesCount - 1; index++) { + if (scale.ticks[index] === null || scale.ticks[index] === undefined) { + continue; + } + + // Set visual settings for current gridLine + if (index === (typeof scale.zeroLineIndex !== 'undefined' ? scale.zeroLineIndex : 0)) { + lineWidth = gridLines.zeroLineWidth; + lineColor = gridLines.zeroLineColor; + borderDash = gridLines.zeroLineBorderDash; + borderDashOffset = gridLines.zeroLineBorderDashOffset; + } else { + lineWidth = helpers.getValueAtIndexOrDefault(gridLines.lineWidth, index); + lineColor = helpers.getValueAtIndexOrDefault(gridLines.color, index); + borderDash = helpers.getValueOrDefault(gridLines.borderDash, globalDefaults.borderDash); + borderDashOffset = helpers.getValueOrDefault(gridLines.borderDashOffset, globalDefaults.borderDashOffset); + } + + // Get position of current gridLine + var x1, x2, y1, y2; + if (isHorizontal) { + var xLineValue = scale.getPixelForTick(index); + + x1 = x2 = xLineValue; + y1 = chartArea.top; + y2 = chartArea.bottom; + } else { + var yLineValue = scale.getPixelForTick(index); + + x1 = chartArea.left; + x2 = chartArea.right; + y1 = y2 = yLineValue; + } + + context.lineWidth = lineWidth; + context.strokeStyle = lineColor; + if (context.setLineDash) { + context.setLineDash(borderDash); + context.lineDashOffset = borderDashOffset; + } + + // Draw current gridLine + drawLine(context, x1, x2, y1, y2); + } + } + + function drawBorders(context, bordersToDraw, undefinedBorderOptions) { + // Draws all the borders. Skips undefined orders which would be overlapped by a defined border + helpers.each(bordersToDraw, function(borderToDraw) { + // When the border is undefined, check if there isn't a defined border on the same place, if there is one dont draw this border + if (!borderToDraw.undefinedBorder || bordersToDraw.findIndex(bordersOverlap, borderToDraw) === -1) { + context.save(); + + var _undefinedBorderOptions = borderToDraw.isHorizontal ? undefinedBorderOptions.horizontal : undefinedBorderOptions.vertical; + + // Use given properties if border is defined or they were explictly set(e.g. when the undefined border is also a zeroLine) + // Otherwise use default properties for undefined borders + var useGivenOptions = !borderToDraw.undefinedBorder || borderToDraw.forceUseStyle; + + context.lineWidth = useGivenOptions ? borderToDraw.lineWidth : _undefinedBorderOptions.lineWidth; + context.strokeStyle = useGivenOptions ? borderToDraw.lineColor : _undefinedBorderOptions.lineColor; + if (context.setLineDash) { + context.setLineDash(useGivenOptions ? borderToDraw.borderDash : _undefinedBorderOptions.borderDash); + context.lineDashOffset = useGivenOptions ? borderToDraw.borderDashOffset : _undefinedBorderOptions.borderDashOffset; + } + + // Draw the border + drawLine(context, borderToDraw.x1, borderToDraw.x2, borderToDraw.y1, borderToDraw.y2); + + // If there are no defined borders overlapping this undefined border, mark it + // as defined to prevent other undefined borders to be drawn over it + if (borderToDraw.undefinedBorder) { + borderToDraw.undefinedBorder = false; + } + } + }); + } + + function setUndefinedBorderOptionsToDefault(undefinedBorderOptions) { + var gridLineDefaults = Chart.defaults.scale.gridLines; + + if (undefinedBorderOptions.horizontal === undefined) { + undefinedBorderOptions.horizontal = { + lineWidth: gridLineDefaults.lineWidth, + lineColor: gridLineDefaults.color, + borderDash: gridLineDefaults.borderDash, + borderDashOffset: gridLineDefaults.borderDashOffset + }; + } + + if (undefinedBorderOptions.vertical === undefined) { + undefinedBorderOptions.vertical = { + lineWidth: gridLineDefaults.lineWidth, + lineColor: gridLineDefaults.color, + borderDash: gridLineDefaults.borderDash, + borderDashOffset: gridLineDefaults.borderDashOffset + }; + } + } + + return { + id: 'gridlines', + + beforeDatasetsDraw: function(chart) { + // Shut down the plugin if the chart is radar or polarArea + if (chart.scale !== undefined && chart.scale.options.type === 'radialLinear') { + return; + } + + var bordersToDraw = []; + + // Properties used by all undefined borders. They are set by the first axis of the corresponding + // orientation (y = horizontal, x = vertical) + var undefinedBorderOptions = {horizontal: undefined, vertical: undefined}; + + helpers.each(chart.scales, function(scale) { + var scaleOptions = scale.options; + + if (scaleOptions.display) { + // Draw gridLines and mark undefined borders + drawGridLines(chart, scale, undefinedBorderOptions, bordersToDraw); + + // Adds the border of this scale to the array to be drawn later after all the gridLines are drawn + if (scaleOptions.gridLines.display || scaleOptions.border.display) { + bordersToDraw.push(getScaleBorder(scale, scaleOptions.border)); + } + } + }); + + // Set common undefinedBorder properties to default gridLines options if no axis for the specific + // direction was found + setUndefinedBorderOptionsToDefault(undefinedBorderOptions); + + drawBorders(chart.ctx, bordersToDraw, undefinedBorderOptions); + } + }; +}; diff --git a/test/specs/core.helpers.tests.js b/test/specs/core.helpers.tests.js index dc865fb01b2..30da4b2f1bf 100644 --- a/test/specs/core.helpers.tests.js +++ b/test/specs/core.helpers.tests.js @@ -109,7 +109,13 @@ describe('Core helper tests', function() { axisProp: 456 }, { display: true, - + border: { + display: false, + color: 'rgba(0, 0, 0, 0.4)', + lineWidth: 1, + borderDash: [], + borderDashOffset: 0.0 + }, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, @@ -148,6 +154,13 @@ describe('Core helper tests', function() { }, { display: true, + border: { + display: false, + color: 'rgba(0, 0, 0, 0.4)', + lineWidth: 1, + borderDash: [], + borderDashOffset: 0.0 + }, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, diff --git a/test/specs/scale.category.tests.js b/test/specs/scale.category.tests.js index 1665235ae8d..7ce71662d21 100644 --- a/test/specs/scale.category.tests.js +++ b/test/specs/scale.category.tests.js @@ -12,6 +12,13 @@ describe('Category scale tests', function() { expect(defaultConfig).toEqual({ display: true, + border: { + display: false, + color: 'rgba(0, 0, 0, 0.4)', + lineWidth: 1, + borderDash: [], + borderDashOffset: 0.0 + }, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index 60dd07698c0..1b07f4b3c25 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -10,6 +10,13 @@ describe('Linear Scale', function() { expect(defaultConfig).toEqual({ display: true, + border: { + display: false, + color: 'rgba(0, 0, 0, 0.4)', + lineWidth: 1, + borderDash: [], + borderDashOffset: 0.0 + }, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js index 9640b1d3d62..79bb47f853d 100644 --- a/test/specs/scale.logarithmic.tests.js +++ b/test/specs/scale.logarithmic.tests.js @@ -9,6 +9,13 @@ describe('Logarithmic Scale tests', function() { var defaultConfig = Chart.scaleService.getScaleDefaults('logarithmic'); expect(defaultConfig).toEqual({ display: true, + border: { + display: false, + color: 'rgba(0, 0, 0, 0.4)', + lineWidth: 1, + borderDash: [], + borderDashOffset: 0.0 + }, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, diff --git a/test/specs/scale.radialLinear.tests.js b/test/specs/scale.radialLinear.tests.js index 707ce0b7c52..7ba2f104ef6 100644 --- a/test/specs/scale.radialLinear.tests.js +++ b/test/specs/scale.radialLinear.tests.js @@ -16,6 +16,13 @@ describe('Test the radial linear scale', function() { }, animate: true, display: true, + border: { + display: false, + color: 'rgba(0, 0, 0, 0.4)', + lineWidth: 1, + borderDash: [], + borderDashOffset: 0.0 + }, gridLines: { circular: false, color: 'rgba(0, 0, 0, 0.1)', diff --git a/test/specs/scale.time.tests.js b/test/specs/scale.time.tests.js index 19189040ee4..cfefcb93790 100755 --- a/test/specs/scale.time.tests.js +++ b/test/specs/scale.time.tests.js @@ -55,6 +55,13 @@ describe('Time scale tests', function() { var defaultConfig = Chart.scaleService.getScaleDefaults('time'); expect(defaultConfig).toEqual({ display: true, + border: { + display: false, + color: 'rgba(0, 0, 0, 0.4)', + lineWidth: 1, + borderDash: [], + borderDashOffset: 0.0 + }, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, From 5598a8f892d8cb381af79f7b3dd045edc049355f Mon Sep 17 00:00:00 2001 From: Zamaroth Date: Sun, 13 Aug 2017 21:57:45 +0200 Subject: [PATCH 2/4] Refactor plugin.gridlines.js --- src/core/core.scale.js | 51 +++- src/plugins/plugin.gridlines.js | 364 ++++++++----------------- test/specs/core.helpers.tests.js | 23 +- test/specs/scale.category.tests.js | 12 +- test/specs/scale.linear.tests.js | 12 +- test/specs/scale.logarithmic.tests.js | 11 +- test/specs/scale.radialLinear.tests.js | 11 +- test/specs/scale.time.tests.js | 11 +- 8 files changed, 184 insertions(+), 311 deletions(-) diff --git a/src/core/core.scale.js b/src/core/core.scale.js index a1af03e93a7..7f500e895dc 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -9,6 +9,10 @@ defaults._set('scale', { display: true, position: 'left', offset: false, + borderColor: 'rgba(0, 0, 0, 0.4)', + borderWidth: 0, + borderDash: [], + borderDashOffset: 0.0, // grid line settings gridLines: { @@ -809,14 +813,15 @@ module.exports = function(Chart) { }); }); - // When offsetGridLines is enabled, there is one less tick mark than - // there are tick labels, thefore it has to be manually added + // When offsetGridLines is enabled, there should be one more tick mark than there + // are ticks in scale.ticks array, thefore the missing gridLine has to be manually added if (gridLines.offsetGridLines) { + var aliasPixel = helpers.aliasPixel(gridLines.lineWidth); itemsToDraw.push({ - tx1: !isHorizontal ? xTickStart : chartArea.right, - ty1: !isHorizontal ? chartArea.bottom : yTickStart, - tx2: !isHorizontal ? xTickEnd : chartArea.right, - ty2: !isHorizontal ? chartArea.bottom : yTickEnd, + tx1: !isHorizontal ? xTickStart : chartArea.right + aliasPixel, + ty1: !isHorizontal ? chartArea.bottom + aliasPixel : yTickStart, + tx2: !isHorizontal ? xTickEnd : chartArea.right + aliasPixel, + ty2: !isHorizontal ? chartArea.bottom + aliasPixel : yTickEnd, tmWidth: gridLines.lineWidth, tmColor: gridLines.color, tmBorderDash: gridLines.borderDash, @@ -902,6 +907,40 @@ module.exports = function(Chart) { context.fillText(scaleLabel.labelString, 0, 0); context.restore(); } + + // gridLines.drawBorder is deprecated + if (gridLines.drawBorder && options.borderColor !== null && options.borderWidth !== 0 && options.borderWidth !== null) { + // Draw the scale border + context.lineWidth = options.borderWidth; + context.strokeStyle = options.borderColor; + if (context.setLineDash) { + context.setLineDash(helpers.getValueOrDefault(options.borderDash, globalDefaults.borderDash)); + context.lineDashOffset = helpers.getValueOrDefault(options.borderDashOffset, globalDefaults.borderDashOffset); + } + + var x1 = Math.round(me.left); + var x2 = Math.round(me.right); + var y1 = Math.round(me.top); + var y2 = Math.round(me.bottom); + + if (isHorizontal) { + y1 = y2 = options.position === 'top' ? me.bottom : me.top; + y1 += helpers.aliasPixel(context.lineWidth); + y2 += helpers.aliasPixel(context.lineWidth); + } else { + x1 = x2 = options.position === 'left' ? me.right : me.left; + x1 += helpers.aliasPixel(context.lineWidth); + x2 += helpers.aliasPixel(context.lineWidth); + } + + context.beginPath(); + + context.moveTo(x1, y1); + context.lineTo(x2, y2); + + context.stroke(); + context.restore(); + } } }); }; diff --git a/src/plugins/plugin.gridlines.js b/src/plugins/plugin.gridlines.js index 33f642712e2..d30df6ef676 100644 --- a/src/plugins/plugin.gridlines.js +++ b/src/plugins/plugin.gridlines.js @@ -1,308 +1,166 @@ 'use strict'; -// Undefined border is a border not explictly set by any axis. Four undefined borders are always created around -// the chartArea. If an axis has border.display set to true, a defined border will be created for that axis, if -// it's set to false an undefined border will be created. -// If there is an undefined and a defined border overlapping each other, only the defined will be drawn. -// -// Simplified, undefined border is some kind of placeholder for every border, which is replaced by a defined one, -// if such border overlapping it exists, guarateeing there will be always a line, even if the border isn't displayed. - module.exports = function(Chart) { var helpers = Chart.helpers; var globalDefaults = Chart.defaults.global; - Chart.defaults.scale.border = { - display: false, - color: 'rgba(0, 0, 0, 0.4)', - lineWidth: 1, - borderDash: [], - borderDashOffset: 0.0 - }; - - function bordersOverlap(b) { - return b.undefinedBorder === false - && b.x1 === this.x1 - && b.x2 === this.x2 - && b.y1 === this.y1 - && b.y2 === this.y2; - } - - function drawLine(context, x1, x2, y1, y2) { - var aliasPixel = helpers.aliasPixel(context.lineWidth); - - if (y1 === y2) { - y1 += aliasPixel; - y2 += aliasPixel; - } else { - x1 += aliasPixel; - x2 += aliasPixel; - } - - context.beginPath(); - - context.moveTo(x1, y1); - context.lineTo(x2, y2); - - context.stroke(); - context.restore(); - } - - function getChartAreaBorder(index, scale, chartArea) { - var gridLines = scale.options.gridLines; - - var isHorizontal = scale.isHorizontal(); - - var border = { - x1: chartArea.left, - x2: chartArea.right, - y1: chartArea.top, - y2: chartArea.bottom, - isHorizontal: isHorizontal, - undefinedBorder: true - }; - - if (!isHorizontal) { - border.y1 = border.y2 = (index === 0 ? chartArea.top : chartArea.bottom); - } else { - border.x1 = border.x2 = (index === 0 ? chartArea.left : chartArea.right); - } + // Stores all gridLines that should be displayed + var lines = []; - // If this gridLine is zeroLine, include zeroLine properties - if ((typeof scale.zeroLineIndex !== 'undefined' ? scale.zeroLineIndex : 0) === index) { - border.forceUseStyle = true; - border.lineWidth = gridLines.zeroLineWidth; - border.lineColor = gridLines.zeroLineColor; - border.borderDash = gridLines.zeroLineBorderDash; - border.borderDashOffset = gridLines.zeroLineBorderDashOffset; - } + function getGridLine(chart, scale, position, tickIndex) { + var chartArea = chart.chartArea; + var scaleOptions = scale.options; + var gridLines = scaleOptions.gridLines; - return border; - } + var lineWidth, lineColor, borderDash, borderDashOffset; - function getChartAreaBorders(scale, chartArea, gridLinesCount, undefinedBorderOptions) { - var gridLines = scale.options.gridLines; - var isHorizontal = scale.isHorizontal(); - - // The undefinedBorder variables are used to determine if the undefinedBorders for the specific - // direction(determined by isHorizontal) is already set, as every direction can only have them set once. - if (isHorizontal && undefinedBorderOptions.horizontal === undefined) { - undefinedBorderOptions.horizontal = { - lineWidth: gridLines.lineWidth, - lineColor: gridLines.color, - borderDash: gridLines.borderDash, - borderDashOffset: gridLines.borderDashOffset - }; - } else if (!isHorizontal && undefinedBorderOptions.vertical === undefined) { - undefinedBorderOptions.vertical = { - lineWidth: gridLines.lineWidth, - lineColor: gridLines.color, - borderDash: gridLines.borderDash, - borderDashOffset: gridLines.borderDashOffset - }; + if (tickIndex === (typeof scale.zeroLineIndex !== 'undefined' ? scale.zeroLineIndex : 0)) { + lineWidth = gridLines.zeroLineWidth; + lineColor = gridLines.zeroLineColor; + borderDash = gridLines.zeroLineBorderDash; + borderDashOffset = gridLines.zeroLineBorderDashOffset; } else { - // Ensures that every chartArea border is added only once - return undefined; + lineWidth = helpers.getValueAtIndexOrDefault(gridLines.lineWidth, tickIndex); + lineColor = helpers.getValueAtIndexOrDefault(gridLines.color, tickIndex); + borderDash = helpers.getValueOrDefault(gridLines.borderDash, globalDefaults.borderDash); + borderDashOffset = helpers.getValueOrDefault(gridLines.borderDashOffset, globalDefaults.borderDashOffset); } - // Get the first and last gridLine as undefined borders - return [getChartAreaBorder(0, scale, chartArea), getChartAreaBorder(gridLinesCount-1, scale, chartArea)]; - } - - function getScaleBorder(scale, borderOptions) { - // Get position of the border - var bx1 = scale.left, - bx2 = scale.right, - by1 = scale.top, - by2 = scale.bottom; + var x1, x2, y1, y2; if (scale.isHorizontal()) { - by1 = by2 = scale.position === 'top' ? scale.bottom : scale.top; + x1 = x2 = position; + y1 = chartArea.top; + y2 = chartArea.bottom; } else { - bx1 = bx2 = scale.position === 'left' ? scale.right : scale.left; + x1 = chartArea.left; + x2 = chartArea.right; + y1 = y2 = position; } - // Axis border(the line near ticks) is defined when border.display option is true or undefined otherwise - // Undefined borders may not be drawn in the end if there is any defined border overlapping them, - // therefore they must be drawn after all the scales are iterated - // The direction has to be flipped because the border line of an axis has the opposite direction than - // its gridLines. For visual explanation check the issue #4041 return { - x1: bx1, - x2: bx2, - y1: by1, - y2: by2, - isHorizontal: !scale.isHorizontal(), - lineWidth: borderOptions.lineWidth, - lineColor: borderOptions.color, - borderDash: helpers.getValueOrDefault(borderOptions.borderDash, globalDefaults.borderDash), - borderDashOffset: helpers.getValueOrDefault(borderOptions.borderDashOffset, globalDefaults.borderDashOffset), - undefinedBorder: !borderOptions.display + x1: x1, + x2: x2, + y1: y1, + y2: y2, + width: lineWidth, + color: lineColor, + borderDash: borderDash, + borderDashOffset: borderDashOffset }; } - function drawGridLines(chart, scale, undefinedBorderOptions, bordersToDraw) { - var gridLines = scale.options.gridLines; - if (!gridLines.display) { - return; - } - - var context = chart.ctx; - var chartArea = chart.chartArea; - - var isHorizontal = scale.isHorizontal(); - - var lineWidth, lineColor, borderDash, borderDashOffset; - - // When gridLines.offsetGridLines is enabled, there is one less tick than - // there should be gridLines, so we have to take that into account - var gridLinesCount = scale.ticks.length + (gridLines.offsetGridLines ? 1 : 0); - - // First and last gridLine of first found axis for each direction are marked as undefined borders. - // First and last gridLines are never drawn for any scale to ensure that there won't be any overlapping on chartArea borders. - // All four sides of chartArea have to be marked, to be sure there will always be some border. If the border is defined by - // any axis, that border will be preferred later - // getChartAreaBorder returns undefined when chartArea border for this direction were already marked - var chartAreaBorders = getChartAreaBorders(scale, chartArea, gridLinesCount, undefinedBorderOptions); - if (chartAreaBorders !== undefined) { - Array.prototype.push.apply(bordersToDraw, chartAreaBorders); - } + function collectVisibleGridLines(chart) { + lines = []; - // Draw all gridLines excluding the first and the last - for (var index = 1; index < gridLinesCount - 1; index++) { - if (scale.ticks[index] === null || scale.ticks[index] === undefined) { - continue; - } + // Temporary object that stores already collected gridLine positions to check and prevent gridLines from overlapping + var gridLinesHash = { + horizontal: {}, + vertical: {} + }; - // Set visual settings for current gridLine - if (index === (typeof scale.zeroLineIndex !== 'undefined' ? scale.zeroLineIndex : 0)) { - lineWidth = gridLines.zeroLineWidth; - lineColor = gridLines.zeroLineColor; - borderDash = gridLines.zeroLineBorderDash; - borderDashOffset = gridLines.zeroLineBorderDashOffset; - } else { - lineWidth = helpers.getValueAtIndexOrDefault(gridLines.lineWidth, index); - lineColor = helpers.getValueAtIndexOrDefault(gridLines.color, index); - borderDash = helpers.getValueOrDefault(gridLines.borderDash, globalDefaults.borderDash); - borderDashOffset = helpers.getValueOrDefault(gridLines.borderDashOffset, globalDefaults.borderDashOffset); - } + // Marks scale border positions to prevent overlapping of gridLines and scale borders + helpers.each(chart.scales, function(scale) { + var scaleOptions = scale.options; - // Get position of current gridLine - var x1, x2, y1, y2; - if (isHorizontal) { - var xLineValue = scale.getPixelForTick(index); + // gridLines.drawBorder is deprecated + if (scaleOptions.gridLines.drawBorder && scaleOptions.borderColor !== null && scaleOptions.borderWidth !== 0 && scaleOptions.borderWidth !== null) { + var glHashByOrientantion = gridLinesHash[!scale.isHorizontal() ? 'horizontal' : 'vertical']; + var borderPosition; - x1 = x2 = xLineValue; - y1 = chartArea.top; - y2 = chartArea.bottom; - } else { - var yLineValue = scale.getPixelForTick(index); + if (scale.isHorizontal()) { + borderPosition = scale.position === 'top' ? scale.bottom : scale.top; + } else { + borderPosition = scale.position === 'left' ? scale.right : scale.left; + } - x1 = chartArea.left; - x2 = chartArea.right; - y1 = y2 = yLineValue; + glHashByOrientantion[Math.round(borderPosition)] = true; } + }); - context.lineWidth = lineWidth; - context.strokeStyle = lineColor; - if (context.setLineDash) { - context.setLineDash(borderDash); - context.lineDashOffset = borderDashOffset; - } + // Collects gridLines + helpers.each(chart.scales, function(scale) { + var scaleOptions = scale.options; - // Draw current gridLine - drawLine(context, x1, x2, y1, y2); - } - } + if (scaleOptions.display && scaleOptions.gridLines.display && scaleOptions.gridLines.drawOnChartArea) { + var glHashByOrientantion = gridLinesHash[scale.isHorizontal() ? 'horizontal' : 'vertical']; + var position; - function drawBorders(context, bordersToDraw, undefinedBorderOptions) { - // Draws all the borders. Skips undefined orders which would be overlapped by a defined border - helpers.each(bordersToDraw, function(borderToDraw) { - // When the border is undefined, check if there isn't a defined border on the same place, if there is one dont draw this border - if (!borderToDraw.undefinedBorder || bordersToDraw.findIndex(bordersOverlap, borderToDraw) === -1) { - context.save(); + for (var tickIndex = 0; tickIndex < scale.ticks.length; tickIndex++) { + var tick = scale.ticks[tickIndex]; - var _undefinedBorderOptions = borderToDraw.isHorizontal ? undefinedBorderOptions.horizontal : undefinedBorderOptions.vertical; + if (tick === null || tick === undefined) { + continue; + } - // Use given properties if border is defined or they were explictly set(e.g. when the undefined border is also a zeroLine) - // Otherwise use default properties for undefined borders - var useGivenOptions = !borderToDraw.undefinedBorder || borderToDraw.forceUseStyle; + position = scale.getPixelForTick(tickIndex); - context.lineWidth = useGivenOptions ? borderToDraw.lineWidth : _undefinedBorderOptions.lineWidth; - context.strokeStyle = useGivenOptions ? borderToDraw.lineColor : _undefinedBorderOptions.lineColor; - if (context.setLineDash) { - context.setLineDash(useGivenOptions ? borderToDraw.borderDash : _undefinedBorderOptions.borderDash); - context.lineDashOffset = useGivenOptions ? borderToDraw.borderDashOffset : _undefinedBorderOptions.borderDashOffset; + if (glHashByOrientantion[position] === undefined) { + glHashByOrientantion[position] = true; + lines.push(getGridLine(chart, scale, position, tickIndex)); + } } - // Draw the border - drawLine(context, borderToDraw.x1, borderToDraw.x2, borderToDraw.y1, borderToDraw.y2); + // When offsetGridLines is enabled, there should be one more gridLine than there + // are ticks in scale.ticks array, thefore the missing gridLine has to be manually added + if (scaleOptions.gridLines.offsetGridLines) { + position = Math.round(!scale.isHorizontal() ? scale.bottom : scale.right); - // If there are no defined borders overlapping this undefined border, mark it - // as defined to prevent other undefined borders to be drawn over it - if (borderToDraw.undefinedBorder) { - borderToDraw.undefinedBorder = false; + if (glHashByOrientantion[position] === undefined) { + glHashByOrientantion[position] = true; + lines.push(getGridLine(chart, scale, position, undefined)); + } } } }); } - function setUndefinedBorderOptionsToDefault(undefinedBorderOptions) { - var gridLineDefaults = Chart.defaults.scale.gridLines; + function drawGridLines(chart) { + var context = chart.ctx; - if (undefinedBorderOptions.horizontal === undefined) { - undefinedBorderOptions.horizontal = { - lineWidth: gridLineDefaults.lineWidth, - lineColor: gridLineDefaults.color, - borderDash: gridLineDefaults.borderDash, - borderDashOffset: gridLineDefaults.borderDashOffset - }; - } + for (var i = 0; i < lines.length; i++) { + var line = lines[i]; - if (undefinedBorderOptions.vertical === undefined) { - undefinedBorderOptions.vertical = { - lineWidth: gridLineDefaults.lineWidth, - lineColor: gridLineDefaults.color, - borderDash: gridLineDefaults.borderDash, - borderDashOffset: gridLineDefaults.borderDashOffset - }; - } - } + context.lineWidth = line.width; + context.strokeStyle = line.color; + if (context.setLineDash) { + context.setLineDash(line.borderDash); + context.lineDashOffset = line.borderDashOffset; + } - return { - id: 'gridlines', + var aliasPixel = helpers.aliasPixel(context.lineWidth); + var x1 = line.x1; + var x2 = line.x2; + var y1 = line.y1; + var y2 = line.y2; - beforeDatasetsDraw: function(chart) { - // Shut down the plugin if the chart is radar or polarArea - if (chart.scale !== undefined && chart.scale.options.type === 'radialLinear') { - return; + if (y1 === y2) { + y1 += aliasPixel; + y2 += aliasPixel; + } else { + x1 += aliasPixel; + x2 += aliasPixel; } - var bordersToDraw = []; - - // Properties used by all undefined borders. They are set by the first axis of the corresponding - // orientation (y = horizontal, x = vertical) - var undefinedBorderOptions = {horizontal: undefined, vertical: undefined}; + context.beginPath(); - helpers.each(chart.scales, function(scale) { - var scaleOptions = scale.options; + context.moveTo(x1, y1); + context.lineTo(x2, y2); - if (scaleOptions.display) { - // Draw gridLines and mark undefined borders - drawGridLines(chart, scale, undefinedBorderOptions, bordersToDraw); + context.stroke(); + context.restore(); + } + } - // Adds the border of this scale to the array to be drawn later after all the gridLines are drawn - if (scaleOptions.gridLines.display || scaleOptions.border.display) { - bordersToDraw.push(getScaleBorder(scale, scaleOptions.border)); - } - } - }); + return { + id: 'gridlines', - // Set common undefinedBorder properties to default gridLines options if no axis for the specific - // direction was found - setUndefinedBorderOptionsToDefault(undefinedBorderOptions); + afterUpdate: function(chart) { + collectVisibleGridLines(chart); + }, - drawBorders(chart.ctx, bordersToDraw, undefinedBorderOptions); + beforeDraw: function(chart) { + drawGridLines(chart); } }; }; diff --git a/test/specs/core.helpers.tests.js b/test/specs/core.helpers.tests.js index 30da4b2f1bf..fcad459ae4b 100644 --- a/test/specs/core.helpers.tests.js +++ b/test/specs/core.helpers.tests.js @@ -109,13 +109,10 @@ describe('Core helper tests', function() { axisProp: 456 }, { display: true, - border: { - display: false, - color: 'rgba(0, 0, 0, 0.4)', - lineWidth: 1, - borderDash: [], - borderDashOffset: 0.0 - }, + borderColor: 'rgba(0, 0, 0, 0.4)', + borderWidth: 0, + borderDash: [], + borderDashOffset: 0.0, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, @@ -153,14 +150,10 @@ describe('Core helper tests', function() { type: 'linear' }, { display: true, - - border: { - display: false, - color: 'rgba(0, 0, 0, 0.4)', - lineWidth: 1, - borderDash: [], - borderDashOffset: 0.0 - }, + borderColor: 'rgba(0, 0, 0, 0.4)', + borderWidth: 0, + borderDash: [], + borderDashOffset: 0.0, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, diff --git a/test/specs/scale.category.tests.js b/test/specs/scale.category.tests.js index 7ce71662d21..039e9b9cd7a 100644 --- a/test/specs/scale.category.tests.js +++ b/test/specs/scale.category.tests.js @@ -11,14 +11,10 @@ describe('Category scale tests', function() { var defaultConfig = Chart.scaleService.getScaleDefaults('category'); expect(defaultConfig).toEqual({ display: true, - - border: { - display: false, - color: 'rgba(0, 0, 0, 0.4)', - lineWidth: 1, - borderDash: [], - borderDashOffset: 0.0 - }, + borderColor: 'rgba(0, 0, 0, 0.4)', + borderWidth: 0, + borderDash: [], + borderDashOffset: 0.0, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, diff --git a/test/specs/scale.linear.tests.js b/test/specs/scale.linear.tests.js index 1b07f4b3c25..fa24aafe691 100644 --- a/test/specs/scale.linear.tests.js +++ b/test/specs/scale.linear.tests.js @@ -9,14 +9,10 @@ describe('Linear Scale', function() { var defaultConfig = Chart.scaleService.getScaleDefaults('linear'); expect(defaultConfig).toEqual({ display: true, - - border: { - display: false, - color: 'rgba(0, 0, 0, 0.4)', - lineWidth: 1, - borderDash: [], - borderDashOffset: 0.0 - }, + borderColor: 'rgba(0, 0, 0, 0.4)', + borderWidth: 0, + borderDash: [], + borderDashOffset: 0.0, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js index 79bb47f853d..0d239e61344 100644 --- a/test/specs/scale.logarithmic.tests.js +++ b/test/specs/scale.logarithmic.tests.js @@ -9,13 +9,10 @@ describe('Logarithmic Scale tests', function() { var defaultConfig = Chart.scaleService.getScaleDefaults('logarithmic'); expect(defaultConfig).toEqual({ display: true, - border: { - display: false, - color: 'rgba(0, 0, 0, 0.4)', - lineWidth: 1, - borderDash: [], - borderDashOffset: 0.0 - }, + borderColor: 'rgba(0, 0, 0, 0.4)', + borderWidth: 0, + borderDash: [], + borderDashOffset: 0.0, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, diff --git a/test/specs/scale.radialLinear.tests.js b/test/specs/scale.radialLinear.tests.js index 7ba2f104ef6..13ba930a2bd 100644 --- a/test/specs/scale.radialLinear.tests.js +++ b/test/specs/scale.radialLinear.tests.js @@ -16,13 +16,10 @@ describe('Test the radial linear scale', function() { }, animate: true, display: true, - border: { - display: false, - color: 'rgba(0, 0, 0, 0.4)', - lineWidth: 1, - borderDash: [], - borderDashOffset: 0.0 - }, + borderColor: 'rgba(0, 0, 0, 0.4)', + borderWidth: 0, + borderDash: [], + borderDashOffset: 0.0, gridLines: { circular: false, color: 'rgba(0, 0, 0, 0.1)', diff --git a/test/specs/scale.time.tests.js b/test/specs/scale.time.tests.js index cfefcb93790..db8672a126d 100755 --- a/test/specs/scale.time.tests.js +++ b/test/specs/scale.time.tests.js @@ -55,13 +55,10 @@ describe('Time scale tests', function() { var defaultConfig = Chart.scaleService.getScaleDefaults('time'); expect(defaultConfig).toEqual({ display: true, - border: { - display: false, - color: 'rgba(0, 0, 0, 0.4)', - lineWidth: 1, - borderDash: [], - borderDashOffset: 0.0 - }, + borderColor: 'rgba(0, 0, 0, 0.4)', + borderWidth: 0, + borderDash: [], + borderDashOffset: 0.0, gridLines: { color: 'rgba(0, 0, 0, 0.1)', drawBorder: true, From eb094f5d6d477084e2799b630943de32f612a84f Mon Sep 17 00:00:00 2001 From: Zamaroth Date: Sat, 21 Oct 2017 12:18:28 +0200 Subject: [PATCH 3/4] Adress comments --- src/core/core.scale.js | 12 ++-- src/plugins/plugin.gridlines.js | 100 ++++++++++++++++++-------------- 2 files changed, 64 insertions(+), 48 deletions(-) diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 7f500e895dc..020b11eec82 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -806,7 +806,7 @@ module.exports = function(Chart) { tmBorderDash: borderDash, tmBorderDashOffset: borderDashOffset, rotation: -1 * labelRotationRadians, - label: label, + label: '' + label, major: tick.major, textBaseline: textBaseline, textAlign: textAlign @@ -814,7 +814,7 @@ module.exports = function(Chart) { }); // When offsetGridLines is enabled, there should be one more tick mark than there - // are ticks in scale.ticks array, thefore the missing gridLine has to be manually added + // are ticks in scale.ticks array, therefore the missing gridLine has to be manually added if (gridLines.offsetGridLines) { var aliasPixel = helpers.aliasPixel(gridLines.lineWidth); itemsToDraw.push({ @@ -822,8 +822,8 @@ module.exports = function(Chart) { ty1: !isHorizontal ? chartArea.bottom + aliasPixel : yTickStart, tx2: !isHorizontal ? xTickEnd : chartArea.right + aliasPixel, ty2: !isHorizontal ? chartArea.bottom + aliasPixel : yTickEnd, - tmWidth: gridLines.lineWidth, - tmColor: gridLines.color, + tmWidth: helpers.valueAtIndexOrDefault(gridLines.lineWidth, ticks.length), + tmColor: helpers.valueAtIndexOrDefault(gridLines.color, ticks.length), tmBorderDash: gridLines.borderDash, tmBorderDashOffset: gridLines.borderDashOffset }); @@ -851,7 +851,7 @@ module.exports = function(Chart) { context.restore(); } - if (optionTicks.display && itemToDraw.label !== undefined && itemToDraw.label !== '') { + if (optionTicks.display && itemToDraw.label) { // Make sure we draw text in the correct color and font context.save(); context.translate(itemToDraw.labelX, itemToDraw.labelY); @@ -909,7 +909,7 @@ module.exports = function(Chart) { } // gridLines.drawBorder is deprecated - if (gridLines.drawBorder && options.borderColor !== null && options.borderWidth !== 0 && options.borderWidth !== null) { + if (gridLines.drawBorder && options.borderColor && options.borderWidth) { // Draw the scale border context.lineWidth = options.borderWidth; context.strokeStyle = options.borderColor; diff --git a/src/plugins/plugin.gridlines.js b/src/plugins/plugin.gridlines.js index d30df6ef676..4c6c7b0b896 100644 --- a/src/plugins/plugin.gridlines.js +++ b/src/plugins/plugin.gridlines.js @@ -1,12 +1,24 @@ 'use strict'; -module.exports = function(Chart) { - var helpers = Chart.helpers; - var globalDefaults = Chart.defaults.global; +var defaults = require('../core/core.defaults'); +var helpers = require('../helpers/index'); +var MODEL_KEY = '$gridlines'; - // Stores all gridLines that should be displayed - var lines = []; +// This is copied from core.scale.js, which is also required here and im not sure where it should be placed for both of them to access it +function getLineValue(scale, index, offsetGridLines) { + var lineValue = scale.getPixelForTick(index); + if (offsetGridLines) { + if (index === 0) { + lineValue -= (scale.getPixelForTick(1) - lineValue) / 2; + } else { + lineValue -= (lineValue - scale.getPixelForTick(index - 1)) / 2; + } + } + return lineValue; +} + +module.exports = function() { function getGridLine(chart, scale, position, tickIndex) { var chartArea = chart.chartArea; var scaleOptions = scale.options; @@ -14,7 +26,7 @@ module.exports = function(Chart) { var lineWidth, lineColor, borderDash, borderDashOffset; - if (tickIndex === (typeof scale.zeroLineIndex !== 'undefined' ? scale.zeroLineIndex : 0)) { + if (tickIndex !== undefined && tickIndex === scale.zeroLineIndex) { lineWidth = gridLines.zeroLineWidth; lineColor = gridLines.zeroLineColor; borderDash = gridLines.zeroLineBorderDash; @@ -22,8 +34,8 @@ module.exports = function(Chart) { } else { lineWidth = helpers.getValueAtIndexOrDefault(gridLines.lineWidth, tickIndex); lineColor = helpers.getValueAtIndexOrDefault(gridLines.color, tickIndex); - borderDash = helpers.getValueOrDefault(gridLines.borderDash, globalDefaults.borderDash); - borderDashOffset = helpers.getValueOrDefault(gridLines.borderDashOffset, globalDefaults.borderDashOffset); + borderDash = helpers.getValueOrDefault(gridLines.borderDash, defaults.global.borderDash); + borderDashOffset = helpers.getValueOrDefault(gridLines.borderDashOffset, defaults.global.borderDashOffset); } var x1, x2, y1, y2; @@ -51,9 +63,9 @@ module.exports = function(Chart) { } function collectVisibleGridLines(chart) { - lines = []; + var lines = []; - // Temporary object that stores already collected gridLine positions to check and prevent gridLines from overlapping + // Temporary object that stores already collected gridLine positions to prevent gridLines from overlapping var gridLinesHash = { horizontal: {}, vertical: {} @@ -62,12 +74,11 @@ module.exports = function(Chart) { // Marks scale border positions to prevent overlapping of gridLines and scale borders helpers.each(chart.scales, function(scale) { var scaleOptions = scale.options; + var glHashByOrientantion = gridLinesHash[!scale.isHorizontal() ? 'horizontal' : 'vertical']; + var borderPosition; // gridLines.drawBorder is deprecated if (scaleOptions.gridLines.drawBorder && scaleOptions.borderColor !== null && scaleOptions.borderWidth !== 0 && scaleOptions.borderWidth !== null) { - var glHashByOrientantion = gridLinesHash[!scale.isHorizontal() ? 'horizontal' : 'vertical']; - var borderPosition; - if (scale.isHorizontal()) { borderPosition = scale.position === 'top' ? scale.bottom : scale.top; } else { @@ -81,19 +92,16 @@ module.exports = function(Chart) { // Collects gridLines helpers.each(chart.scales, function(scale) { var scaleOptions = scale.options; + var glHashByOrientantion = gridLinesHash[scale.isHorizontal() ? 'horizontal' : 'vertical']; + var position; if (scaleOptions.display && scaleOptions.gridLines.display && scaleOptions.gridLines.drawOnChartArea) { - var glHashByOrientantion = gridLinesHash[scale.isHorizontal() ? 'horizontal' : 'vertical']; - var position; - - for (var tickIndex = 0; tickIndex < scale.ticks.length; tickIndex++) { - var tick = scale.ticks[tickIndex]; - - if (tick === null || tick === undefined) { + for (var tickIndex = 0, ticksCount = scale.ticks.length; tickIndex < ticksCount; tickIndex++) { + if (helpers.isNullOrUndef(scale.ticks[tickIndex])) { continue; } - position = scale.getPixelForTick(tickIndex); + position = getLineValue(scale, tickIndex, scaleOptions.gridLines.offsetGridLines && ticksCount > 1); if (glHashByOrientantion[position] === undefined) { glHashByOrientantion[position] = true; @@ -102,7 +110,7 @@ module.exports = function(Chart) { } // When offsetGridLines is enabled, there should be one more gridLine than there - // are ticks in scale.ticks array, thefore the missing gridLine has to be manually added + // are ticks in scale.ticks array, therefore the missing gridLine has to be manually added if (scaleOptions.gridLines.offsetGridLines) { position = Math.round(!scale.isHorizontal() ? scale.bottom : scale.right); @@ -113,26 +121,29 @@ module.exports = function(Chart) { } } }); + + return lines; } - function drawGridLines(chart) { - var context = chart.ctx; + function drawGridLines(ctx, lines) { + var aliasPixel; + var x1, x2, y1, y2; - for (var i = 0; i < lines.length; i++) { + for (var i = 0, ilen = lines.length; i < ilen; i++) { var line = lines[i]; - context.lineWidth = line.width; - context.strokeStyle = line.color; - if (context.setLineDash) { - context.setLineDash(line.borderDash); - context.lineDashOffset = line.borderDashOffset; + ctx.lineWidth = line.width; + ctx.strokeStyle = line.color; + if (ctx.setLineDash) { + ctx.setLineDash(line.borderDash); + ctx.lineDashOffset = line.borderDashOffset; } - var aliasPixel = helpers.aliasPixel(context.lineWidth); - var x1 = line.x1; - var x2 = line.x2; - var y1 = line.y1; - var y2 = line.y2; + aliasPixel = helpers.aliasPixel(ctx.lineWidth); + x1 = line.x1; + x2 = line.x2; + y1 = line.y1; + y2 = line.y2; if (y1 === y2) { y1 += aliasPixel; @@ -142,13 +153,13 @@ module.exports = function(Chart) { x2 += aliasPixel; } - context.beginPath(); + ctx.beginPath(); - context.moveTo(x1, y1); - context.lineTo(x2, y2); + ctx.moveTo(x1, y1); + ctx.lineTo(x2, y2); - context.stroke(); - context.restore(); + ctx.stroke(); + ctx.restore(); } } @@ -156,11 +167,16 @@ module.exports = function(Chart) { id: 'gridlines', afterUpdate: function(chart) { - collectVisibleGridLines(chart); + chart[MODEL_KEY] = { + lines: collectVisibleGridLines(chart) + }; }, beforeDraw: function(chart) { - drawGridLines(chart); + var model = chart[MODEL_KEY]; + if (model) { + drawGridLines(chart.ctx, model.lines); + } } }; }; From 3523f9bd61b0968c2f2a228b1233c8fac7bc6bcb Mon Sep 17 00:00:00 2001 From: Zamaroth Date: Thu, 2 Nov 2017 11:08:28 +0100 Subject: [PATCH 4/4] Fix typos and add borderAliasPixel variable --- src/core/core.scale.js | 9 +++++---- src/plugins/plugin.gridlines.js | 14 +++++++------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 020b11eec82..801df6846b3 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -923,14 +923,15 @@ module.exports = function(Chart) { var y1 = Math.round(me.top); var y2 = Math.round(me.bottom); + var borderAliasPixel = helpers.aliasPixel(context.lineWidth); if (isHorizontal) { y1 = y2 = options.position === 'top' ? me.bottom : me.top; - y1 += helpers.aliasPixel(context.lineWidth); - y2 += helpers.aliasPixel(context.lineWidth); + y1 += borderAliasPixel; + y2 += borderAliasPixel; } else { x1 = x2 = options.position === 'left' ? me.right : me.left; - x1 += helpers.aliasPixel(context.lineWidth); - x2 += helpers.aliasPixel(context.lineWidth); + x1 += borderAliasPixel; + x2 += borderAliasPixel; } context.beginPath(); diff --git a/src/plugins/plugin.gridlines.js b/src/plugins/plugin.gridlines.js index 4c6c7b0b896..d4635481ea8 100644 --- a/src/plugins/plugin.gridlines.js +++ b/src/plugins/plugin.gridlines.js @@ -74,7 +74,7 @@ module.exports = function() { // Marks scale border positions to prevent overlapping of gridLines and scale borders helpers.each(chart.scales, function(scale) { var scaleOptions = scale.options; - var glHashByOrientantion = gridLinesHash[!scale.isHorizontal() ? 'horizontal' : 'vertical']; + var glHashByOrientation = gridLinesHash[!scale.isHorizontal() ? 'horizontal' : 'vertical']; var borderPosition; // gridLines.drawBorder is deprecated @@ -85,14 +85,14 @@ module.exports = function() { borderPosition = scale.position === 'left' ? scale.right : scale.left; } - glHashByOrientantion[Math.round(borderPosition)] = true; + glHashByOrientation[Math.round(borderPosition)] = true; } }); // Collects gridLines helpers.each(chart.scales, function(scale) { var scaleOptions = scale.options; - var glHashByOrientantion = gridLinesHash[scale.isHorizontal() ? 'horizontal' : 'vertical']; + var glHashByOrientation = gridLinesHash[scale.isHorizontal() ? 'horizontal' : 'vertical']; var position; if (scaleOptions.display && scaleOptions.gridLines.display && scaleOptions.gridLines.drawOnChartArea) { @@ -103,8 +103,8 @@ module.exports = function() { position = getLineValue(scale, tickIndex, scaleOptions.gridLines.offsetGridLines && ticksCount > 1); - if (glHashByOrientantion[position] === undefined) { - glHashByOrientantion[position] = true; + if (glHashByOrientation[position] === undefined) { + glHashByOrientation[position] = true; lines.push(getGridLine(chart, scale, position, tickIndex)); } } @@ -114,8 +114,8 @@ module.exports = function() { if (scaleOptions.gridLines.offsetGridLines) { position = Math.round(!scale.isHorizontal() ? scale.bottom : scale.right); - if (glHashByOrientantion[position] === undefined) { - glHashByOrientantion[position] = true; + if (glHashByOrientation[position] === undefined) { + glHashByOrientation[position] = true; lines.push(getGridLine(chart, scale, position, undefined)); } }