search

RhoInc / Webcharts

Reusable, flexible, interactive charts with JavaScript
MIT License
34 stars 6 forks source link

'clip-path' of all marks should default to `url(#${chart.id})`. #235

Closed samussiah closed 5 years ago

samussiah commented 5 years ago

Test notes

jwildfire commented 5 years ago

@samussiah @pburnsdata Any idea why only drawBars has a clip-path property? Is the goal of this issue to add a clip-path to other marks, to fix the clip-path for bars, or both?

samussiah commented 5 years ago

Not sure, but the goal here is to contain the marks to their corresponding chart's canvas. We often set clip-path manually but when would it ever be set to anything other than the chart canvas.

pburnsdata commented 5 years ago

Yeah I can't think of any other situations... certainly not for typical use. And I don't know why the other marks don't have it either, but this would be a fix for bars - not the other marks.

I guess if we haven't seen the other marks going where they shouldn't yet - then it's not a concern?

samussiah commented 5 years ago

The issue arises when plotting marks that fall outside the x- or y-domain, for instance when applying a custom domain. Setting their clip-path prevents them from displaying.

pburnsdata commented 5 years ago

Ahhh gotcha, that makes sense. Wouldn't hurt to throw some .style('clip-path', 'url(#' + chart.id + ')') into drawLines and drawPoints then right? I'm assuming drawText might be outside of the chart area

samussiah commented 5 years ago

That's what I was thinking. I'd include text for consistency; we can always extend the margins or reposition the text to prevent clipping.