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

DOMUtils.getStringSize sometimes returns 0 width #224

Closed
amay opened this issue Sep 9, 2016 · 8 comments
Closed

DOMUtils.getStringSize sometimes returns 0 width #224

amay opened this issue Sep 9, 2016 · 8 comments
Labels
bug General bug label

Comments

@amay
Copy link
Contributor

amay commented Sep 9, 2016

I am getting an error in the x-axis labels for a graph where the labels run into each other:

screen shot 2016-09-09 at 4 48 09 pm

I have traced this down to the DOMUtils.getStringSize util function, and noticed that it returns a width of 0 in some cases.

To trigger this, I render a chart on one page, navigate away from that page and navigate to another page (navigation happens on client side, so no full page reload). On the second page, the 0 width error comes up. I also noticed that if I remove the span caching from getStringSize everything works as well. I haven't been able to figure out why this util breaks some of the time but not all of the time. Any assistance is much appreciated.

@xile611
Copy link
Member

xile611 commented Sep 10, 2016

@amay 😭 I can't reproduce this bug.

@amay
Copy link
Contributor Author

amay commented Sep 11, 2016

Thanks for taking a look. I realize I didn't give much to help in reproducing. I'll see if I can put together a little example of the bug to share with you.

@xile611
Copy link
Member

xile611 commented Sep 12, 2016

OK! Thank you!

@amay
Copy link
Contributor Author

amay commented Sep 12, 2016

Ok. I figured out how to reproduce it and made a little demo showing the issue: https://flame-healer.hyperdev.space/ (click the "Switch" button to render a new graph that shows the label collision issue).

The issue is that I'm using turbolinks on this project, so the body of the page gets re-rendered on page navigation. The effect of this is that the span that is automatically added to the DOM is removed. However, because turbolinks only replaces the body, and doesn't do a full browser navigation, we aren't reloading the page's javascript. The Recharts library caches the span in memory (via a pointer), and this pointer is not cleared when the user navigates via turbolinks. This means that the getStringSize attempts to use that span to do the dimension calculation, but the span is no longer in the DOM, so the result is 0.

I'm not sure what the best solution for this is. I attempted some hacks to make it so turbolinks doesn't remove the span, but none of those worked. If I could clear the cached span pointer, or turn off caching that would fix this issue for me. Please let me know what seems reasonable to you.

Thanks!

@amay
Copy link
Contributor Author

amay commented Sep 12, 2016

Small update. I did figure out a way to store the span element and restore it after turbolinks does the page navigation, so I have a workaround for this. If there was an API for clearing this cached span that would still be cleaner, but I am at least not experiencing the bug anymore.

@xile611 xile611 added the bug General bug label label Sep 14, 2016
@xile611
Copy link
Member

xile611 commented Sep 14, 2016

@amay Thank you! Maybe we should remove span cache.

@amay
Copy link
Contributor Author

amay commented Sep 14, 2016

We don't want to do that or else a new span will get added every time getStringSize is called, which is messy. A better approach would be to set a well known id on the measurement span (e.g. 'recharts_measurement_span' or something), and use document.getElementById('rechargs_measurement_span') to look up the span in getStringSize. If it doesn't exist in the body, then add it.

I made the change here #241.

@amay
Copy link
Contributor Author

amay commented Sep 18, 2016

@xile611 any feedback on #241?

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

No branches or pull requests

2 participants