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

xValue and yValue incorrectly computed for Bar chart #65

Open
eltonjude opened this issue Oct 21, 2016 · 5 comments
Open

xValue and yValue incorrectly computed for Bar chart #65

eltonjude opened this issue Oct 21, 2016 · 5 comments
Labels

Comments

@eltonjude
Copy link

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: eltonjude@d453f71

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
Copy link
Collaborator

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
Copy link
Collaborator

Related to #54.

@pawelfus pawelfus added the bug label Oct 21, 2016
@eltonjude
Copy link
Author

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 [email protected] 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 [email protected]
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
#65 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAdZr1p1DJ5IP-Ida__Gx54MgAOyR9jxks5q2JITgaJpZM4Kc03A
.

@eltonjude
Copy link
Author

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 [email protected] 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
#65 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAdZr1p1DJ5IP-Ida__Gx54MgAOyR9jxks5q2JITgaJpZM4Kc03A
.

@pawelfus
Copy link
Collaborator

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 :)

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

No branches or pull requests

2 participants