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

Blank text is not placed by texttemplate #4173

Closed
archmoj opened this issue Sep 9, 2019 · 6 comments · Fixed by #4179
Closed

Blank text is not placed by texttemplate #4173

archmoj opened this issue Sep 9, 2019 · 6 comments · Fixed by #4179
Assignees
Labels
bug something broken
Milestone

Comments

@archmoj
Copy link
Contributor

archmoj commented Sep 9, 2019

Codepen

cc: @antoinerg

@archmoj archmoj added the bug something broken label Sep 9, 2019
@archmoj archmoj added this to the v1.50.0 milestone Sep 9, 2019
@archmoj
Copy link
Contributor Author

archmoj commented Sep 9, 2019

I think this line

return v || v === 0;

should be

return v || v === '' || v === 0;

@etpinard
Copy link
Contributor

etpinard commented Sep 9, 2019

OR this line:

if(value) getterCache[key] = value;

@antoinerg antoinerg self-assigned this Sep 9, 2019
@antoinerg
Copy link
Contributor

antoinerg commented Sep 9, 2019

@etpinard The function templateFormatString actually consider empty string a valid value:

plotly.js/src/lib/index.js

Lines 1044 to 1047 in b5f0316

if(obj.hasOwnProperty(key)) {
value = obj[key];
break;
}

The change proposed by @archmoj in #4173 (comment) fixes the issue. Hopefully, it doesn't break anything else. Unfortunately, it does break other things.

@etpinard
Copy link
Contributor

etpinard commented Sep 9, 2019

Ok, but I would prefer not rendering <text> elements associated with '' strings.

@antoinerg
Copy link
Contributor

antoinerg commented Sep 9, 2019

@archmoj It doesn't seem like the issue lies with the texttemplate routine itself but rather in how sunburst calls it.

Did you encounter this error with other traces than sunburst?

This commit 9b4a38d should flag the traces other than sunburst for which this issue exists!

@antoinerg
Copy link
Contributor

antoinerg commented Sep 9, 2019

Ok so here are the traces affected:

  • pie
  • funnelarea
  • sunburst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants