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 & treemap root color attribute #5232

Merged
merged 4 commits into from
Nov 3, 2020

Conversation

thierryVergult
Copy link
Contributor

new attribute for treemap & sunburst traces to set the color of the root node.

Setting the color of the root node of a treemap can also be used to fix the height of a treemap. By default the root node has no color, and when doing a drilldown, the surface of the root nood is overwritten with descendent data. Having a root node color set makes the surface used more prominent visible.

See also #5199

@archmoj archmoj added status: reviewable community community contribution feature something new labels Oct 24, 2020
@archmoj
Copy link
Contributor

archmoj commented Oct 27, 2020

Please also consider handling the root/trunk color here:

if(isRoot && fillColor === 'rgba(0,0,0,0)') {
opacity = 0;
lineColor = 'rgba(0,0,0,0)';
lineWidth = 0;
} else {

should become something like:

        if(isRoot && fillColor === trace.root.color) {
            opacity = 100;
            lineColor = 'rgba(0,0,0,0)';
            lineWidth = 0;
        } else {

Thanks.

@archmoj
Copy link
Contributor

archmoj commented Oct 27, 2020

Then please add root/trunk color equal yellow to all the traces of the following: mocks:
https://github.com/plotly/plotly.js/blob/master/test/image/mocks/treemap_textposition.json
https://github.com/plotly/plotly.js/blob/master/test/image/mocks/sunburst_inside-text-orientation_clock

to regenerate the new baseline:
once

npm run docker -- run 

then

npm run baseline treemap_textposition sunburst_inside-text-orientation_clock

More info: https://github.com/plotly/plotly.js/blob/master/CONTRIBUTING.md#image-pixel-comparison-tests

Also if you had difficulty making these baselines; you could give us (as maintainers) the permission to push to your branch so that I could generate the baselines for you!
see https://github.com/plotly/plotly.js/blob/master/.github/PULL_REQUEST_TEMPLATE.md

Thanks.

@archmoj archmoj added this to the v1.58.0 milestone Oct 27, 2020
@nicolaskruchten
Copy link
Contributor

I like this new attribute in principle :)

@archmoj
Copy link
Contributor

archmoj commented Nov 3, 2020

@thierryVergult we are planning next plotly.js minor (possibly next week) to include new features like this one.
Would you please addressing the following comments?
#5232 (comment)
#5232 (comment)
Thanks.

@thierryVergult
Copy link
Contributor Author

I'm pretty busy these days. Not sure I will find some time this week for further follow up.

@archmoj
Copy link
Contributor

archmoj commented Nov 3, 2020

I'm pretty busy these days. Not sure I will find some time this week for further follow up.

Thanks for the info.
💃
And I'll open up a PR shortly to address my comment and add tests.

@archmoj archmoj merged commit aad34f1 into plotly:master Nov 3, 2020
@thierryVergult thierryVergult deleted the treemapDrilldown branch June 1, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants