Skip to content

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

Merged
simonbrunel merged 4 commits into
chartjs:masterfrom
serhii-yakymuk:master
Apr 3, 2018
Merged

Line Chart - line on the edge get cut fix ( #4202)#5321
simonbrunel merged 4 commits into
chartjs:masterfrom
serhii-yakymuk:master

Conversation

@serhii-yakymuk

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

Copy link
Copy Markdown
Contributor

Extending chartArea to contain thicker lines at the edges.

Fixes #4202

@martijnmelchers

Copy link
Copy Markdown

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

Comment thread src/controllers/controller.line.js Outdated
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
Copy Markdown
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

serhii-yakymuk commented Apr 2, 2018

Copy link
Copy Markdown
Contributor Author

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

Any ideas how can I fix this?

Comment thread src/controllers/controller.line.js Outdated
var points = meta.data || [];
var area = chart.chartArea;
var ilen = points.length;
var halfBorderWidth = (meta.dataset._model.borderWidth || 0) / 2;

Copy link
Copy Markdown
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);
}

@simonbrunel simonbrunel Apr 2, 2018

Copy link
Copy Markdown
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
Copy Markdown
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 requested a review from etimberg April 2, 2018 15:05
@simonbrunel simonbrunel merged commit 7c3e934 into chartjs:master Apr 3, 2018
@lethosor lethosor mentioned this pull request May 27, 2018
@ldemailly

Copy link
Copy Markdown

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

@armenzg

armenzg commented Oct 12, 2018

Copy link
Copy Markdown

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

nagix commented Oct 23, 2018

Copy link
Copy Markdown
Contributor

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.

8 participants