Move drawing of gridLines into separate plugin and add border functionality #4117
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,182 @@ | ||
| 'use strict'; | ||
|
|
||
| var defaults = require('../core/core.defaults'); | ||
| var helpers = require('../helpers/index'); | ||
| var MODEL_KEY = '$gridlines'; | ||
|
|
||
| // 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 | ||
|
There was a problem hiding this comment. This should not be here, but i dont know where to move it for both core.scale.js and this plugin 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; | ||
| var gridLines = scaleOptions.gridLines; | ||
|
|
||
| var lineWidth, lineColor, borderDash, borderDashOffset; | ||
|
|
||
| if (tickIndex !== undefined && tickIndex === scale.zeroLineIndex) { | ||
| lineWidth = gridLines.zeroLineWidth; | ||
| lineColor = gridLines.zeroLineColor; | ||
| borderDash = gridLines.zeroLineBorderDash; | ||
| borderDashOffset = gridLines.zeroLineBorderDashOffset; | ||
| } else { | ||
| lineWidth = helpers.getValueAtIndexOrDefault(gridLines.lineWidth, tickIndex); | ||
| lineColor = helpers.getValueAtIndexOrDefault(gridLines.color, tickIndex); | ||
| borderDash = helpers.getValueOrDefault(gridLines.borderDash, defaults.global.borderDash); | ||
| borderDashOffset = helpers.getValueOrDefault(gridLines.borderDashOffset, defaults.global.borderDashOffset); | ||
| } | ||
|
|
||
| var x1, x2, y1, y2; | ||
|
|
||
| if (scale.isHorizontal()) { | ||
| x1 = x2 = position; | ||
| y1 = chartArea.top; | ||
| y2 = chartArea.bottom; | ||
| } else { | ||
| x1 = chartArea.left; | ||
| x2 = chartArea.right; | ||
| y1 = y2 = position; | ||
| } | ||
|
|
||
| return { | ||
| x1: x1, | ||
| x2: x2, | ||
| y1: y1, | ||
| y2: y2, | ||
| width: lineWidth, | ||
| color: lineColor, | ||
| borderDash: borderDash, | ||
| borderDashOffset: borderDashOffset | ||
| }; | ||
| } | ||
|
|
||
| function collectVisibleGridLines(chart) { | ||
| var lines = []; | ||
|
|
||
| // Temporary object that stores already collected gridLine positions to prevent gridLines from overlapping | ||
| var gridLinesHash = { | ||
| horizontal: {}, | ||
| vertical: {} | ||
| }; | ||
|
|
||
| // 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']; | ||
|
There was a problem hiding this comment. [nit] |
||
| var borderPosition; | ||
|
|
||
| // gridLines.drawBorder is deprecated | ||
| if (scaleOptions.gridLines.drawBorder && scaleOptions.borderColor !== null && scaleOptions.borderWidth !== 0 && scaleOptions.borderWidth !== null) { | ||
| if (scale.isHorizontal()) { | ||
| borderPosition = scale.position === 'top' ? scale.bottom : scale.top; | ||
| } else { | ||
| borderPosition = scale.position === 'left' ? scale.right : scale.left; | ||
| } | ||
|
|
||
| glHashByOrientantion[Math.round(borderPosition)] = true; | ||
| } | ||
| }); | ||
|
|
||
| // 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) { | ||
| for (var tickIndex = 0, ticksCount = scale.ticks.length; tickIndex < ticksCount; tickIndex++) { | ||
| if (helpers.isNullOrUndef(scale.ticks[tickIndex])) { | ||
|
There was a problem hiding this comment. @simonbrunel @benmccann does this check make sense in the context of how ticks changed in 2.7.0? |
||
| continue; | ||
| } | ||
|
|
||
| position = getLineValue(scale, tickIndex, scaleOptions.gridLines.offsetGridLines && ticksCount > 1); | ||
|
|
||
| if (glHashByOrientantion[position] === undefined) { | ||
| glHashByOrientantion[position] = true; | ||
| lines.push(getGridLine(chart, scale, position, tickIndex)); | ||
| } | ||
| } | ||
|
|
||
| // When offsetGridLines is enabled, there should be one more gridLine than there | ||
| // 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); | ||
|
|
||
| if (glHashByOrientantion[position] === undefined) { | ||
| glHashByOrientantion[position] = true; | ||
| lines.push(getGridLine(chart, scale, position, undefined)); | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| return lines; | ||
| } | ||
|
|
||
| function drawGridLines(ctx, lines) { | ||
| var aliasPixel; | ||
| var x1, x2, y1, y2; | ||
|
|
||
| for (var i = 0, ilen = lines.length; i < ilen; i++) { | ||
| var line = lines[i]; | ||
|
|
||
| ctx.lineWidth = line.width; | ||
| ctx.strokeStyle = line.color; | ||
| if (ctx.setLineDash) { | ||
| ctx.setLineDash(line.borderDash); | ||
| ctx.lineDashOffset = line.borderDashOffset; | ||
| } | ||
|
|
||
| aliasPixel = helpers.aliasPixel(ctx.lineWidth); | ||
| x1 = line.x1; | ||
| x2 = line.x2; | ||
| y1 = line.y1; | ||
| y2 = line.y2; | ||
|
|
||
| if (y1 === y2) { | ||
| y1 += aliasPixel; | ||
| y2 += aliasPixel; | ||
| } else { | ||
| x1 += aliasPixel; | ||
| x2 += aliasPixel; | ||
| } | ||
|
|
||
| ctx.beginPath(); | ||
|
|
||
| ctx.moveTo(x1, y1); | ||
| ctx.lineTo(x2, y2); | ||
|
|
||
| ctx.stroke(); | ||
| ctx.restore(); | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| id: 'gridlines', | ||
|
|
||
| afterUpdate: function(chart) { | ||
| chart[MODEL_KEY] = { | ||
| lines: collectVisibleGridLines(chart) | ||
| }; | ||
| }, | ||
|
|
||
| beforeDraw: function(chart) { | ||
| var model = chart[MODEL_KEY]; | ||
| if (model) { | ||
| drawGridLines(chart.ctx, model.lines); | ||
| } | ||
| } | ||
| }; | ||
| }; | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might make sense to do this for any value of
itemToDraw.labelthat isn't truthy. Blank labels don't need to be drawn and same withnulllabels. What do you think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
items with null or undefined label aren't pushed into the array at all, thanks to this statement.
The blank labels don't need to be drawn tho. The question is if they should be skipped the same way as when it's null(its ticks and gridLines wont be drawn) or in the statement you linked so there will be a blank tick, but the label itself wont be drawn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, for grid lines we have the following logic:
if label is a string, draw label, tick and grid line
if label is
nullorundefinedhide the label, tick and gridline.This allows users to filter out grid lines by return
nullfrom the tick callback. here is an example of filteringThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i understand, i'll add just add the statement to skip the drawing of label itself if it's empty string then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good