-
Notifications
You must be signed in to change notification settings - Fork 20
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
Data point popups aren't at highest Z index #24
Comments
I would still like to submit a PR for this, but I would like to wait until #23 is merged, because that also changed the same area of the code. Is there any chance of that happening fairly soon? |
Will merge it over easter (today / tomorrow). Cheers
|
@lumean Thank you! |
To be honest, I can't tell if that would cause many issues. As you probably saw from the commit history I'm not extremely active on this project and just maintaining it in my free time. The biggest pain for me is to re-test all graph types and check if they still look correct. Before releasing a new version. I would appreciate a lot if you can also contribute an example or testcase that allows to easily verify the change (e.g. codesnipped you used for the screenshot above). If you want me to, I can release a beta/pre version of the gem with your changes so you can already test it with your application and give a short feeback before I release the final version. |
The screenshot is from http://covid-tracking-charts.herokuapp.com, and the code is at https://github.com/marnen/covid_tracking_charts/blob/master/app/models/chart.rb. Of course I'm intending to write tests for the change, as long as the code in this gem is actually testable (it's not great in that respect...) |
Yikes. This is really hard code to work with. @lumean I think I can refactor this whole gem to make it a lot more tractable, but it will be a good deal of work, and I'll probably need to change the tests to use RSpec if that's OK. Any objections or questions before I start? |
Data point popups are rendered in the SVG document just after the data point that they're associated with, but before the next data point. Unfortunately, unlike HTML, SVG 1.1 has no z-index attribute, but rather paints elements declared later in the document on top of elements declared earlier. This means that in a closely spaced graph, a popup can appear obscured by the following data point or its label:
![image](https://user-images.githubusercontent.com/34378/78853541-1d0fb480-79ed-11ea-8655-6569406ae702.png)
So what I propose (and will hopefully submit a PR for soon) is a slightly different rendering process, where the nodes for the data point popups aren't put into the SVG document till after all the data point nodes are written. Does that sound reasonable, or would that cause problems somewhere else?
BTW, I do really appreciate this gem quite a lot. It's making my life much easier on http://covid-tracking-charts.herokuapp.com. @lumean Thanks for maintaining it!
The text was updated successfully, but these errors were encountered: