chartist
chartist copied to clipboard
Use two different containers for horizontal and vertical lines
This makes styling the grid much more flexible, because it allows css selectors such as ":first-child", ":last-child", ":nth-child(even)", etc to be used.
Hi there. Thanks for the PR! Please fix the failing tests before we process this any further. I guess this is not really a breaking change, although it will effect current stylings in some projects I guess. We should ship this with a minor not with a patch I guess.
I fixed the tests. And yes, I agree that this this should not be in 0.9.x, but rather in 0.10 or 1.0. But that choice is up to you =)
I made the same change for the labels as I had the same "issue" with the labels as well (not being able to properly apply alternate styles using "nth-child(even)" and the like.
The question now is, do we still need the .ct-vertical and .ct-horizontal classes? Since we could rely on child selectors now right. If this a breaking change anyway, we should flag it for 1.0.0 and even remove the .ct-vertical and .ct-horizontal classes. But then we also need to re-write our Sass files.
I've used the classes in the "chart.on('draw', ...)" callback to distinguish which element I'm currently modifying. That might be a reason to keep them, but with an additional line of code I could also read the class from the parent element ;-) Other people might rely on those classes for the styling. I don't really see a reason why they need to be removed. It's not that this would hurt performance.
Hi @mweimerskirch Can you please resolve the conflicts with the recently added grid background functionality by @hansmaad ? After this I think we're good to go! Sorry for the delay here.
I actually don't see any value being added here.
You can already select first/last grid lines with the -of-type
selectors.
.ct-grid.ct-horizontal:first-of-type
: First horizontal grid line
.ct-grid.ct-vertical:last-of-type
: Last vertical grid line
There's even :nth-of-type()
to target a specific index of horizontal/vertical. This PR only adds more dom node levels, more logic and would break users' existing css for grid lines.
@JonDum No, first-of-type or nth-of-type only matches the element name, not the class. So if you the following structure:
<div class="ct-vertical"></div>
<div class="ct-vertical"></div>
<div class="ct-horizontal"></div>
<div class="ct-horizontal"></div>
... there is no way to match the first element with that class. ":nth-match" will be able to do that, but that's not supported by any browser yet, as it's still part of the CSS4 draft.
@gionkunz I rebased the code and adapted it for the grid backgrounds. I placed the grid background element in the same container as the horizontal lines. Not sure if that is the best way though.
:shipit: but maybe retain emitting the ct-grids
and ct-labels
css classes for backwards compatibly?
e.g. emit BOTH classes so existing stuff that styles via ct-grids
still works