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

Introduce Colors plugin #10764

Merged
merged 16 commits into from
Oct 21, 2022
Merged

Introduce Colors plugin #10764

merged 16 commits into from
Oct 21, 2022

Conversation

igorlukanin
Copy link
Member

@igorlukanin igorlukanin commented Oct 7, 2022

This pull request provides reasonable default borderColor and backgroundColor 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:

const data = {
  labels: [ ... ],
  datasets: [
    {
      label: 'Dataset 1',
      data: [ ... ],
      borderColor: '#36A2EB',
      backgroundColor: '#9BD0F5',
    },
    {
      label: 'Dataset 2',
      data: [ ... ],
      borderColor: '#FF6384',
      backgroundColor: '#FFB1C1',
    }
  ]
};

After:

const data = {
  labels: [ ... ],
  datasets: [
    {
      label: 'Dataset 1',
      data: [ ... ],
    },
    {
      label: 'Dataset 2',
      data: [ ... ],
    }
  ]
};

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 as chart.js, the "default colors" behavior is opt-in—the plugin will have to be imported and registered:

Chart.register(Colors);

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.

@LeeLenaleee
Copy link
Collaborator

LeeLenaleee commented Oct 7, 2022

@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

@igorlukanin
Copy link
Member Author

@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?

@kurkle
Copy link
Member

kurkle commented Oct 7, 2022

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.

@kurkle
Copy link
Member

kurkle commented Oct 7, 2022

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.

@stockiNail
Copy link
Contributor

What about colorscheme plugin?

@igorlukanin
Copy link
Member Author

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.

@etimberg etimberg added this to the Version 4.0 milestone Oct 12, 2022
@igorlukanin
Copy link
Member Author

@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?

Copy link
Collaborator

@dangreen dangreen left a 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

docs/general/colors.md Outdated Show resolved Hide resolved
docs/general/colors.md Outdated Show resolved Hide resolved
@kurkle
Copy link
Member

kurkle commented Oct 13, 2022

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?

docs/general/colors.md Outdated Show resolved Hide resolved
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.

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 :)

docs/general/colors.md Show resolved Hide resolved
@igorlukanin
Copy link
Member Author

@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 colorschemes author on GitHub and Twitter. Hopefully, it would help bring this plugin back on track!

@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 😄

@stockiNail
Copy link
Contributor

@stockiNail Thanks for great suggestions! I hope my reasoning for not introducing them in this plugin make sense.

I didn't reply you but yes!! Thank you very much!! Great job, guys! I really appreciate it.

@igorlukanin igorlukanin changed the title WIP: Introduce Colors plugin Introduce Colors plugin Oct 20, 2022
src/plugins/plugin.colors.ts Show resolved Hide resolved
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.

Default Colors
6 participants