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

Tooltip text not working when text is 0 (number) #2660

Closed
stillesjo opened this issue May 24, 2018 · 7 comments · Fixed by #2682
Closed

Tooltip text not working when text is 0 (number) #2660

stillesjo opened this issue May 24, 2018 · 7 comments · Fixed by #2682
Labels
bug something broken

Comments

@stillesjo
Copy link

Example that displays the error

I want to be able to get a value from the text property in the data object. I noticed that if the value is the number 0 it won't be displayed in the plot.

          text: [0, 0, 0, 0, 0],
          hoverinfo: "x+text+name",

The code above will make the tooltip only show x and name. The text property won't be visible.
image

All other values except 0 will render properly, as can be seen in the code example linked above. Also if all the numbers are strings they will be shown correctly.

I guess that there is a null-check somewhere that treats 0 as false and therefore won't add it to the tooltip.

@dalle
Copy link
Contributor

dalle commented May 24, 2018

I think this will fix it:

diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js
index 0e2dd1b10..ae9c8120e 100644
--- a/src/components/drawing/index.js
+++ b/src/components/drawing/index.js
@@ -650,7 +650,7 @@ drawing.textPointStyle = function(s, trace, gd) {
         var p = d3.select(this);
         var text = Lib.extractOption(d, trace, 'tx', 'text');

-        if(!text) {
+        if(!text && text !== 0) {
             p.remove();
             return;
         }

@alexcjohnson
Copy link
Collaborator

Thanks @stillesjo ! Interestingly we have a comment implying that we had addressed exactly this issue, so we'll have to look at it.

// accept all truthy values and 0 (which gets cast to '0' in the hover labels)
function isValid(v) {
return v || v === 0;
}

@dalle thanks, that would fix a related bug - display of the text alongside the markers. Add this to each of the traces in @stillesjo's example:

mode: 'lines+markers+text',
textposition: 'top right'

and you'll see the zeros get left out there too:
screen shot 2018-05-24 at 9 09 47 am

@alexcjohnson alexcjohnson added the bug something broken label May 24, 2018
@dalle
Copy link
Contributor

dalle commented May 25, 2018

Okay, one more try:

diff --git a/src/components/fx/hover.js b/src/components/fx/hover.js
index 0da92d4a8..ff8175c69 100644
--- a/src/components/fx/hover.js
+++ b/src/components/fx/hover.js
@@ -842,7 +842,7 @@ function createHoverText(hoverData, opts, gd) {
         else if(d.yLabel === undefined) text = d.xLabel;
         else text = '(' + d.xLabel + ', ' + d.yLabel + ')';

-        if(d.text && !Array.isArray(d.text)) {
+        if((d.text || d.text === 0) && !Array.isArray(d.text)) {
             text += (text ? '<br>' : '') + d.text;
         }

@alexcjohnson
Copy link
Collaborator

@dalle Looks likely that's the culprit - any chance you'd like to make a PR combining those two diffs with a test covering both parts? Perhaps based on:

describe('hover info text', function() {

@dalle
Copy link
Contributor

dalle commented May 28, 2018

@alexcjohnson Creating a new hover label test seems easy, but I could not understand how to test for the marker.

@alexcjohnson
Copy link
Collaborator

Creating a new hover label test seems easy

Ah great, that's the harder test!

but I could not understand how to test for the marker.

Once you've made a plot that has text on the markers (which is done in the beforeEach block in the linked test) you should just be able to do something like:

d3.select(gd).selectAll('.textpoint text').size()

to check that the correct number of text markers are created - in the codesandbox example above, the .textpoint groups are empty in the bug case.

@dalle
Copy link
Contributor

dalle commented May 30, 2018

@alexcjohnson Did I create the PR correctly?

dalle added a commit to dalle/plotly.js that referenced this issue May 30, 2018
* treat zero as valid fixes plotly#2660

* PR suggestions: toBe instead of toEqual
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