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 the ability to add a title to the legend #6906

Merged
merged 6 commits into from
Jan 10, 2020

Conversation

etimberg
Copy link
Member

@etimberg etimberg commented Jan 4, 2020

Resolves #4466

  • Legend title can be specified
  • Font & color options added
  • Padding option added
  • Positioning option added
  • Legend title sample file added

legend title

Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Just one question

src/plugins/plugin.legend.js Outdated Show resolved Hide resolved
kurkle
kurkle previously approved these changes Jan 4, 2020
@benmccann
Copy link
Contributor

I'm not sure all the positions end up quite where I'd expect. The orange one I agree. The others they're all very far from the legend. I'd expect it to be within the bounds of the legend, but potentially left, center, or right aligned. See below

71770982-0d6e6480-2f02-11ea-92ae-5e00fe5d0377

@etimberg
Copy link
Member Author

etimberg commented Jan 4, 2020

I see what you mean about the vertical positioned title (a separate setting would need to be added to move the entire legend up and down). For the horizontal legend, it's because by default the legend takes the entire width of the chart.

@etimberg
Copy link
Member Author

etimberg commented Jan 4, 2020

Looking into this some more ... I don't know what I was thinking when I first wrote the legend. Lots of updating happens in draw() and I have no idea why. If everything happened in fit this would be relatively easy since I'd know the height/width of the largest column/row which is what I really need.

@etimberg
Copy link
Member Author

etimberg commented Jan 5, 2020

Ok, figured out why it was needed ... the final width isn't know until the layout process is done. I've got it working with the align setting. I've updated the sample to be more thorough

positions2

src/plugins/plugin.legend.js Outdated Show resolved Hide resolved
@benmccann
Copy link
Contributor

In the last image it looks like the title position affects the entire legend and not just the title. Perhaps instead of calling them position and title.position it should be align and verticalAlign?

@etimberg
Copy link
Member Author

etimberg commented Jan 5, 2020

@benmccann sorry for the confusion, it’s actually a mix of settings. align moves the legend up and down. Title align just moves the title. I added align into the sample so I could be sure it worked in all cases

@etimberg etimberg requested a review from benmccann January 5, 2020 20:10
@benmccann
Copy link
Contributor

@etimberg this one will need to be rebased

src/plugins/plugin.legend.js Outdated Show resolved Hide resolved
src/plugins/plugin.legend.js Outdated Show resolved Hide resolved
etimberg and others added 5 commits January 9, 2020 07:24
- Legend title can be specified
- Font & color options added
- Padding option added
- Positioning option added
- Legend title sample file added
@kurkle
Copy link
Member

kurkle commented Jan 9, 2020

Build failing!

@etimberg etimberg merged commit d04cdfc into chartjs:master Jan 10, 2020
@etimberg etimberg added this to the Version 3.0 milestone Jan 10, 2020
@etimberg etimberg deleted the legend-title branch January 19, 2020 18:09
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.

Add Legend title
3 participants