Skip to content

Add 'hovertext' attribute in scatter* traces #1523

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

Merged
merged 13 commits into from
Apr 7, 2017
Merged

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Mar 28, 2017

Scatter traces (i.e. scatter, scatter3d, scattergeo, scatterternary and scattermapbox) have a text attribute which is displayed on the graph when mode contains the 'text' flag. The text content is also displayed in the hover label when hoverinfo contains the 'text' flag.

But, what if a user wants to display some text elements on the chart and some other (distinct) text elements in the hover labels? Currently we would have to use two traces to achieve this.

So, I proposed adding a hovertext attribute. Now it's a little difficult to preserve backward compatible; we should make sure to agree on the details.

if(di.tx) pointData.text = di.tx;
if(di.htx) pointData.text = di.htx;
else if(trace.hovertext) pointData.text = trace.hovertext;
else if(di.tx) pointData.text = di.tx;
Copy link
Contributor Author

@etpinard etpinard Mar 28, 2017

Choose a reason for hiding this comment

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

This is what I came up with:

  • If trace hoverinfo contains a 'text' flag and hovertext is not set the text elements will be seen in the hover labels
  • If trace hoverinfo contains a 'text' flag and hovertext is set, hovertext takes precedence over text i.e. the hoverinfo elements will be seen in the hover labels

@etpinard
Copy link
Contributor Author

Would love some 👍 s from @alexcjohnson @bpostlethwaite @n-riesco

@n-riesco
Copy link
Contributor

👍 from me. text and hovertext.

Besides the use case described by Étienne, I also find, from a user point of view, that hovertext is an easier concept than text+hoverinfo.

@alexcjohnson
Copy link
Collaborator

👍 seems clear and I think it covers all use cases. I suppose you could argue for the ability to display BOTH text and hovertext in the hover labels, but that opens a big can of worms just to avoid a bit of duplication in hovertext so doesn't seem like a good idea.

Bar and pie could also benefit from this feature.

@etpinard
Copy link
Contributor Author

Bar and pie could also benefit from this feature.

Good call. Thanks!

@etpinard
Copy link
Contributor Author

Thanks @n-riesco @alexcjohnson , dropping the discussion needed label.

@etpinard
Copy link
Contributor Author

etpinard commented Mar 29, 2017

@@ -79,7 +79,27 @@ module.exports = {

text: {
valType: 'data_array',
Copy link
Contributor Author

@etpinard etpinard Apr 3, 2017

Choose a reason for hiding this comment

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

@alexcjohnson I just noticed that text for pie is declared as a data_array whereas other trace types have it as a string with arrayOk: true. Was that on purpose? Or should we allow single-value text input for pie traces?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That was on purpose - though probably never explicitly discussed. My rationale is that text is essentially the x data for pies, and they would be meaningless without this being an array of data. Can you think of a counterexample?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree for the on-graph textinfo: 'text' part. But if someone wants to show the same text piece in all hover labels? But I guess after this PR, hovertext will cover that case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh sorry, that's label... I still don't see a good usage for a single-value text but perhaps it would be better to define it consistently with other trace types anyway.


var labels = Plotly.d3.selectAll('.hovertext .nums .line');
expect(labels[0].length).toBe(expected.length);
Copy link
Collaborator

Choose a reason for hiding this comment

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

🐄 labels.size()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good 👁️ thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 1bf54e2

@alexcjohnson
Copy link
Collaborator

nice. 💃

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

Successfully merging this pull request may close these issues.

3 participants