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

Zoom should be called when chart is applicable for it #125

Open
random-one opened this issue Jan 10, 2017 · 4 comments
Open

Zoom should be called when chart is applicable for it #125

random-one opened this issue Jan 10, 2017 · 4 comments

Comments

@random-one
Copy link

random-one commented Jan 10, 2017

I asked my question in #89, but since it's closed I think I should submit it as an issue, so I'm moving my comment here.

I think I have a problem with the check you dropped for the zoom in v0.3.2.
I have a pie chart that gets redrawn because of this zoom and I receive following error in c3:

Uncaught TypeError: Cannot read property 'data_types' of null at ChartInternal.c3_chart_internal_fn.isPieType (c3.js:3581)

I don't think pie charts are applicable for zoom, so why to redraw the chart once again?
Maybe the problem is somewhere else but this is where I got so far.

@maxklenk Could you please check this? I also tested c3 only without angular-chart and without angular and could not reproduce the error.

At the point when applyZoom is called and the error happens I have a baseConfiguration that's basically an empty object - no data['json'], no data['types'], which causes the c3 crash.

@maxklenk
Copy link
Member

Thank you for create an issue for your problem, this helps a lot.

I was not able to reproduce your problem, can you please adopt this plnkr to show me when it fails.

https://plnkr.co/edit/BT1CFkMAXFTjo5Jzh1LK?p=preview

@random-one
Copy link
Author

@maxklenk I managed to reproduce it but including one more dependency - ui-router. This is actually my real use case, but I never thought this could break the charts...
Please see my plunk here: https://plnkr.co/KOMKGekcJNkQVOIblFum
It's forked from yours. Switch quickly between the two states(to reproduce it easier) with the buttons on each state until you see the this.config is null message in the developer console and the chart will not appear until app is reloaded.

@maxklenk
Copy link
Member

puh, I see. Interesting bug, we should check if destruction of the chart is not finished and is responsible for this strange behavior. If you find a solution please tell me.

@random-one
Copy link
Author

random-one commented Jan 10, 2017

Like I told you I traced it to the applyZoom in the ChartService.prototype.stateCallback function. And when I don't call it it does not happen which is strange to me too.
My "solution"(and I am not proud of it, but I need the charts to always appear) is a monkey-patched chartService that uses the code of applyZoom before you removed the check.

Maybe this stateCallback needs to me smarter?

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

No branches or pull requests

2 participants