search

blacklabel / annotations

Annotations plugin for Highstock by Black Label
http://blacklabel.github.io/annotations/
Other
22 stars 16 forks source link

xValue and yValue incorrectly computed for Bar chart #65

Open eltonjude opened 7 years ago

eltonjude commented 7 years ago

Consider a Column chart and I want to draw a circle for [Europe, 0]. Works as expected - http://jsfiddle.net/a5h870wt/2/

But for a Bar chart, the circle is not at the correct place: http://jsfiddle.net/a5h870wt/3/

The solution is to swap the computed x and y if they were calculated from xValue and yValue after this line: https://github.com/blacklabel/annotations/blob/master/js/annotations.js#L602 Here's how it looks: http://jsfiddle.net/a5h870wt/4/

I have a commit here with the fix: https://github.com/eltonjude/annotations/commit/d453f71f05d2350429bface0b126e45e6f948eef

If you could suggest other properties that would have similar issue, I can include fixes for them in a PR with the above fix.

Thanks

pawelfus commented 7 years ago

Hi @eltonjude

Thank you for sharing! How about checking chart.inverted instead of chart.options.chart.type? Bar chart in fact is a column type with inverted flag set to true. The same way we can get inverted spline, line etc. series. What do you think?

pawelfus commented 7 years ago

Related to #54.

eltonjude commented 7 years ago

I need to use shape.units = 'values' for bar chart and I feel it needs a fix too. I also created an example in which i used xValueEnd and yValueEnd for bar chart and it too needs the fix that was needed for xValue and yValue.

I will include these in a PR.

Thanks Elton

On Fri, Oct 21, 2016 at 9:23 AM, Elton Pereira eltonjude@gmail.com wrote:

It works with chart.inverted. Thanks! I guess I should also get it to work for xValueEnd and yValueEnd What about if shape.units = axis values? That also would be affected, isn't it?

On Fri, Oct 21, 2016 at 6:21 AM, Paweł Fus notifications@github.com wrote:

Hi @eltonjude https://github.com/eltonjude

Thank you for sharing! How about checking chart.inverted instead of chart.options.chart.type? Bar chart in fact is a column type with inverted flag set to true. The same way we can get inverted spline, line etc. series. What do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/blacklabel/annotations/issues/65#issuecomment-255346693, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdZr1p1DJ5IP-Ida__Gx54MgAOyR9jxks5q2JITgaJpZM4Kc03A .

eltonjude commented 7 years ago

It works with chart.inverted. Thanks! I guess I should also get it to work for xValueEnd and yValueEnd What about if shape.units = axis values? That also would be affected, isn't it?

On Fri, Oct 21, 2016 at 6:21 AM, Paweł Fus notifications@github.com wrote:

Hi @eltonjude https://github.com/eltonjude

Thank you for sharing! How about checking chart.inverted instead of chart.options.chart.type? Bar chart in fact is a column type with inverted flag set to true. The same way we can get inverted spline, line etc. series. What do you think?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/blacklabel/annotations/issues/65#issuecomment-255346693, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdZr1p1DJ5IP-Ida__Gx54MgAOyR9jxks5q2JITgaJpZM4Kc03A .

pawelfus commented 7 years ago

Yes, probably all the values should be swapped for an inverted chart. Annotations were implemented as plugin for Highstock, which doesn't have inverted option :)