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

Pass more data to sunburst color method to allow greater customization and consistency #1677

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Pass more data to sunburst color method to allow greater customization and consistency #1677

wants to merge 1 commit into from

Conversation

manbearwiz
Copy link

This fix passes the entire data object and index into the color function rather than just the name. In addition to to allowing more intelligent color functions, this change makes the API more consistent with the pie chart.

The motivation for this change is to be able to use an attribute of the data passed in to generate the fill colors for the sections in order to signify intensity.

BREAKING CHANGE:

Any existing sunburst chart that uses a custom color function will need to be updated to the new API; however, the name is still passed to this function as an attribute of the object. Migration is as simple as changing

function(name) {
  // ...
}

to

function(data) {
  var name = data.name;
  // ...
}

I would also like to note that in the current state, it seems unlikely anyone would be using a node name to do any sort of color calculation/generation.

This fix passes the entire data object and index into the color function rather than just the name. In addition to to allowing more intelligent color functions, this change makes the API more consistent with the pie chart.

BREAKING CHANGE:

Any existing sunburst chart that uses a custom color function will need to be updated to the new api; however, the name is still passed to this function as an attribute of the object.
@manbearwiz manbearwiz changed the title Pass more data to color method to allow greater customization and consistency Pass more data to sunburst color method to allow greater customization and consistency May 23, 2016
}
else {
return color(d.name);
return color(d, i);
Copy link
Contributor

Choose a reason for hiding this comment

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

The default color function is nv.utils.defaultColor, which uses nv.utils.getColor which has a definition of just function(color). While it might work, it's going to hash a whole object every time, which is going to end up generating different colors if the data structure is modified.

Why would you be changing/duplicating the name attribute anyway? If you really have to do that, then I suggest putting a unique number on the end of the name and then strip it off with a custom tooltip.headerFormatter option

@manbearwiz
Copy link
Author

This change make it consistent with the pie chart. Correct me if I'm wrong, but any disadvantage to this implementation would already be evident on the pie chart then. From pie.js:

slices.attr('fill', function(d,i) { return color(d.data, i); });

My general use case is to pass in objects like

{
  name: "Wisconsin",
  count: 5758,
  temp: 67
}

and then use a d3 linear color scale to color segments by the temp field. Any suggestions on a better way to achieve this then passing that object into the color function?

@liquidpele
Copy link
Contributor

I'll have to think about this one, I don't much care for passing in objects as keys but it would be nice to be able to reference the full object too...

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.

2 participants