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

Add pointStyleWidth option for legend #10412

Merged
merged 3 commits into from
Jul 18, 2022
Merged

Conversation

touletan
Copy link
Contributor

@touletan touletan commented Jun 9, 2022

When I'm using 'usePointStyle' for legend option, I want to be able to control the width of the pointStyle for Line and Rect options. For now the width is restricted to font size.

@@ -622,7 +622,7 @@ export default {

labels: {
color: (ctx) => ctx.chart.options.color,
boxWidth: 40,
// boxWidth: default is 40,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would become a breaking change for users

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default value has been transferred to getBoxSize function.
let {boxHeight = fontSize, boxWidth = 40} = labelOpts;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@touletan that is a breaking change, because that default is no longer configurable.

@touletan
Copy link
Contributor Author

What if we use a new option?

@kurkle
Copy link
Member

kurkle commented Jul 10, 2022

What if we use a new option?

Just revert the move of default value?
Nevermind, I see you already did something to that.

But the drawPoint is a public function, so change in its signature is also a breaking change.

@etimberg
Copy link
Member

etimberg commented Jul 10, 2022

I think the signature change in this case is OK since the code handles it being unset. I suppose Typescript users would see a difference if we've provided a type for this helper

@touletan
Copy link
Contributor Author

I added a new function 'drawPointLegend', so drawPoint signature is not impacted

@touletan
Copy link
Contributor Author

is there a way to ignore the codeclimate issues? The complexity of the new function is the same as it was with drawPoint.

@etimberg
Copy link
Member

is there a way to ignore the codeclimate issues? The complexity of the new function is the same as it was with drawPoint.

We can ignore it when we merge. It's not a blocker

@kurkle kurkle changed the title Use boxWidth for line and rect pointStyle in the legend Add pointStyleWidth option for legend Jul 18, 2022
@kurkle kurkle added this to the Version 3.9.0 milestone Jul 18, 2022
@etimberg etimberg merged commit 5452502 into chartjs:master Jul 18, 2022
@touletan touletan deleted the feature-4811 branch July 18, 2022 12:00
@0ro
Copy link

0ro commented Jul 28, 2022

@kurkle @etimberg Can you please help me with this option? I want to see 'circle' in the legend by default but I'm getting 'ellipse'. Is it a bug or expected behavior?

I'm using the next config

const config = {
  type: "line",
  data: data,
  options: {
    plugins: {
      legend: {
        labels: {
          usePointStyle: true
        }
      }
    }
  }
}

And here a link to codesadbox for your quick review of current behavior

@touletan
Copy link
Contributor Author

This pull request should be able to fix this bug:

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.

4 participants