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

Issue #2252, added usePointStyle option to allow label boxes to match… #2902

Merged
merged 3 commits into from
Jul 6, 2016

Conversation

slinhart
Copy link
Contributor

@slinhart slinhart commented Jul 3, 2016

Solution to this issue: #2252

New option added: options.legend.labels.usePointStyle (Boolean)
When true, legend "boxes" will be drawn in the same shape as the pointStyle specified for the data.

Simple screenshot for clarity.
image

…o match the shape(pointStyle) of the corresponding data.
@@ -52,6 +52,7 @@ module.exports = function(Chart) {
lineJoin: dataset.borderJoinStyle,
lineWidth: dataset.borderWidth,
strokeStyle: dataset.borderColor,
pointStyle: dataset.pointStyle,
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 document this in Legend Item Interface section of 01-Chart-Configuration.md

@etimberg
Copy link
Member

etimberg commented Jul 3, 2016

+1 to merge.

@slinhart nothing jumped out at me while looking at this.

CC: @tannerlinsley @derekperkins @zachpanz88 @simonbrunel

@panzarino
Copy link
Contributor

This looks awesome (assuming it works as defined), but could you please fix that merge conflict @slinhart?

@slinhart
Copy link
Contributor Author

slinhart commented Jul 4, 2016

@zachpanz88 merged 😁

@tannerlinsley
Copy link
Contributor

Looks good to me! Very cool!

@tannerlinsley tannerlinsley merged commit 0dccc85 into chartjs:master Jul 6, 2016
@neilmonroe
Copy link

Do these merged commits on pull requests ever require the version number be bumped? I was previously using the npm version of chart.js and it is also version 2.1.6, but doesn't include the changes in this issue.

@etimberg
Copy link
Member

@neilmonroe this will be included in the next release

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

Successfully merging this pull request may close these issues.

5 participants