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

sunburst chart doesn't render when key reduced to multiple values #1440

Closed
mukherjeea opened this issue May 18, 2018 · 6 comments
Closed

sunburst chart doesn't render when key reduced to multiple values #1440

mukherjeea opened this issue May 18, 2018 · 6 comments

Comments

@mukherjeea
Copy link

I am using dc.js v3.0.3

Sunburst chart works as expected when a key is reduced to a single value.
However when a key is reduced to multiple values and valueAccessor is used to specify one of them I see the following error.

screen shot 2018-05-18 at 10 33 17 am

this shouldn't be the case as all the values are 'defined'. Am I missing something here?
Both the cases are illustrated in the following jsfiddle
https://jsfiddle.net/brL1x26g/

@kum-deepak
Copy link
Collaborator

kum-deepak commented May 20, 2018

I think I understand the underlying reason. The Sunburst chart creates additional nodes (as parents) to create the hierarchy. These nodes are not same structure as the leaf level nodes.

The exception is generated for the first parent node that it encounters.

I have updated the fiddle with a possible solution:

https://jsfiddle.net/brL1x26g/1/

    .valueAccessor(function(p) {
      if(p.path) {
      	return p.value;
      }
      return p.value._nd.exceptionSum;
    })

Can you please validate the resulting chart is correct, if that is so, I will make the corresponding change in the sunburst code.

In all likelihood similar issue might exist while using keyAccessor as well.

@mukherjeea
Copy link
Author

Thanks @kum-deepak. This does look correct.

@kum-deepak
Copy link
Collaborator

@mukherjeea please continue with the workaround. The underlying code is proving difficult to fix. I will update when I have a clean solution implemented.

@gordonwoodhull
Copy link
Contributor

@kum-deepak, this problem of "synthetic nodes" is similar to the Others item in capped charts. There we use a special accessor which wraps the original accessor and does the same thing you show (but with a different field).

It just means the chart must consistently use the wrapped accessor instead of the original. I'm not sure if it solves all problems, but I think it does pretty well.

@kum-deepak
Copy link
Collaborator

Thanks for your timely advice @gordonwoodhull! I have attempted (see #1444), will need to work in tidying the example and figure out test cases.

@gordonwoodhull
Copy link
Contributor

Fixed by #1444

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 a pull request may close this issue.

3 participants