-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Line Chart - line on the edge get cut fix ( #4202) #5321
Conversation
Please merge this, this has been an issue since 2.4! Please... |
src/controllers/controller.line.js
Outdated
@@ -283,11 +283,19 @@ module.exports = function(Chart) { | |||
var points = meta.data || []; | |||
var area = chart.chartArea; | |||
var ilen = points.length; | |||
var dataset = me.getDataset(); | |||
var lineElementOptions = chart.options.elements.line; | |||
var halfBorderWidth = helpers.valueOrDefault(dataset.borderWidth, lineElementOptions.borderWidth) / 2; |
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.
We should not recompute the border width in draw
, it's already done in update()
.
I think meta.dataset._model.borderWidth
is more accurate:
var halfBorderWidth = (meta.dataset._model.borderWidth || 0) / 2;
@simonbrunel Any ideas how can I fix this? |
src/controllers/controller.line.js
Outdated
@@ -283,9 +283,15 @@ module.exports = function(Chart) { | |||
var points = meta.data || []; | |||
var area = chart.chartArea; | |||
var ilen = points.length; | |||
var halfBorderWidth = (meta.dataset._model.borderWidth || 0) / 2; |
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.
I should have read update()
more carefully because _model
is only available if options.showLine: true
.
We could also avoid clipping if there is nothing to draw ...
if (lineEnabled(me.getDataset(), chart.options)) {
halfBorderWidth = (meta.dataset._model.borderWidth || 0) / 2;
helpers.canvas.clipArea(chart.ctx, {
left: area.left,
right: area.right,
top: area.top - halfBorderWidth,
bottom: area.bottom + halfBorderWidth
});
meta.dataset.draw();
helpers.canvas.unclipArea(chart.ctx);
}
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.
Please verify that it works locally, I didn't test it, neither that tests pass (gulp test
).
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.
@simonbrunel
Yeah that fixed the problem, did some manual tests too, everything seems fine for both options: showLine: true
and showLine: false
.
That's awesome, Any chance to put this in a release, looks like it missed 2.7.2 which starts to be old? |
What's required to include this in a release? Thanks for a great product! |
This fix has been included and released in 2.7.3. |
Extending chartArea to contain thicker lines at the edges.
Fixes #4202