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

Stacked Column Chart Tooltip issues in 5.0.0 #5832

Closed
patrickbrownsync opened this Issue Oct 14, 2016 · 8 comments

Comments

Projects
None yet
3 participants
@patrickbrownsync

patrickbrownsync commented Oct 14, 2016

Expected behaviour

When using point placement with a stacked column chart, shared labels should show all stacks, worked in 4.2

Actual behaviour

When point placement is added, stacks do not appear correctly in shared tooltip.

Live demo with steps to reproduce

http://jsfiddle.net/brzs1vLw/

Affected browser(s)

Chrome

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Oct 17, 2016

Collaborator

Thanks for reporting!

I'm not sure why you use pointPlacement, but if your objective is to pack the columns tighter together, you should use the pointPadding option: http://jsfiddle.net/highcharts/brzs1vLw/1/.

Nevertheless, it is a regression.

Collaborator

TorsteinHonsi commented Oct 17, 2016

Thanks for reporting!

I'm not sure why you use pointPlacement, but if your objective is to pack the columns tighter together, you should use the pointPadding option: http://jsfiddle.net/highcharts/brzs1vLw/1/.

Nevertheless, it is a regression.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Oct 17, 2016

Collaborator

Fails since v4.2.7, 5ef14ea

Removing the pointPlacement parameter on this line fixes the issue, but obviously causes the other ones to re-emerge.

Probably we don't want to modify the clientX value when we have a shared tooltip, but at this point (in Series.translate) we don't know if the tooltip is shared or not. What do you think @pawelfus? Do we have another way to resolve #5518?

Collaborator

TorsteinHonsi commented Oct 17, 2016

Fails since v4.2.7, 5ef14ea

Removing the pointPlacement parameter on this line fixes the issue, but obviously causes the other ones to re-emerge.

Probably we don't want to modify the clientX value when we have a shared tooltip, but at this point (in Series.translate) we don't know if the tooltip is shared or not. What do you think @pawelfus? Do we have another way to resolve #5518?

@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Oct 17, 2016

Contributor

Not really - I think it should be there. As an alternative, we can make this less strict and involve point.x there, for example:

while (i--) {
    if ((kdpoints[i].clientX !== kdpoints[0].clientX && kdpoints[i].x !== kdpoints[0].x) || kdpoints[i].series.noSharedTooltip) {
        kdpoints.splice(i, 1);
    }
}

It will filter out cases, where clientX is different by pointPlacement, but x-values are in fact excatly the same.

Edit: That solution won't resolve displacement with +/- 1 and above, for example blue series: http://jsfiddle.net/brzs1vLw/4/ - however, pointPlacement with value higher than +1 (or lower than -1) doesn't really make sense to me.

Contributor

pawelfus commented Oct 17, 2016

Not really - I think it should be there. As an alternative, we can make this less strict and involve point.x there, for example:

while (i--) {
    if ((kdpoints[i].clientX !== kdpoints[0].clientX && kdpoints[i].x !== kdpoints[0].x) || kdpoints[i].series.noSharedTooltip) {
        kdpoints.splice(i, 1);
    }
}

It will filter out cases, where clientX is different by pointPlacement, but x-values are in fact excatly the same.

Edit: That solution won't resolve displacement with +/- 1 and above, for example blue series: http://jsfiddle.net/brzs1vLw/4/ - however, pointPlacement with value higher than +1 (or lower than -1) doesn't really make sense to me.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Oct 17, 2016

Collaborator

It sounds like a good idea... Do we need the check for clientX at all? Can we think of situations were a test for point.x is not sufficient? By definition, the shared tooltip shows points of the same x value. The clientX property is used in

  • Heat maps for getting the tooltip in the center (shared tooltips don't apply)
  • K-d-tree for getting the closest point in x and y dimensions. Shared tooltips don't apply because they only search for the closest point in the X dimension (?)

If you can't think of other problem areas I suggest we replace the comparing clientX with simply comparing point.x.

Collaborator

TorsteinHonsi commented Oct 17, 2016

It sounds like a good idea... Do we need the check for clientX at all? Can we think of situations were a test for point.x is not sufficient? By definition, the shared tooltip shows points of the same x value. The clientX property is used in

  • Heat maps for getting the tooltip in the center (shared tooltips don't apply)
  • K-d-tree for getting the closest point in x and y dimensions. Shared tooltips don't apply because they only search for the closest point in the X dimension (?)

If you can't think of other problem areas I suggest we replace the comparing clientX with simply comparing point.x.

@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Oct 17, 2016

Contributor

Do we need the check for clientX at all? Can we think of situations were a test for point.x is not sufficient?

I think we should. The test for x only will always fail when using two (or more) separate xAxes, for example when comparing different weeks/months of the same data set: http://jsfiddle.net/jhyhn2ty/ - quite common use case for a Highstock, which is very easy to achieve with an ordinal xAxis.. (I mean aligned points with the same clientX, but x-values are different).

K-d-tree for getting the closest point in x and y dimensions. Shared tooltips don't apply because they only search for the closest point in the X dimension (?)

Not exactly, it finds all closest point by x-dimension, as you said, but according to x+y distance we can determine which of the points is the closest one (for example click event on a tracker - that param byProximity in mouse over method). But we can try to make this work.

Contributor

pawelfus commented Oct 17, 2016

Do we need the check for clientX at all? Can we think of situations were a test for point.x is not sufficient?

I think we should. The test for x only will always fail when using two (or more) separate xAxes, for example when comparing different weeks/months of the same data set: http://jsfiddle.net/jhyhn2ty/ - quite common use case for a Highstock, which is very easy to achieve with an ordinal xAxis.. (I mean aligned points with the same clientX, but x-values are different).

K-d-tree for getting the closest point in x and y dimensions. Shared tooltips don't apply because they only search for the closest point in the X dimension (?)

Not exactly, it finds all closest point by x-dimension, as you said, but according to x+y distance we can determine which of the points is the closest one (for example click event on a tracker - that param byProximity in mouse over method). But we can try to make this work.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Oct 18, 2016

Collaborator

But using a shared tooltip with two X axes is wrong under any circumstances, in your sample the tooltip header shows only the date of the first point: http://jsfiddle.net/jhyhn2ty/

In this case we would at least need to get rid of the shared header.

Collaborator

TorsteinHonsi commented Oct 18, 2016

But using a shared tooltip with two X axes is wrong under any circumstances, in your sample the tooltip header shows only the date of the first point: http://jsfiddle.net/jhyhn2ty/

In this case we would at least need to get rid of the shared header.

@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Oct 18, 2016

Contributor

Yes, of course - it was just a minimal example, an idea.

Contributor

pawelfus commented Oct 18, 2016

Yes, of course - it was just a minimal example, an idea.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Oct 20, 2016

Collaborator

I think we'll give it a try with comparing x-es...

Collaborator

TorsteinHonsi commented Oct 20, 2016

I think we'll give it a try with comparing x-es...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment