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

Line Chart - line on the edge get cut fix ( #4202) #5321

Merged
merged 4 commits into from Apr 3, 2018

Conversation

serhii-yakymuk
Copy link
Contributor

@serhii-yakymuk serhii-yakymuk commented Mar 6, 2018

Extending chartArea to contain thicker lines at the edges.

Fixes #4202

@martijnmelchers
Copy link

Please merge this, this has been an issue since 2.4! Please...

@@ -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;
Copy link
Member

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 simonbrunel added this to the Version 2.8 milestone Apr 2, 2018
@serhii-yakymuk
Copy link
Contributor Author

serhii-yakymuk commented Apr 2, 2018

@simonbrunel
Tests are failing now with:
TypeError: Cannot read property 'borderWidth' of undefined

Any ideas how can I fix this?

@@ -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;
Copy link
Member

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);
}

Copy link
Member

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).

Copy link
Contributor Author

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.

@simonbrunel simonbrunel merged commit 7c3e934 into chartjs:master Apr 3, 2018
@lethosor lethosor mentioned this pull request May 27, 2018
@ldemailly
Copy link

That's awesome, Any chance to put this in a release, looks like it missed 2.7.2 which starts to be old?

@armenzg
Copy link

armenzg commented Oct 12, 2018

What's required to include this in a release?
Until all issues in https://github.com/chartjs/Chart.js/milestone/17 are tackled?
I'm new to the project.

Thanks for a great product!

@nagix
Copy link
Contributor

nagix commented Oct 23, 2018

This fix has been included and released in 2.7.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants