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

Separate tooltips for candlestick hover #2959

Merged
merged 5 commits into from
Oct 2, 2018
Merged

Conversation

codrut3
Copy link
Contributor

@codrut3 codrut3 commented Aug 31, 2018

Hi,

I am interested in learning plotly.js and I thought I would make some contributions along the way :) I apologize if I am not following the rules properly.

This PR is for feature request #2706. I followed the suggestions in the comments and I modified the box hover code to work for ohlc and candlestick. Here is a screenshot of how it looks:

ohlc

I also added a test in hover_label_test. I should say that I don't really understand everything: for example, I defined spikeDistance for the points that the functions returns, even though I am not sure how it's used.

This commit adds a new attribute to ohlc and candlestick figures
called 'hoveron' (as in box hover). Setting 'hoveron' to 'ohlc'
shows at most 4 tooltips, for high, open, close and low. If several
values should appear at the same coordinate, they are shown together
in a single tooltip.
@alexcjohnson
Copy link
Collaborator

Thanks @codrut3 - very nice!
Grouping matching values into a single box is a particularly nice touch 🎉

hoveron in other trace types answers the question "which objects do you get hover info for" - for example for box it's a flaglist with options ['boxes', 'points'] - meaning you can choose 'boxes' to just hover on the boxes, 'points' to just hover on the individual sample points, or 'boxes+points' to show info for either/both. That's not really what you're doing here - it's always the same ohlc/candlestick combination object the user is hovering on, you're just controlling how the information about that object is displayed, and it wouldn't make sense to show both versions simultaneously (in this case 'ohlc+points' - that would just be redundant). Before we discuss an attribute name, type, and values that would make sense for this, I'll bring up what I said in #2706:

perhaps even just change to 4 separate tooltips as you suggest, rather than making this configurable

@etpinard @chriddyp @cldougl what do you think? Do we need to keep the existing behavior (a single hover label containing all four values) and allow this as an option, or should we just switch to this version? The best argument I can think of for the existing version is if users like to be able to read off the four values in a consistent order (top to bottom open, high, low, close) but I'm not really a user of this chart type myself.

To your other question:

I defined spikeDistance for the points that the functions returns, even though I am not sure how it's used.

This controls when to show spike lines, and which ones to show. Click "Toggle spike lines" in the modebar. This is different from distance because even in "compare" hovermode (where only the x value of the cursor is used to decide which objects to show hoverinfo for) we only show at most one spikeline. I'll play with it when this is further along to be sure, but as long as the spikelines in the new mode behave the same as in the existing version you should be fine.

(BTW happy Labor day 😅 - we may not hear back from other folks until next week)

@chriddyp
Copy link
Member

Looks great! I don't really have a preference for the default behavior.

@codrut3
Copy link
Contributor Author

codrut3 commented Sep 1, 2018

Hi @alexcjohnson ,

Thank you for the comments! I left hoveron because it was named so in box trace, and I couldn't think of another name. I know it doesn't fit the situation, so if you prefer another attribute name just let me know.

I also left 'points' as an option, because if the figure is small all the boxes are displayed close to one another, and it might be better to have the info in a single block. Like in the following example:
ohlc_first_hover1
ohlc_first_hover2

Also, I didn't add the text anymore, as hoverOnPoints does using fillHoverText. This is another difference between 'ohlc' and 'points'. Should I do this? If yes, should I add it to the high box?

I tried with "Toggle spike lines" and it looked fine to me.

I wish you a nice weekend! Enjoy Labor day! 😄
(I live in Switzerland. Monday is not free ☹️ )

@etpinard
Copy link
Contributor

etpinard commented Sep 4, 2018

Do we need to keep the existing behavior (a single hover label containing all four values) and allow this as an option

This gets my vote. Somewhat related to this PR, I've heard users on the forums wanting to show only a few selected sub-labels (e.g. q1,q3,mean) in box traces. So it would be a good time to start thinking about adding more per-trace hover labels attributes. In other words, something like hoverlabel.mode might not be adequate to cover both the "show this hover information in separate labels" case and the "show only these sub-labels". I'm starting to think that the missing attribute for this PR would work best as a boolean e.g hoverlabel.split.

Also, I didn't add the text anymore

This brings in an interesting point. Where should we place text in this new hover label configuration?

peek 2018-09-04 10-34

@codrut3
Copy link
Contributor Author

codrut3 commented Sep 5, 2018

Hi @etpinard,

That's a good idea and better than what I did. Should I add a new boolean attribute hoverlabel.split? I think this will make it available for all traces.

@etpinard
Copy link
Contributor

Should I add a new boolean attribute hoverlabel.split? I think this will make it available for all traces.

No, we don't want to make it available to all trace types. It should be available only for ohlc and candlestick.

The way to do it is:

  • add var fxAttrs = require('../../components/fx/attributes') in traces/ohlc/attributes.js,
  • extend fxAttrs with new key 'split',
  • link the new attribute object to key hoverlabel at the end of the ohlc attribute declarations. This will override the "global" hoverlabel attribute object set in plots/attributes.js
  • make candlestick reuses the ohlc hoverlabel attribute object.

@codrut3
Copy link
Contributor Author

codrut3 commented Oct 1, 2018

Hi @etpinard,

I changed the attribute to hoverlabel.split as you suggested.

@etpinard
Copy link
Contributor

etpinard commented Oct 2, 2018

Brilliant PR @codrut3

Thanks for very much 🎉

@gmarathi-nisum-com
Copy link

gmarathi-nisum-com commented Nov 20, 2018

@codrut3 Can we have this common one tooltip for other charts as-well ??
I am using a time series chart where I'm plotting different types of graphs. so when hover on a point, multiple tooltips shows-up which makes it clumsy.

Created a stackover-flow question for the same with code and screen shots.

https://stackoverflow.com/questions/53380131/customize-tooltip-for-multiple-graphs-in-one-plot
Please suggest a solution.

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.

None yet

5 participants