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

Scale: draw border on separate layer after grid #8936

Merged
merged 1 commit into from Apr 18, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 41 additions & 23 deletions src/core/core.scale.js
Expand Up @@ -1007,7 +1007,7 @@ export default class Scale extends Element {
const tl = getTickMarkLength(grid);
const items = [];

const borderOpts = grid.setContext(me.getContext(0));
const borderOpts = grid.setContext(me.getContext());
const axisWidth = borderOpts.drawBorder ? borderOpts.borderWidth : 0;
const axisHalfWidth = axisWidth / 2;
const alignBorderValue = function(pixel) {
Expand Down Expand Up @@ -1369,9 +1369,6 @@ export default class Scale extends Element {
const me = this;
const grid = me.options.grid;
const ctx = me.ctx;
const chart = me.chart;
const borderOpts = grid.setContext(me.getContext());
const axisWidth = grid.drawBorder ? borderOpts.borderWidth : 0;
const items = me._gridLineItems || (me._gridLineItems = me._computeGridLineItems(chartArea));
let i, ilen;

Expand Down Expand Up @@ -1418,27 +1415,42 @@ export default class Scale extends Element {
}
}
}
}

if (axisWidth) {
// Draw the line at the edge of the axis
const lastLineWidth = borderOpts.lineWidth;
const borderValue = me._borderValue;
let x1, x2, y1, y2;
/**
* @protected
*/
drawBorder() {
const me = this;
const {chart, ctx, options: {grid}} = me;
const borderOpts = grid.setContext(me.getContext());
const axisWidth = grid.drawBorder ? borderOpts.borderWidth : 0;
if (!axisWidth) {
return;
}
const lastLineWidth = grid.setContext(me.getContext(0)).lineWidth;
const borderValue = me._borderValue;
let x1, x2, y1, y2;

if (me.isHorizontal()) {
x1 = _alignPixel(chart, me.left, axisWidth) - axisWidth / 2;
x2 = _alignPixel(chart, me.right, lastLineWidth) + lastLineWidth / 2;
y1 = y2 = borderValue;
} else {
y1 = _alignPixel(chart, me.top, axisWidth) - axisWidth / 2;
y2 = _alignPixel(chart, me.bottom, lastLineWidth) + lastLineWidth / 2;
x1 = x2 = borderValue;
}
drawLine(
{x: x1, y: y1},
{x: x2, y: y2},
{width: axisWidth, color: borderOpts.borderColor});
if (me.isHorizontal()) {
x1 = _alignPixel(chart, me.left, axisWidth) - axisWidth / 2;
x2 = _alignPixel(chart, me.right, lastLineWidth) + lastLineWidth / 2;
y1 = y2 = borderValue;
} else {
y1 = _alignPixel(chart, me.top, axisWidth) - axisWidth / 2;
y2 = _alignPixel(chart, me.bottom, lastLineWidth) + lastLineWidth / 2;
x1 = x2 = borderValue;
}
ctx.save();
ctx.lineWidth = borderOpts.borderWidth;
ctx.strokeStyle = borderOpts.borderColor;

ctx.beginPath();
ctx.moveTo(x1, y1);
ctx.lineTo(x2, y2);
ctx.stroke();

ctx.restore();
}

/**
Expand Down Expand Up @@ -1520,6 +1532,7 @@ export default class Scale extends Element {

me.drawBackground();
me.drawGrid(chartArea);
me.drawBorder();
me.drawTitle();
me.drawLabels(chartArea);
}
Expand All @@ -1534,7 +1547,7 @@ export default class Scale extends Element {
const tz = opts.ticks && opts.ticks.z || 0;
const gz = opts.grid && opts.grid.z || 0;

if (!me._isVisible() || tz === gz || me.draw !== Scale.prototype.draw) {
if (!me._isVisible() || me.draw !== Scale.prototype.draw) {
// backward compatibility: draw has been overridden by custom scale
return [{
z: tz,
Expand All @@ -1551,6 +1564,11 @@ export default class Scale extends Element {
me.drawGrid(chartArea);
me.drawTitle();
}
}, {
z: gz + 1, // TODO, v4 move border options to its own object and add z
draw() {
me.drawBorder();
}
}, {
z: tz,
draw(chartArea) {
Expand Down
5 changes: 5 additions & 0 deletions src/scales/scale.radialLinear.js
Expand Up @@ -506,6 +506,11 @@ export default class RadialLinearScale extends LinearScaleBase {
}
}

/**
* @protected
*/
drawBorder() {}

/**
* @protected
*/
Expand Down
37 changes: 37 additions & 0 deletions test/fixtures/core.scale/grid/border-over-grid.js
@@ -0,0 +1,37 @@
module.exports = {
config: {
type: 'scatter',
options: {
scales: {
x: {
position: {y: 0},
min: -10,
max: 10,
grid: {
borderColor: 'black',
borderWidth: 5,
color: 'lightGray',
lineWidth: 3,
},
ticks: {
display: false
},
},
y: {
position: {x: 0},
min: -10,
max: 10,
grid: {
borderColor: 'black',
borderWidth: 5,
color: 'lightGray',
lineWidth: 3,
},
ticks: {
display: false
},
}
}
}
}
};
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
38 changes: 38 additions & 0 deletions test/fixtures/core.scale/grid/colors.js
@@ -0,0 +1,38 @@
module.exports = {
config: {
type: 'line',
data: {
labels: ['1', '2', '3', '4', '5', '6'],
datasets: [{
label: '# of Votes',
data: [12, 19, 3, 5, 2, 3]
}],
},
options: {
scales: {
x: {
ticks: {
display: false
},
grid: {
borderColor: 'blue',
borderWidth: 2,
color: 'green',
drawTicks: false,
}
},
y: {
ticks: {
display: false
},
grid: {
borderColor: 'black',
borderWidth: 2,
color: 'red',
drawTicks: false,
}
}
}
}
}
};
Binary file added test/fixtures/core.scale/grid/colors.png
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.category/ticks-from-data.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
35 changes: 6 additions & 29 deletions test/specs/core.scale.tests.js
Expand Up @@ -433,7 +433,7 @@ describe('Core.scale', function() {
});

describe('_layers', function() {
it('should default to one layer', function() {
it('should default to three layers', function() {
var chart = window.acquireChart({
type: 'line',
options: {
Expand All @@ -446,7 +446,7 @@ describe('Core.scale', function() {
});

var scale = chart.scales.x;
expect(scale._layers().length).toEqual(1);
expect(scale._layers().length).toEqual(3);
});

it('should default to one layer for custom scales', function() {
Expand Down Expand Up @@ -482,30 +482,7 @@ describe('Core.scale', function() {
expect(scale._layers()[0].z).toEqual(20);
});

it('should default to one layer when z is equal between ticks and grid', function() {
var chart = window.acquireChart({
type: 'line',
options: {
scales: {
x: {
type: 'linear',
ticks: {
z: 10
},
grid: {
z: 10
}
}
}
}
});

var scale = chart.scales.x;
expect(scale._layers().length).toEqual(1);
});


it('should return 2 layers when z is not equal between ticks and grid', function() {
it('should return 3 layers when z is not equal between ticks and grid', function() {
var chart = window.acquireChart({
type: 'line',
options: {
Expand All @@ -520,7 +497,7 @@ describe('Core.scale', function() {
}
});

expect(chart.scales.x._layers().length).toEqual(2);
expect(chart.scales.x._layers().length).toEqual(3);

chart = window.acquireChart({
type: 'line',
Expand All @@ -536,7 +513,7 @@ describe('Core.scale', function() {
}
});

expect(chart.scales.x._layers().length).toEqual(2);
expect(chart.scales.x._layers().length).toEqual(3);

chart = window.acquireChart({
type: 'line',
Expand All @@ -555,7 +532,7 @@ describe('Core.scale', function() {
}
});

expect(chart.scales.x._layers().length).toEqual(2);
expect(chart.scales.x._layers().length).toEqual(3);

});

Expand Down