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

Updating 3d columns with z stacking fails #4743

Closed
TorsteinHonsi opened this Issue Nov 16, 2015 · 5 comments

Comments

Projects
None yet
3 participants
@TorsteinHonsi
Collaborator

TorsteinHonsi commented Nov 16, 2015

Demo: http://jsfiddle.net/highcharts/m69rbc9p/15/

The demo shows updating showInLegend, but it also applies to changing Y value etc.

@TorsteinHonsi TorsteinHonsi added the Bug label Nov 16, 2015

@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Nov 16, 2015

Contributor

I think this example shows better what happens: http://jsfiddle.net/m69rbc9p/16/ - all columns are placed with wrong groupZPadding.

Contributor

pawelfus commented Nov 16, 2015

I think this example shows better what happens: http://jsfiddle.net/m69rbc9p/16/ - all columns are placed with wrong groupZPadding.

@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Nov 16, 2015

Contributor

Small debugging:

This is caused by this line - series._i is wrong (?) when series are updated because of this line. We can replace series._i with series.index, however, there will be still problem with zIndexes caused by those line. When removing this part we get working code - all tests are passed, but I'm not sure what is the purpose of this part..

Contributor

pawelfus commented Nov 16, 2015

Small debugging:

This is caused by this line - series._i is wrong (?) when series are updated because of this line. We can replace series._i with series.index, however, there will be still problem with zIndexes caused by those line. When removing this part we get working code - all tests are passed, but I'm not sure what is the purpose of this part..

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Nov 16, 2015

Collaborator

Okay, please feel free to provide a fix. As for the lines in Column.js, they point to #4062 so at least you need to check that.

Collaborator

TorsteinHonsi commented Nov 16, 2015

Okay, please feel free to provide a fix. As for the lines in Column.js, they point to #4062 so at least you need to check that.

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Nov 16, 2015

Collaborator

... and otherwise try to replicate the conditions of those lines and see what it does.

Collaborator

TorsteinHonsi commented Nov 16, 2015

... and otherwise try to replicate the conditions of those lines and see what it does.

@pawelfus pawelfus self-assigned this Nov 16, 2015

pawelfus added a commit that referenced this issue Dec 1, 2015

@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Dec 1, 2015

Contributor

I didn't found any case, when it fails. There is only one thing I am concerned about: zIndexes are floating point numbers, not integers - we had similar issue with pie chart a while ago, but I don't want to add more logic if this is not necessary.

Additionally, I pushed test for #4062.

Contributor

pawelfus commented Dec 1, 2015

I didn't found any case, when it fails. There is only one thing I am concerned about: zIndexes are floating point numbers, not integers - we had similar issue with pie chart a while ago, but I don't want to add more logic if this is not necessary.

Additionally, I pushed test for #4062.

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