-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Introduce Colors plugin #10764
Introduce Colors plugin #10764
Conversation
@kurkle already made a plugin for this which I think should be good enough, many we can link to that instead from the starter page of the docs 🤔 https://github.com/kurkle/chartjs-plugin-autocolors And for V2 there was one from nagix which did the same but had support for a lot of different color schemes. Which should be pretty easily adoptable to v3. https://nagix.github.io/chartjs-plugin-colorschemes So if we want to ship a plugin that does this by default I think it might be more efficient to pull either one of those into the main repo instead of re inventing the wheel again |
@LeeLenaleee Wow, thanks for pointing it out! I somehow missed that one 😅 I'm fully ready to abandon my code (haha, it's just a dozen lines of very trivial code). However, I think there's a lot of value in a plugin that is bundled with Chart.js. It would help drop the bar for those who are getting started. What do you think about bundling @kurkle's plugin? |
My plugin does not really generate brand colors, but if that is ok, its fine by me to bundle it. If it is bundled, it would probably be best to move it to the org. |
And to this PR, it would also be fine by me to include a set of brand colors. I'd actually like that more, because of the brand. |
What about colorscheme plugin? |
Currently working on an update to this PR. @LeeLenaleee @etimberg Can we mark this for inclusion to v4? I believe, it would be very appropriate. |
@kurkle @LeeLenaleee @etimberg @dangreen I've rewritten the docs page for Colors and incorporated the recommendations for using plugins there. Could you please take a look? Do you think it would work that way? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you test it with all chart types? Cause not all chart types to look good with borders.
Also for certain chart types, backgroundColor should be an array of colors, for example: https://www.chartjs.org/docs/latest/samples/other-charts/doughnut.html
It might be better (for maintenance) to add a link to awesome instead of advertising a number of plugins in the docs. But I can live with this also. What is it with the ubuntu CI? |
36ed065
to
29a89bc
Compare
ef27e20
to
06f4ca6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is chartjs-plugin-gradient
also, that could be mentioned here.
This comment is here only to give more reason for directing users to the awesome list instead of listing some of the available plugins here.
So, I'd remove the autocolors and colorscheme mentions and point users in that section to the awesome list to look for more options. Other than that, this looks awesome to me :)
@LeeLenaleee @kurkle @stockiNail @dangreen Thanks a lot for your comments and help! @kurkle I've removed references to individual plugins and added a link to awesome. However, I think that the plugins in the awesome list can be rearranged for the ease of use, hence this PR: chartjs/awesome#52 @LeeLenaleee I've pinged @stockiNail Thanks for great suggestions! I hope my reasoning for not introducing them in this plugin make sense. Looks like the updated docs page and the plugin is ready for the final review 😄 |
724288c
to
9566500
Compare
I didn't reply you but yes!! Thank you very much!! Great job, guys! I really appreciate it. |
This pull request provides reasonable default
borderColor
andbackgroundColor
if datasets don't have these properties set. Datasets will cycle through a list of 7 pre-defined "brand" colors (blue, red, orange, etc.).Before:
After:
It resolves #815, #884, and #1618 and makes Chart.js more developer-friendly, especially for those who are just getting started. It removes the need for excessive configuration and brings Chart.js closer to other charting libraries that also provide default colors.
As @etimberg recommended in this comment, it makes sense to have this as a plugin. When Chart.js is imported as
chart.js/auto
, the plugin will have an immediate effect. When Chart.js is imported aschart.js
, the "default colors" behavior is opt-in—the plugin will have to be imported and registered:It's a breaking change to the current behavior with a default border color and a default background color. So, it should be integrated into the upcoming v4 release.
This is work-in-progress, but I'd like to get feedback. The PR obviously lacks docs and maybe tests. Also, when it's merged, the excessive configuration would be removed from docs in a separate PR.