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

Fixup sankey text and shadow #5531

Merged
merged 11 commits into from
Mar 5, 2021
Merged

Fixup sankey text and shadow #5531

merged 11 commits into from
Mar 5, 2021

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Mar 4, 2021

Started out as a bugfix to take into account paper_bgcolor is sankey text-shadow following #5506,
We discovered that textfont.color was not able to control the color of labels on a sankey graph!
It addition to those fixes this PR resolves #3742.
One could also pass MathJax; but (at the moment) the label positioning is not perfect in that case.

Also vertical sankeygraphs used to trim the text to force the labels inside the node.
This behaviour is also changed in this PR and full labels are presented.

@plotly/plotly_js

cc: #5395

@archmoj archmoj added bug something broken feature something new Major change labels Mar 4, 2021
@archmoj archmoj added this to the NEXT milestone Mar 4, 2021
@archmoj archmoj requested a review from alexcjohnson March 4, 2021 18:27
@alexcjohnson
Copy link
Collaborator

Looks great - as discussed let's just prevent MathJax in Sankey with data-notex and this should be good to go!

@archmoj
Copy link
Contributor Author

archmoj commented Mar 4, 2021

Looks great - as discussed let's just prevent MathJax in Sankey with data-notex and this should be good to go!

Addressed in 2f38a9d.


nodeLabel
.style('text-shadow', function(d) {
return d.horizontal ? svgTextUtils.makeTextShadow(1, '#fff') : 'none';
.attr('data-notex', 1) // prohibit tex interpretation until we can handle tex and regular text together
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nonblocking, but this isn't quite the rationale here, here we don't want tex because of shadows and positioning issues :)

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 Looks great!

@archmoj archmoj merged commit ff0c00a into master Mar 5, 2021
@archmoj archmoj deleted the sankey-text-and-shadow branch March 5, 2021 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sankey: node labels do not render HTML tags
2 participants